-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Translation] Speed up TranslationDebugCommand #49585
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Hi @enumag, thank you for this contribution. As you said you get the very same result with or without this line in At first sight, I tend to agree with your fix, as |
@welcoMattic Not sure what test you mean. There is https://github.com/symfony/symfony/blob/6.3/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/TranslationDebugCommandTest.php and this one is still passing. I wasn't using TranslationUpdateCommand but not that you mention it it actually has the exact same problem:
|
If we take a step back, those 2 commands should find translations keys from your code. However, translation keys could be located anywhere in If you need to narrow the search location, there is I'm afraid that if we remove |
That's true but as far as I can tell the point of analyzing which services are using TranslatorInterface in TranslatorPass is to limit the paths which will be searched. There is no point in analyzing the entire src directory when we already have a much narrower list to search.
IIRC bundles are no longer recommended. My app isn't using them so this isn't an option.
Even if you want to keep the src there for compatibility with some obscure cases that TranslatorPass would miss shouldn't there be an option to disable it? |
Could you test something with the |
@welcoMattic I had no clue such function existed. How does it even locate a translator instance? Anyway yeah there is no way those calls would be found without the global search. |
The |
@welcoMattic Ah I see. Yeah I'm pretty sure those wouldn't be discovered in that case and kinda make the inclusion of |
What is the slow part? Is it parsing with PhpParser? Or just opening them all? If it's PhpParser, can we fuzzy search the content before parsing it using some clever |
@nicolas-grekas I don't have a way to easily check the bottleneck since blackfire.io removed the free tier. |
That said I believe that in my case the main issue is the sheer number of files in /src directory. The project is large and only a few files are using translations. |
Can you maybe give my idea a try to see if that provides anything measurable for you? |
@nicolas-grekas I tried your way by changing PhpExtractor like this: public function extract($resource, MessageCatalogue $catalog)
{
$files = $this->extractFiles($resource);
foreach ($files as $file) {
$contents = file_get_contents($file);
if (strpos($contents, 'trans') !== false) {
$this->parseTokens(token_get_all($contents), $catalog, $file);
gc_mem_caches();
}
}
} Current implementation: 302 seconds While your idea does provide significant improvement it's still nowhere near fast enough. |
I would still to it. Dividing the time by 5 is still worth it. |
Unfortunately your idea isn't good enough for me so I won't spend time on implementing it. I'm forced to decorate the extractor to ignore the src directory no matter if your idea is implemented or not. |
Removing the whole src directory is a no-go, as this would break the feature (as explained before) |
I'm aware but I'd still like to see an option to suppress it. |
Let's close then. PR welcome if someone wants to submit the perf improvement that has been discussed. And PR welcome also of course if another approach could work for what you're trying to do @enumag |
…or big code bases (welcoMattic) This PR was merged into the 6.3 branch. Discussion ---------- [Translation] Improve message extraction performance for big code bases | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | no | New feature? | yes <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tickets | Fix #49585 <!-- prefix each issue number with "Fix #", no need to create an issue if none exists, explain below instead --> | License | MIT | Doc PR | As discussed in #49585, we try to extract messages from all src/ directory in `translation:debug` and `translation:extract` commands. But, it's not necessary to look into all files, as we can easily detect if a file contains something about translation (with some regexes). This PR filter files before messages extraction, and it divide by at least 3 the time used to extract messages from an app with 100 000 php files. Commits ------- ae6c25b fix(translation): Improve performance of debug and extract command for big code base
The
TranslationDebugCommand
is currently very very slow. I was digging in Symfony codebase and it turns out that Symfony has quite a few optimizations to limit the search paths for this command to only files where translator is used and twig templates:But all of this hard work is then thrown out of the window by the command itself adding
$kernel->getProjectDir().'/src'
to the list - making it impossible to search only the optimized list and searching the entire project anyway. In my case the command takes over 5 minutes with this line but less then 5 seconds when I remove it while keeping the exact same result.