Page MenuHomePhabricator

Add bandwidth tests for JavaScript and CSS to Vector and component repo
Open, MediumPublic

Description

Total JavaScript and style bytes shipped to clients is an important metric to gauge with each patch so that significant changes are known. This will be an especially important metric to track prior to the development of client features for the Vue.js search iterations as well as any other change.

bundlesize

bundlesize has proven to be an invaluable and well-known tool for these measurements in Popups, MobileFrontend and Portals but require bundling. If we have bundling, maybe it's a nice file system test to have. If we don't have bundling, skip the below acceptance criteria.

Note: This has been done for Minerva. Learn from it generalising and following conventions where ever possible. It is also now possible to store this configuration data in a separate file (e.g., bundlesize.config.json, package.json).

Acceptance criteria (skip if bundlesize is unused)

  • bundlesize is added an NPM dependency to Vector and mw-vue-components.
  • A bundlesize configuration file specifies the current sizing of each JavaScript ResourceLoader module / chunk in Vector and mw-vue-components.
  • A bundlesize configuration file specifies the current sizing of each CSS ResourceLoader module / chunk to protect spikes in first paint in Vector and mw-vue-components.
  • npm test calls test:size which validates that the JavaScript does not exceed the bundlesize specified in the config in Vector and mw-vue-components.
  • A patch that exceeds the maximum expected bundlesize fails jenkins-bot's tests (votes -1).
  • Bundlesize reports are recorded in docs/bundleSizeMinGzip.txt (e.g: npm -s run test:size|grep dist > docs/minGzipBundleSize.txt) in Vector and mw-vue-components.

Quibble/CI

Use the CI's MediaWiki development environment to test the size of each module:

  • skins.vector.styles.legacy: this should probably be carefully guarded. We don't want to increase the size of the legacy skin.
  • skins.vector.styles: more leeway than legacy but should still be cautious.
  • skins.vector.js: also a good dependency to be cautious on. This will help inform our loading strategy for larger dependencies like Vue.js itself.
  • Any dynamic modules or module dependencies requested by the client. For example, if the Vue.js module is requested, let's consider it as an input into Vector's footprint.

It will be on us to remember to add new modules as they're added.

Per T244276#6029796:

Inside Quibble/CI and most mw-dev environments there are MW_* environment variables available. These are used by Selenium and QUnit jobs for example, which means if you're running either of those from the command-line locally today, you have them set up (e.g. saved in your bashrc file, or automatically by Docker/Vagrant).

A few lines of bash or Node code to make a request to load.php would get you there. For example:

#!/bin/bash -eu

curl -fsS "$MW_SERVER/$MW_SCRIPT_PATH/load.php?lang=en&modules=skins.vector.styles.legacy&only=styles" > /tmp/output.css
wc -c /tmp/output.css
#> 33745 bytes
gzip -9 -c /tmp/output.css | wc -c
#> 7511 bytes

^I think JavaScript would be more accessible, less error-prone, more scalable, and more portable for most web developers, and dependencies could leverage NPM. Further, we could share useful scripts via NPM if we wanted to. Maybe we can use node-fetch or some existing package for HTTP statistics.

Acceptance criteria

  • Tests run in CI but not on precommit.
  • The listed modules are tested.
  • Modules and their expected sizes are easily extendable separate from the code.
  • Modules have some tolerance.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 4 2020, 7:12 PM
Jdlrobson renamed this task from Add gzipped minified bundle size test to Add gzipped minified bundle size test to Vector.Feb 9 2020, 10:58 PM
Jdlrobson added a project: Vector.
Jdlrobson moved this task from Incoming to Needs Prioritization on the Readers-Web-Backlog board.
ovasileva triaged this task as Medium priority.Feb 20 2020, 1:28 PM
Jdlrobson updated the task description. (Show Details)Feb 24 2020, 4:52 PM
Restricted Application added a subscriber: Masumrezarock100. · View Herald TranscriptFeb 24 2020, 4:52 PM
Jdlrobson added a subscriber: Jdlrobson.

We talked about this in standup and attempted to estimate.

We decided we'd prefer a solution that uses ResourceLoader URIs to check bundlesizes rather than builds assets itself using lessc (as we did in Minerva)

We'd like to investigate how possible that is before estimating.

From my perspective, given the tightly coupled Vue.js integration, it would be best if this task were completed before the search experience work starts.

We decided we'd prefer a solution that uses ResourceLoader URIs to check bundlesizes rather than builds assets itself using lessc (as we did in Minerva)

I missed the discussion last week but I think using Webpack for the JavaScript bundlesize tests would be useful both for the purposes of measuring performance as well as all the other functionality it provides, notably giving us the flexibility to compile _and_ minify the Vue.js + search JavaScript bundle.

I am still not convinced that Vue code will live inside Vector (it may live in core). If we want to add webpack as part of this change I would strongly suggest punting this until work on search begins. Setting up webpack is a task in itself.

If we want to use ResourceLoader URIs we'll need to make sure an environment variable exists pointing to a mediawiki instance and that environment variable will need to be setup manually as it can vary between users (I use localhost:8888 for example). It looks like wmf-quibble-core-vendor-mysql-php72-docker has one but you can't add jobs to that like you can in npm test AFAICS.

With these two things in mind I suggest either taking the simple tried and tested Minerva approach of lessc + bundlesize or punting this task until later. Moving to upcoming to have that conversation.

Per T205129#4671988, mw.inspect() may not be the best evaluation for these metrics. Maybe this is something Fresnel can help with though. /cc @Jdrewniak

