Page MenuHomePhabricator

Figure out how to make LocalisationCache a service without hurting performance
Closed, ResolvedPublic

Description

Context: T231198, T231200, https://gerrit.wikimedia.org/r/c/mediawiki/core/+/532399. LocalisationCache was made a service in e4468a1d6b6b9fdc5b64800febdc8748d21f213d, but this apparently caused severe performance problems. Presumably this is because suddenly we were re-fetching all the messages on every request. The question is how to solve the test performance problem while still keeping LocalisationCache a service.

I don't know how to answer this question because I don't understand well enough what the code here is doing. I think the solution should boil down to a per-process cache of the parsed data that survives a service reset, assuming tests don't do anything that can affect the data's parsing. But I don't know the best way to do that. Perhaps tests should configure LocalisationCache to use an LCStoreStaticArray that's persisted between tests, and tests that want to not use it can opt out somehow?

This blocks all my Language work (which is five outstanding patches, not counting LocalisationCache and LanguageNameUtils which are in the process of being reverted).

Details

Related Gerrit Patches:
mediawiki/core : masterMake LocalisationCache a service
mediawiki/core : REL1_34Make LocalisationCache a service

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 26 2019, 5:48 PM

By the way, @Ladsgroup, how did you actually measure the performance hit here? I'm going to need a way to test it to make sure I've fixed the problem before re-committing.

(For the future, perhaps we should make performance drops on test runs block merging if they're big enough, like a 20% increase in overall test run time or something.)

Krinkle added a comment.EditedAug 26 2019, 6:36 PM

See also T225730#5400979 which is about making Quibble use a file-based LCStore (instead of LCStoreDB). The patch for it was merged, but not yet deployed (ping @hashar, @Ladsgroup).

We can also think about setting $wgUseLocalMessageCache to true by default in core (or start by enabling it in DevelopmentSettings.php only, for a few months first). This setting enables an in-memory APC layer on top of LCStore for calls from wfMessage().

Krinkle added a subscriber: hashar.Aug 26 2019, 6:36 PM

See also T225730#5400979 which is about making Quibble use a file-based LCStore (instead of LCStoreDB). The patch for it was merged, but not yet deployed (ping @hashar, @Ladsgroup).

Filled a task to document the update process: T231251 No clue where to write down the doc though.

Change 532679 had a related patch set uploaded (by simetrical; owner: simetrical):
[mediawiki/core@master] Make LocalisationCache a service

https://gerrit.wikimedia.org/r/532679

daniel triaged this task as Medium priority.Sep 30 2019, 4:29 PM

Change 532679 merged by jenkins-bot:
[mediawiki/core@master] Make LocalisationCache a service

https://gerrit.wikimedia.org/r/532679

hashar closed this task as Resolved.Oct 8 2019, 9:54 AM
hashar assigned this task to Simetrical.

With https://gerrit.wikimedia.org/r/532679 , the PHPUnit test now seem to use an in memory localization cache. That makes the tests way faster. wmf-quibble-core-vendor-mysql-php72-docker went from 11/12 minutes down to 8:30 minutes.

So tentatively that is fixed and even get improved! Well done.

Change 541624 had a related patch set uploaded (by Jforrester; owner: simetrical):
[mediawiki/core@REL1_34] Make LocalisationCache a service

https://gerrit.wikimedia.org/r/541624

Change 541624 merged by jenkins-bot:
[mediawiki/core@REL1_34] Make LocalisationCache a service

https://gerrit.wikimedia.org/r/541624