Page MenuHomePhabricator

VisualEditorDesktopArticleTargetInitModule::getMessages runs expensive checks for startup hashing
Closed, ResolvedPublic

Description

This method is evaluated during the startup hashing of all modules. This happens because part of the version hash, must be the message blob, and the message blob varies by two things: message keys, and language.

The preload logic for all 3000 modules on enwiki, currently spends about a 1/3rd of its time on this one module.

Screenshot 2019-04-17 at 22.13.45.png (988×2 px, 470 KB)

The list of message keys computed by this module class currently contains an expensive conditional based on local existence of a message (involving LCStore, MessageCache, and sometimes slave database reads).

If I understand correctly, the JavaScript code in this module also checks for existence via mw.message().exists(). As such, we could probably omit this and end up shipping the same data as otherwise, but with less computation cost.

Event Timeline

Change 504786 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] resourceloader: Omit non-existent messages in MessageBlobStore

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

Change 504788 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/extensions/VisualEditor@master] DesktopArticleTargetInitModule: Remove expensive $msg->exists check

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

[mediawiki/extensions/VisualEditor@master] DesktopArticleTargetInitModule: Remove expensive $msg->exists check
https://gerrit.wikimedia.org/r/504788

I've updated this to no longer need the mediawiki-core patch.

Change 504786 merged by jenkins-bot:
[mediawiki/core@master] resourceloader: Omit non-existent messages in MessageBlobStore

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

If I understand correctly, the JavaScript code in this module also checks for existence via mw.message().exists(). As such, we could probably omit this and end up shipping the same data as otherwise, but with less computation cost.

I would support removing the ->exists() check in PHP entirely and always listing all messages – but currently doing so would cause test failures in MW core in ResourcesTest::testMissingMessages(). I would be happy if that could be suppressed.

Change 504788 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] DesktopArticleTargetInitModule: Avoid expensive $msg->exists check

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