This task was originally supposed to evaluate JavaScript sizing differences for the search experience _and_ any of its dependencies (e.g., the Vue.js compiler or a component library), but there are other important metrics to measure such as styling size and rendering performance. The hope was to avoid unexpected changes in performance, most especially for enterprising Vue.js work. I think perspectives from multiple tools would help us have higher confidence in the changes we make.

Maybe the Performance-Team has some insights. We're going to be developing a new search experience in Vector using Vue.js at some point, possibly soon. Do you have any recommendations on how best to measure the performance impact of each patch? How should we minimize risk for new patches from multiple contributors on and off the Web team?

I was thinking it would be nice to at least understand the JavaScript, template, styles, Vue.js compiler, and any dependencies. Comparing different loading strategies like static ResourceLoader module dependency vs deferring loading after initial render would also be valuable. Do you consider Fresnel adequate or should we explore additional options?

Jdlrobson renamed this task from Add gzipped minified bundle size test to Vector to Add gzipped minified bundle size test to Vector for JavaScript AND CSS.Mar 27 2020, 5:22 PM
Jdlrobson updated the task description. (Show Details)
Niedzielski renamed this task from Add gzipped minified bundle size test to Vector for JavaScript AND CSS to Add gzipped minified bundle size test to Vector for JavaScript and CSS.Apr 3 2020, 2:51 AM

Inside Quibble/CI and most mw-dev environments there are MW_* environment variables available. These are used by Selenium and QUnit jobs for example, which means if you're running either of those from the command-line locally today, you have them set up (e.g. saved in your bashrc file, or automatically by Docker/Vagrant).

A few lines of bash or Node code to make a request to load.php would get you there. For example:

#!/bin/bash -eu

curl -fsS "$MW_SERVER/$MW_SCRIPT_PATH/load.php?lang=en&modules=skins.vector.styles.legacy&only=styles" > /tmp/output.css
wc -c /tmp/output.css
#> 33745 bytes
gzip -9 -c /tmp/output.css | wc -c
#> 7511 bytes
Niedzielski renamed this task from Add gzipped minified bundle size test to Vector for JavaScript and CSS to Add gzipped minified bundle size test to Vector for JavaScript and CSS for Vue.js search.Apr 5 2020, 4:22 PM
Niedzielski renamed this task from Add gzipped minified bundle size test to Vector for JavaScript and CSS for Vue.js search to Add bandwidth tests to Vector for JavaScript and CSS for Vue.js search.Apr 6 2020, 3:00 PM
Niedzielski updated the task description. (Show Details)
Restricted Application added a subscriber: Liuxinyu970226. · View Herald TranscriptApr 6 2020, 3:00 PM

Change 586413 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/Vector@master] Check and enforce bundlesizes in Vector

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

Thanks for the protip @Krinkle I hadn't realised those globals were available here.

@Krinkle I'm seeing "curl: (3) <url> malformed" which suggests those globals are not available (works fine locally). Are you sure they are available in npm test?

Krinkle added a comment.EditedApr 6 2020, 7:14 PM

@Krinkle I'm seeing "curl: (3) <url> malformed" which suggests those globals are not available (works fine locally). Are you sure they are available in npm test?

No, the basic Node.js pipeline just clones the repo and runs npm test. It's not that the globals are missing, the MediaWiki install command is skipped there entirely. As a quick hack you can try adding it to packages.scripts.selenium or (if already set) to packages.scripts.postselenium - to confirm that it does indeed work.

Hacks aside, you'll want to add this npm run command to the quibble job for the Vector repo. If the list of commands Quibble ran was maintained in a simple .ci.yaml file that would be easy to do, but that is not yet the case. I can take look but I don't know off-hand where to add it. RelEng would know best. It's probably somewhere in integration/config.

Krinkle moved this task from Inbox to Radar on the Performance-Team board.Apr 6 2020, 7:54 PM
Krinkle edited projects, added Performance-Team (Radar); removed Performance-Team.
Krinkle moved this task from Limbo to Watching on the Performance-Team (Radar) board.
Niedzielski updated the task description. (Show Details)Apr 7 2020, 1:47 PM

Change 586413 abandoned by Jdlrobson:
Check and enforce bundlesizes in Vector

Reason:
My goal here was to determine if the idea works in npm test (which it doesnt).

I'll abandon this to be clearer that I'm not planning to work on this further. We can talk about writing in Node.js during estimation but I suspect we will be limited by a generic solution in the CI pipeline that lives outside this repo.

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

Niedzielski removed ovasileva as the assignee of this task.Apr 7 2020, 3:36 PM
Niedzielski added a subscriber: ovasileva.

Removing @ovasileva as assignee as this task has been prioritized (see T244276#5898385).

Can we revise the title of this so it's not linked to Vue.js search? We need this for everything else too :)

Niedzielski renamed this task from Add bandwidth tests to Vector for JavaScript and CSS for Vue.js search to Add bandwidth tests for JavaScript and CSS to Vector.Apr 17 2020, 12:06 AM
Niedzielski updated the task description. (Show Details)
Niedzielski renamed this task from Add bandwidth tests for JavaScript and CSS to Vector to Add bandwidth tests for JavaScript and CSS to Vector and component repo.Wed, May 6, 2:57 PM
Jdlrobson moved this task from Backlog to Technical on the Vector board.Fri, May 15, 5:00 PM
Niedzielski updated the task description. (Show Details)Thu, May 21, 7:29 PM
Niedzielski updated the task description. (Show Details)Thu, May 21, 7:31 PM
Niedzielski updated the task description. (Show Details)