Page MenuHomePhabricator

[EPIC] Create a set of tests for defining a performance budget
Closed, ResolvedPublic

Description

The goals around this test have been documented at https://www.mediawiki.org/wiki/Performance_budgeting

Background

The web team enforces a performance budget test via a test in Vector skin:
https://github.com/wikimedia/mediawiki-skins-Vector/blob/master/tests/phpunit/structure/PerformanceBudgetTest.php

This has been successful at catching unintentional performance regressions to the main namespace (T350514), but has also thrown up false positives - particularly due to the fact that some projects run different extensions (e.g. Wikibase / FlaggedRevisions). Examples: T360102, T350338

It also runs on 3rd party extensions which may want to provide different performance budgets or not monitor performance at all (T358432#9578189).

After talking to @Jdforrester-WMF we think this can evolve into a new type of test

Requirements

  • The budget can be changed for different setups - for example projects with FlaggedRevisions should be able to define a slightly larger budget. [This is possible because each extension defines their own bundlesize.config.json file)
  • The code should live in core or its own dedicated extension.
  • The code should run on all Wikimedia-deployed extensions
  • Extension maintainers can choose to not track the exact size of bundles if they wish
  • It should not be possible to skip tests when the budget is exceeded - when a test fails the only allowable action should be to discuss increasing the budget or optimizing the code that triggered the failure.
  • Errors should only be triggered on patches that exceed the budget -we've had cases where skins/extensions have bypassed the budget, have been unable to merge code and caused CI issues in extensions such as GrowthExperiments and Charts where no changes to JS/CSS have been made (see discussions in T373017) [Note: CodexModules and SkinModule modules (to some extent) remain the exception to the rule here)
  • The test should only run for anonymous page views.
  • The test should prevent extensions adding large JS/CSS assets in Wikimedia production to the article namespace without conversation For example currently OOUI is not loaded on page load, but if it was we'd expect to see an associated large increase in bytes of CSS/JS shipped and would warrant discussion. This should happen during the code review process, as often once code is merged, it is often too late to rollback and can end up in production and in our HTML caches. We have seen this in two high profile incidents so far - Graphs (when fallback images were removed); and roll out of Phonos.
  • It should be possible to monitor the size of compressed bundles
  • It should be possible to monitor the size of uncompressed bundles, as this tends to contribute to visual completion and responsiveness

Descoped

The following requirements have not been requested so far, even though the test has been enabled for several months now. Feel free to create a ticket if these impact you and further work is needed in this field:

  • It should be possible to disable the test in certain setups e.g. SemanticMediaWiki should be able to disable the test
  • Different projects should be able to define different budgets for different circumstances - for example Wikidata should be able to define a performance budget for the item namespace (e.g. Q1)

Notes

From chat with @Jdforrester-WMF

  • The code could live in core
  • Code could use a bespoke Quibble-buster-performance-test
  • Could use fresnel

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
mediawiki/extensions/LanguageSelectorREL1_44+17 -0
mediawiki/extensions/TocTreemaster+17 -0
mediawiki/extensions/LanguageSelectormaster+17 -0
mediawiki/extensions/CategoryTreemaster+21 -0
mediawiki/coremaster+44 -19
mediawiki/extensions/Acrolinxmaster+17 -0
mediawiki/coremaster+0 -26
mediawiki/extensions/Wikibasemaster+48 -0
mediawiki/extensions/NavigationTimingmaster+20 -0
mediawiki/extensions/CheckUsermaster+17 -0
mediawiki/coremaster+2 -0
mediawiki/extensions/VisualEditormaster+14 -0
mediawiki/coremaster+2 -1
mediawiki/coremaster+1 -0
mediawiki/extensions/Echomaster+20 -0
mediawiki/coremaster+113 -1
mediawiki/extensions/WikiLambdamaster+6 -0
mediawiki/extensions/UniversalLanguageSelectormaster+4 -0
mediawiki/extensions/PopupsREL1_41+1 -612
mediawiki/extensions/Popupsmaster+1 -1 K
mediawiki/coremaster+136 -0
mediawiki/extensions/PopupsREL1_43+5 -7
mediawiki/extensions/Popupsmaster+5 -7
mediawiki/extensions/PopupsREL1_39+1 -612
mediawiki/extensions/PopupsREL1_42+3 -1 K
mediawiki/extensions/PopupsREL1_43+1 -1 K
mediawiki/extensions/Popupswmf/1.44.0-wmf.6+1 -1 K
mediawiki/extensions/Popupsmaster+18 -0
Show related patches Customize query in gerrit

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

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

[mediawiki/extensions/Popups@master] Monitor bundle size of module added on page load

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

Change #1073513 merged by jenkins-bot:

[mediawiki/extensions/Popups@master] Monitor bundle size of module added on page load

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

  • It should not be possible to skip tests when the budget is exceeded - when a test fails the only allowable action should be to discuss increasing the budget or optimizing the code that triggered the failure.

This is easily stated but it's hiding a lot of complexity:

  • Do teams who fully own an extension also fully own the performance budget? Can they increase the performance budget without discussing it with the wider developer community?
  • As the maintainer of extensions that hosts assets owned/maintained by other teams (EventLogging, WikimediaEvents), is Data Products responsible for enforcing the performance budget or are the owners/maintainers of those hosted assets?
  • What about an IC who has to touch an asset in MediaWiki Core that hasn't been touched in a year? And if the team that introduced the asset isn't the team that is actively maintaining it? And if the IC is a volunteer?

I suspect all of the above could be addressed with changes in team practices and publicly-available guidance. Are these (or something like them) available anywhere? I don't see them linked in the task description nor from the BundleSizeTest abstract base class.

Thanks for the thoughtful questions @phuedx !
After consulting with team members and @Jdforrester-WMF we've removed the problematic test in Vector to resolve T373017 and stop disrupting other teams.

Going forward, essentially we'd like to enforce budgets for any extensions that alter the experience of anonymous page views.
This prompted me to write out https://www.mediawiki.org/wiki/Performance_budgeting to answer your questions (see ownership section) but yes the expectation is to trust extension maintainers to do the right thing and to use documentation to share best practices and guidance. Would that suffice for now - if not perhaps we could work out what's missing from the talk page?

In terms of next steps:

  1. I'm going to continue to work with extensions modifying the core experience to add their own bundle size tests
  2. Post patch for new proposed test to MediaWiki core and gather feedback (https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1075663?usp=search)
  3. After the next MediaWiki release, we plan to roll out a test that requires extensions to publish their budgets if they modify the core experience (but does not test the overall bundle size)
  4. We then plan to think about this problem more holistically - for example thinking about how to monitor total bundle size and report big increases (likely possibly to docs.wikimedia.org) -for example we could visualize this on Special:Version.

Hopefully this satisfies a lot of the feedback we've been having about the existing test. Please let me know here if it doesn't or you have any unaddressed/new concerns.

@Jdlrobson and @Jdforrester-WMF this performance budget work is really great. Thank you for taking the lead.

Do you know how many teams are affected by this new requirement to own the performance budget? Are all the teams aware of this new effort?

Going forward, we'd like to raise awareness across extensions around the code we load and why we load.

If I am understanding the changes correctly, this is more than raising awareness, this is enforcing the ownership– or is this a necessary step in raising awareness and not actually enforcement?

Since this is not directly related to OKR work and not yet specifically defined essential work, this will be low priority while we work through higher priority work, but I see how valuable it is.

The following extensions and associated teams (growth, language, fundraising) have a budget in place:
https://codesearch.wmcloud.org/deployed/?q=.&files=%5Ebundlesize.config.json%24&excludeFiles=&repos=

Editing and data platform are the remaining teams that would be impacted by opting in and having conversations around it.

Awareness is the current goal per https://www.mediawiki.org/wiki/Performance_budgeting which should have all info you need (let me know if it doesnt answer all your questions).

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

[mediawiki/core@master] Enforce bundle size requirement for modules loaded on page load

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

Going forward, essentially we'd like to enforce budgets for any extensions that alter the experience of anonymous page views.

I'm surprised to hear about this decision. Whenever Editing was consulted we have unaninoumsly against the idea of a voting CI test for bundle size. Recently DST also expressed concerns about the noisiness of these tests, and how it disupts the Codex release process.

We have been consistent in advocating for an alternative solution, as anyone who has worked with the bundlesize test for a singnificnant amount of time has found it be more of a nuissnace than helpful.

Whenever Editing was consulted we have unaninoumsly against the idea of a voting CI test for bundle size.

You would still be able to do this. Per https://www.mediawiki.org/wiki/Performance_budgeting#Guidelines_around_picking_a_bundle_size you would be able to set null as the bundle size. The thing that would be changing however is that any modules added on article page load would need to be declared in that manifest (which I assume is going to be rare and only impact new features). To be clear the goals of this test are:

  1. Guard against new modules being accidentally added to article page load
  2. Make sure decisions around bundle size sit with individual teams.

Does that make sense?

Recently DST also expressed concerns about the noisiness of these tests, and how it disupts the Codex release process.

The DST situation was a lightly more complicated. Here the web team was monitoring bundle sizes for modules defined in Vector that were built by Codex, thus they changed every time the Codex release happened. When discussed, we agreed that the web team shouldn't be monitoring these bundles sizes and to remedy this, the design system team now owns those tests.

You would still be able to do this. Per https://www.mediawiki.org/wiki/Performance_budgeting#Guidelines_around_picking_a_bundle_size you would be able to set null as the bundle size.

This seems like a very odd way to define which modules are part of the initial page load. That information would surely be better stored in the ResourceLoader definition?

Also the way has been rolled out is that patches have been submitted to each team with hard-coded limits, so in practice we now have a dozen new extensions with enforced bundlesizes.

It is claimed in the task a voting test is required because merged code would get deployed before we noticed the regression, however adequate monitoring should also allow us to revert problematic patches before deployment.

I would also hope that once adequate monitoring is in place we can remove the sizes hard-coded in bundlesize, and instead compute them dynamically.

It is claimed in the task a voting test is required because merged code would get deployed before we noticed the regression, however adequate monitoring should also allow us to revert problematic patches before deployment.
I would also hope that once adequate monitoring is in place we can remove the sizes hard-coded in bundlesize, and instead compute them dynamically.

Sorry this task is a little outdated. If you look at https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1075663 we are moving towards a more monitoring like approach (I was hoping for some pointers on that particular bit). For now the bundlesize is indeed being computed dynamically separately from the bundlesize.config.json and is non-blocking. The enforced bundlesizes are an optional artifact of this.

This seems like a very odd way to define which modules are part of the initial page load. That information would surely be better stored in the ResourceLoader definition?

I think it would make sense to move towards defining them in ResourceLoader itself on the long run if this proves a useful feature.

For now I am using the bundlesize.config.json as it was an existing construct, doesn't touch production code (so less risky) and provides a cheap way to track this with the added benefit that teams have an increased awareness of their performance footprint. In most cases the tracked modules are not actively being developed on so should not be causing much friction.

This seemed preferable to maintaining this list inside MediaWiki core as I want to make sure that we avoid situations where a bundle size change in one repo requires a modification in another repo (e.g. how it worked previously with Codex releases).

My hope is that if this experiment doesn't work out, that this is easy to undo (e.g. not subject to PHP stable policy).

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

[mediawiki/core@master] Tests: Enforce requirement that modules loaded on page load are documented

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

Change #1104596 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/extensions/Popups@master] Kick bundlesize out of package.json

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

Change #1104597 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/extensions/Popups@master] Clean up bundlesize config

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

Change #1104596 merged by jenkins-bot:

[mediawiki/extensions/Popups@master] Kick bundlesize out of package.json

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

Change #1104629 had a related patch set uploaded (by Michael Große; author: Lucas Werkmeister (WMDE)):

[mediawiki/extensions/Popups@wmf/1.44.0-wmf.6] Kick bundlesize out of package.json

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

Change #1104629 merged by jenkins-bot:

[mediawiki/extensions/Popups@wmf/1.44.0-wmf.6] Kick bundlesize out of package.json

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

Mentioned in SAL (#wikimedia-operations) [2024-12-16T15:04:23Z] <ladsgroup@deploy2002> Started scap sync-world: Backport for [[gerrit:1104629|Kick bundlesize out of package.json (T382192 T360590)]], [[gerrit:1104633|fix(surfacing): Show highlights in lists as well (T381841)]], [[gerrit:1104624|stats(surfacing): track link recommendation api recommendations (T378536)]]

Mentioned in SAL (#wikimedia-operations) [2024-12-16T15:09:08Z] <ladsgroup@deploy2002> migr, ladsgroup: Backport for [[gerrit:1104629|Kick bundlesize out of package.json (T382192 T360590)]], [[gerrit:1104633|fix(surfacing): Show highlights in lists as well (T381841)]], [[gerrit:1104624|stats(surfacing): track link recommendation api recommendations (T378536)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

Mentioned in SAL (#wikimedia-operations) [2024-12-16T15:15:54Z] <ladsgroup@deploy2002> Finished scap sync-world: Backport for [[gerrit:1104629|Kick bundlesize out of package.json (T382192 T360590)]], [[gerrit:1104633|fix(surfacing): Show highlights in lists as well (T381841)]], [[gerrit:1104624|stats(surfacing): track link recommendation api recommendations (T378536)]] (duration: 11m 30s)

Change #1104673 had a related patch set uploaded (by Michael Große; author: Lucas Werkmeister (WMDE)):

[mediawiki/extensions/Popups@REL1_41] Kick bundlesize out of package.json

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

Change #1104675 had a related patch set uploaded (by Michael Große; author: Lucas Werkmeister (WMDE)):

[mediawiki/extensions/Popups@REL1_42] Kick bundlesize out of package.json

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

Change #1104676 had a related patch set uploaded (by Michael Große; author: Lucas Werkmeister (WMDE)):

[mediawiki/extensions/Popups@REL1_43] Kick bundlesize out of package.json

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

Change #1104673 merged by jenkins-bot:

[mediawiki/extensions/Popups@REL1_41] Kick bundlesize out of package.json

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

Change #1104679 had a related patch set uploaded (by Jforrester; author: Lucas Werkmeister (WMDE)):

[mediawiki/extensions/Popups@REL1_39] Kick bundlesize out of package.json

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

Change #1104676 merged by jenkins-bot:

[mediawiki/extensions/Popups@REL1_43] Kick bundlesize out of package.json

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

Change #1104675 merged by jenkins-bot:

[mediawiki/extensions/Popups@REL1_42] Kick bundlesize out of package.json

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

Change #1104679 merged by jenkins-bot:

[mediawiki/extensions/Popups@REL1_39] Kick bundlesize out of package.json

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

Change #1104597 merged by jenkins-bot:

[mediawiki/extensions/Popups@master] Clean up bundlesize config

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

Change #1104972 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/extensions/Popups@REL1_43] Clean up bundlesize config

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

Change #1104972 merged by jenkins-bot:

[mediawiki/extensions/Popups@REL1_43] Clean up bundlesize config

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

Change #1075663 merged by jenkins-bot:

[mediawiki/core@master] Tests: Monitor total bundle size of article views

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

Change #1105005 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/extensions/UniversalLanguageSelector@master] bundlesize.config.json: Declare ext.uls.pt for wikis where it's loaded in articles, like Wikifunctions

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

Change #1105006 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/extensions/WikiLambda@master] bundlesize.config.json: Declare ext.wikilambda.languageselector, as it's loaded on page paint

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

Change #1105005 merged by jenkins-bot:

[mediawiki/extensions/UniversalLanguageSelector@master] bundlesize.config.json: Declare ext.uls.pt for wikis where it's loaded in articles, like Wikifunctions

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

Change #1105006 merged by jenkins-bot:

[mediawiki/extensions/WikiLambda@master] bundlesize.config.json: Declare ext.wikilambda.languageselector, as it's loaded on page paint

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

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

[mediawiki/extensions/Wikibase@master] WIP: Declare bundlesizes for modules that are loaded on article views

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

Change #1092892 merged by jenkins-bot:

[mediawiki/core@master] Tests: Enforce requirement that modules loaded on page load are documented

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

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

[mediawiki/extensions/Echo@master] Echo declares modules added on page load

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

Change #1108828 merged by jenkins-bot:

[mediawiki/extensions/Echo@master] Echo declares modules added on page load

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

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

[mediawiki/extensions/CheckUser@master] CheckUser must declare modules that are loaded on page load

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

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

[mediawiki/extensions/VisualEditor@master] VisualEditor must declare modules that are loaded on page load

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

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

[mediawiki/extensions/NavigationTiming@master] Track bundlesize of module loaded on article page load

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

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

[mediawiki/core@master] Tests: Remove hardcoded exclusion list from PerformanceBudgetTest

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

Change #1109042 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/core@master] tests: Add ext.wikimediaBadges to bundlesize exclusions

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

Change #1109042 merged by jenkins-bot:

[mediawiki/core@master] tests: Add ext.wikimediaBadges to bundlesize exclusions

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

Change #1109065 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/core@master] tests: Add WikimediaEvents modules to bundlesize exclusions

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

Change #1109066 had a related patch set uploaded (by Michael Große; author: Michael Große):

[mediawiki/core@master] tests: Add ext.wikimediaEvents.wikibase to bundlesize exclusions

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

Change #1109066 abandoned by Michael Große:

[mediawiki/core@master] tests: Add ext.wikimediaEvents.wikibase to bundlesize exclusions

Reason:

in favor of I2f1996c4e931bcac47e2aab71c1876e8c9e4cde7

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

Change #1109065 merged by jenkins-bot:

[mediawiki/core@master] tests: Add WikimediaEvents modules to bundlesize exclusions

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

Change #1108842 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] VisualEditor must declare modules that are loaded on page load

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

Change #1108821 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@master] CheckUser must declare modules that are loaded on page load

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

Change #1108824 merged by jenkins-bot:

[mediawiki/extensions/NavigationTiming@master] Track bundlesize of module loaded on article page load

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

Change #1108519 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] Declare modules that are loaded on article views

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

Change #1108820 merged by jenkins-bot:

[mediawiki/core@master] Tests: Remove hardcoded exclusion list from PerformanceBudgetTest

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

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

[mediawiki/core@master] PerformanceBudgetTest: Support testing uncompressed as well as compressed

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

Change #1113072 had a related patch set uploaded (by Fomafix; author: Fomafix):

[mediawiki/extensions/Acrolinx@master] Declare module that extension loads on article page load

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

Change #1113072 merged by jenkins-bot:

[mediawiki/extensions/Acrolinx@master] Declare module that extension loads on article page load

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

Change #1109780 merged by jenkins-bot:

[mediawiki/core@master] PerformanceBudgetTest: Support testing uncompressed as well as compressed

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

Jdlrobson-WMF updated the task description. (Show Details)

Change #1151297 had a related patch set uploaded (by Fomafix; author: Fomafix):

[mediawiki/extensions/CategoryTree@master] Declare modules that are loaded on article views

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

Change #1151307 had a related patch set uploaded (by Fomafix; author: Fomafix):

[mediawiki/extensions/LanguageSelector@master] Declare modules that are loaded on article views

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

Change #1151297 merged by jenkins-bot:

[mediawiki/extensions/CategoryTree@master] Declare modules that are loaded on article views

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

Change #1151307 merged by jenkins-bot:

[mediawiki/extensions/LanguageSelector@master] Declare modules that are loaded on article views

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

Change #1153992 had a related patch set uploaded (by Fomafix; author: Fomafix):

[mediawiki/extensions/TocTree@master] Declare modules that are loaded on article views

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

Change #1170329 had a related patch set uploaded (by Jforrester; author: Fomafix):

[mediawiki/extensions/LanguageSelector@REL1_44] Declare modules that are loaded on article views

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

Change #1170329 merged by jenkins-bot:

[mediawiki/extensions/LanguageSelector@REL1_44] Declare modules that are loaded on article views

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