Page MenuHomePhabricator

PerformanceBudgetTest should include module dependencies
Closed, ResolvedPublicFeature

Description

Feature summary (what you would like to be able to do and where):
Currently we require all JavaScript on page load to declare itself however currently there is a loophole which is that you can pull in libraries as dependencies.
Since there are some expensive libraries we want to avoid loading on page load - such as Codex and OOUI I think it's important we also check dependencies. This will help guide developers better so they use best practices on https://wikitech.wikimedia.org/wiki/MediaWiki_Engineering/Guides/Frontend_performance_practices

Use case(s) (list the steps that you performed to discover that problem, and describe the actual underlying problem which you want to solve. Do not describe only a solution):

  • We'll check module dependencies as well as the modules directly added to page

Benefits (why should this be implemented?):

  • Less chance of performance issues due to accidental adds of OOUI / Codex.

Event Timeline

Change #1152302 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@master] Declare modules loaded on page as dependencies

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

Change #1152303 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/extensions/EventLogging@master] Declare modules loaded on page as dependencies

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

Change #1152304 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] Tests: Also check module dependencies that are loaded on page

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

Change #1152369 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/extensions/VisualEditor@master] Declare the ResourceLoader modules permitted on page load

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

Change #1152303 merged by jenkins-bot:

[mediawiki/extensions/EventLogging@master] tests: Check size of ext.wikimedia.metricsPlatform

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

Change #1152302 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Declare modules loaded on page as dependencies

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

Change #1153718 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] Drop wikibase.client.data-bridge.init workaround

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

Change #1154051 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/extensions/Wikibase@master] POC: Delay OOUI load until dispatch event

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

Change #1153718 abandoned by Jdlrobson:

[mediawiki/core@master] Drop wikibase.client.data-bridge.init workaround

Reason:

I will circle back to this when I return from my break.

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

Change #1154051 abandoned by Jdlrobson:

[mediawiki/extensions/Wikibase@master] POC: Delay OOUI load until dispatch event

Reason:

I will circle back to this when I return from my break. I encourage you to take a look ASAP given this seems like quite a serious and unnecessary performance problem on Wikidata.org.

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

Change #1152369 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Declare the ResourceLoader modules permitted on anonymous page loads

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

Change #1153718 restored by Jdlrobson:

[mediawiki/core@master] Drop wikibase.client.data-bridge.init workaround

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

Change #1182679 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/extensions/WikimediaEvents@master] ext.wikimediaEvents.xLab is now being loaded on page load

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

Change #1182679 merged by jenkins-bot:

[mediawiki/extensions/WikimediaEvents@master] ext.wikimediaEvents.xLab is now being loaded on page load

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

Change #1152304 merged by jenkins-bot:

[mediawiki/core@master] Tests: Also check module dependencies that are loaded on page

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

Change #1184626 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] Tests: Also check module dependencies that are loaded on page"

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

Change #1184626 abandoned by Jdlrobson:

[mediawiki/core@master] Tests: Also check module dependencies that are loaded on page"

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

Change #1184627 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] Tests: Also check module dependencies that are loaded on page"

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

Change #1184832 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/extensions/CentralNotice@master] Ignore dependencies when evaluating bundle

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

Change #1184832 merged by jenkins-bot:

[mediawiki/extensions/CentralNotice@master] bundlesize: Ignore dependencies when evaluating bundle

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

Change #1184627 merged by jenkins-bot:

[mediawiki/core@master] Tests: Also check module dependencies that are loaded on page

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

Thanks all. I've just sent an e-mail to wikitech-l. I'm monitoring CI. I'll be off shortly for the night but will jump on any issues that have surfaced tomorrow morning.

This is causing https://gerrit.wikimedia.org/r/c/mediawiki/extensions/CookieConsent/+/1187366/8 to fail with:

09:37:02 There was 1 error:
09:37:02 
09:37:02 1) PerformanceBudgetTest::testTotalModulesSize
09:37:02 Undefined array key "mediawiki.widgets.styles"
09:37:02 
09:37:02 /workspace/src/tests/phpunit/structure/PerformanceBudgetTest.php:187
09:37:02 /workspace/src/tests/phpunit/structure/PerformanceBudgetTest.php:282
09:37:02

FIXED

And I got another one that fails https://gerrit.wikimedia.org/r/c/mediawiki/extensions/BlueSpiceFoundation/+/1187441:

There was 1 failure:

1) PerformanceBudgetTest::testTotalModulesSize
⚠️ PLEASE DO NOT SKIP THIS TEST ⚠️

If this is blocking a merge this might signal a potential performance regression with the desktop site.

All extensions/skins adding code to page load for an article must monitor their ResourceLoader modules.

Read https://www.mediawiki.org/wiki/Performance_budgeting for guidance on how to suppress this error message.

The following modules have not declared budgets:

mwstake.component.alertbanners (loaded by ext.bluespice )

Failed asserting that actual size 1 matches expected size 0.

/workspace/src/tests/phpunit/structure/PerformanceBudgetTest.php:222
/workspace/src/tests/phpunit/structure/PerformanceBudgetTest.php:283

And I have absolutely no idea how I can act on that note that this change in particular is the third layers of deepness from my original task and I can not decently jump deeper and figure out what configuration is required for PerformanceBudget :] EDIT: the doc is mentioned in the wikitech-l email and is on wiki at https://www.mediawiki.org/wiki/Performance_budgeting

Change #1187461 had a related patch set uploaded (by Hashar; author: Hashar):

[mediawiki/extensions/BlueSpiceFoundation@master] Ignore RL budget size of mwstake.component.alertbanners

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

Change #1187461 merged by Hashar:

[mediawiki/extensions/BlueSpiceFoundation@master] Ignore RL budget size of mwstake.component.alertbanners

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

@hashar it seems to depend on a module in the archived extension https://gerrit.wikimedia.org/r/q/project:mediawiki/extensions/Workflow so I can't patch Workflow with the necessary code. Is this an intentional dependency?

ContentTranslation extension is affected as well

The following modules have not declared budgets:

ext.centralauth.ForeignApi (loaded by mediawiki.ForeignApi,mw.cx.SiteMapper,ext.cx.uls.quick.actions )

ContentTranslation extension is affected as well

The following modules have not declared budgets:

ext.centralauth.ForeignApi (loaded by mediawiki.ForeignApi,mw.cx.SiteMapper,ext.cx.uls.quick.actions )

Thanks for fixing - it is now tracked as T404463: ForeignAPI should not be loaded on page load by mw.cx.SiteMapper and ext.cx.uls.quick.actions

Jdlrobson-WMF claimed this task.