Skip to content
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

Do not look for translations if no locale #12940

Conversation

matks
Copy link
Contributor

@matks matks commented Mar 15, 2019

Questions Answers
Branch? develop
Description? You can create new languages from the BO but they get created without a locale. This can create performance issues when the core looks for related translations. This avoids the performance issue.
Type? bug fix
Category? CO
BC breaks? no
Deprecations? no
Fixed ticket?
How to test? Waiting for @jocel1 to explain it a bit 😉

This change is Reviewable

@prestonBot prestonBot added develop Branch Bug Type: Bug labels Mar 15, 2019
@PierreRambaud
Copy link
Contributor

It's because we don't want to get all catalog if there is no locale set.
So $translator->getCatalogue($locale)->all() will be not executed and it'll improve performance a lot.

@PierreRambaud PierreRambaud added this to the 1.7.6.0 milestone Mar 15, 2019
@jocel1
Copy link
Contributor

jocel1 commented Mar 16, 2019

@PierreRambaud @matks actually it's not the getCatalogue() which hurts, it's the Finder::create() just after, because the pattern name ->name('*.' . $locale . '.*'); becomes *..* if locale is empty, and it matches a lot of file and directories!
So how to test: manually create a language, try to display a landing page with the language, with open_base_dir on to disable the filesystem cache (the Finder::create() will hurt a lot in this case). You can do a blackfire profiling to see the impact.

@PierreRambaud PierreRambaud added the Waiting for QA Status: action required, waiting for test feedback label Mar 17, 2019
@PierreRambaud
Copy link
Contributor

Thanks @jocel1 for explanation 😉

@ntiepresta ntiepresta self-assigned this Mar 18, 2019
@ntiepresta ntiepresta added Waiting for QA Status: action required, waiting for test feedback QA ✔️ Status: check done, code approved and removed Waiting for QA Status: action required, waiting for test feedback labels Mar 18, 2019
@ntiepresta ntiepresta removed their assignment Mar 18, 2019
@matks
Copy link
Contributor Author

matks commented Mar 18, 2019

Thank you for review and comments !

@matks matks merged commit fac79d2 into PrestaShop:develop Mar 18, 2019
@matks matks deleted the avoid-parsing-mock-translations-for-fake-language branch March 18, 2019 16:50
@kpodemski
Copy link
Contributor

Wow, that was a tricky one, i stumble upon this issue but couldn't find proper solution 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Type: Bug develop Branch QA ✔️ Status: check done, code approved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants