[SPIKE] Collecting performance information #3239
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
request-checks: true
The code is very dirty and just a buldozzed way to get through to what needed displaying to check feasability
Adds a new `/stats` route that'll point to the `public` folder of the `stats` project. The route uses `express.static` to serve the files themselves, and `serve-index` to provide a folder navigation.
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-3239
DistFile sizes represent minified code.
ModulesFile sizes represent non-minified code. all.mjs: 136.94 KiB, 32 modules
all.mjs (
|
| Module Path | Size | Percentage |
|---|---|---|
| package/govuk-esm/common/index.mjs | 4.92 KiB | 7.28% |
| package/govuk-esm/common/normalise-dataset.mjs | 1.29 KiB | 1.91% |
| package/govuk-esm/i18n.mjs | 13.75 KiB | 20.35% |
| package/govuk-esm/vendor/polyfills/Object/defineProperty.mjs | 2.96 KiB | 4.38% |
| package/govuk-esm/vendor/polyfills/Function/prototype/bind.mjs | 8.06 KiB | 11.93% |
| package/govuk-esm/vendor/polyfills/DOMTokenList.mjs | 9.14 KiB | 13.52% |
| package/govuk-esm/vendor/polyfills/Document.mjs | 1.17 KiB | 1.74% |
| package/govuk-esm/vendor/polyfills/Element.mjs | 3.1 KiB | 4.59% |
| package/govuk-esm/vendor/polyfills/Element/prototype/classList.mjs | 3.48 KiB | 5.15% |
| package/govuk-esm/vendor/polyfills/Element/prototype/dataset.mjs | 2.53 KiB | 3.74% |
| package/govuk-esm/vendor/polyfills/String/prototype/trim.mjs | 712 B | 1.03% |
| package/govuk-esm/components/accordion/accordion.mjs | 16.48 KiB | 24.38% |
components/accordion/accordion.mjs: 67.59 KiB, 13 modules
| Module Path | Size | Percentage |
|---|---|---|
| package/govuk-esm/common/index.mjs | 4.92 KiB | 7.28% |
| package/govuk-esm/common/normalise-dataset.mjs | 1.29 KiB | 1.91% |
| package/govuk-esm/i18n.mjs | 13.75 KiB | 20.35% |
| package/govuk-esm/vendor/polyfills/Object/defineProperty.mjs | 2.96 KiB | 4.38% |
| package/govuk-esm/vendor/polyfills/Function/prototype/bind.mjs | 8.06 KiB | 11.93% |
| package/govuk-esm/vendor/polyfills/DOMTokenList.mjs | 9.14 KiB | 13.52% |
| package/govuk-esm/vendor/polyfills/Document.mjs | 1.17 KiB | 1.74% |
| package/govuk-esm/vendor/polyfills/Element.mjs | 3.1 KiB | 4.59% |
| package/govuk-esm/vendor/polyfills/Element/prototype/classList.mjs | 3.48 KiB | 5.15% |
| package/govuk-esm/vendor/polyfills/Element/prototype/dataset.mjs | 2.53 KiB | 3.74% |
| package/govuk-esm/vendor/polyfills/String/prototype/trim.mjs | 712 B | 1.03% |
| package/govuk-esm/components/accordion/accordion.mjs | 16.48 KiB | 24.38% |
Interactive visualisation can be downloaded from the build's artifacts
7 tasks
This was referenced Feb 2, 2023
|
This looks great! A few thoughts from me:
|
|
It'd be good to come up with next steps as an output of this spike. And especially good if we could split the work up so folks can work on different bits. I imagine we can at least split the file size stuff from the rollup stuff. Maybe also split the comment generation into its own bit of work. |
|
This is ace! A few thoughts from me too, in no particular order:
+1 – let's make sure we do this. @romaricpascal do you think you'd be able to draft some issues, or would it be useful to find some time to do them together? |
|
@domoscargin @36degrees Cheers both for the comments 😊
I was thinking of drafting a list of steps and posting them on Slack for discussion before creating actual issues. |
|
Spike is complete and next steps are listed in #3279 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR explores how we may collect stats about the size of our built files, as well as different ways we can access these stats. It looks at two sets of files:
dist, that'll likely be loaded in a browser straight awaypackage, likely to be consumed by a bundler to build the final JS appCollecting the stats
fs.statUse Node to get the size of the file on disk using
fs.stat.This is the method used for assessing the files in
distas each Kb in these files is a Kb the browser will load.Getting more information like the number of modules in the bundles didn't feel useful for the files in
distas they bundle the entirety of the library anyway.rollup-plugin-visualizer
rollup-plugin-visualizer is a plugin that collects statistics during a Rollup build and outputs them, either as interactive HTML visualisations or raw data (YAML or JSON).
This method is used for scrutinising the ES Modules. It allows us to get a more information than the file on disk. By bundling a file that
exports the module we want information on, it can let us know which other modules will get included, as well as how much each will contribute (accounting for tree-shaking). It also accounts for tree-shaking, when only part of a module would end up in the final bundle.Unlike
fs.statswhich looks at the minified size on disk, this looks at the unminified code (even with@rollup/plugin-terserin the build).Accessing the stats
The PR updates the
testworkflow to add a stat collection job upon PR. It also extracts the steps that were buildingdistandpackageto their own jobs as those are pre-requisite to collecting the stats.On the PR as comment
To get quick access to the information, the workflow adds a comment on the PR with the information it collected. It'll reuse the same comment if new pushes are made on the PR (using specific content in the comment to identify it)
For the modules, we can extract quite a lot of information from the raw data exported by
rollup-plugin-visualizerand format it however we want. We'll likely want to use a few<details>element to not have that comment be too overwhelming. We'll also have to keep in mind the 65536 characters limit for comments on Github.Downloadable archive
Besides the raw data we can format on the PR comment,
rollup-plugin-visualizeralso creates HTML visualisations. Those are zipped and attached to the Github action's artifacts so they can be downloaded.To speed up access, the PR also adds a link in the comment.
### Visualisation in the review app
The PR also adds access to the visualisations in the review app. It adds the stats collection to the Procfile so it runs before startup and serves them under
/stats.A link to the stats is added to the PR comment as well (and
serve-indexprovides a cheap way to browse the files).How it works
The work for collecting the ES module stats and commenting on Github is isolated in its own npm workspace, inside the
statsfolder.The collection of file sizes for the
distfolder happens in the script doing the comment itself, as it's onlyfs.statscalls. We could look to extract it in its own function come the time to properly implement things.ES modules stats collection
You can run the stats collection with
npm start --workspace statsfrom the root of the project.The stats end up in
stats/public, with naming conventions based on the file being looked at and what was asked to be exported.rollup.config.jscontrols the Rollup build that'll output the statistics. It creates one entry to build per item in thestats.config.jsfile.Those entries are purely virtual, not files on disk, (thanks to
@rollup/plugin-virtual) andexporteither the whole module under scrutiny or specific exports from that module, so we can check tree-shaking.Rollup outputs the built file to stdout. It's a bit verbose in the console, so we could either ignore it (
> /dev/null) or could build into thetmpfolder at the root of the project.Stats serving in the review app
The review app adds a new middleware for
/statsthat serves the files themselves usingexpress-staticand usesserve-indexto let us navigate between the different files exported.CI workflow
The CI workflow, running after
distandpackageare built:The commenting of the PR is split between:
comment-on-pr.js, the entry point exporting the function run by the GitHub Script action. It's mostly code for interacting with Github API, delegating the bulk of rendering the comment togenerateStatsMessage()generate-stats-message.jscontains the code generating the comment with the stats.lib/rollup-plugin-visualiser-stats.jscontains the logic parsing the raw data from rollup-plugin-visualiser and aggregating it in a way that suits usThoughts and questions
distandpackageare re-built beforehand.distandpackage. For the former, they're the actual size on disc. For the later, the size of the source. Not a bad thing, especially as they're relevant to what happens to each kind of files, but something to keep in mind.rollup-plugin-visualizerwill integrate with our build once we move to Rollup 3, so we could collect stats without making 1 build per file we want to look at. Making a new build is closer to how our files will be consumed though.rollup-plugin-visualizerinclude the weight of the comments. In a way this makes sense as there's no guarantee that a bundler loading our file will have a minifier stripping comments, so these comments would contribute to the bundle size after all.rollup-plugin-visualiseris not as well specified as that of Webpack. The project is also a much smaller scale. This may explain why many stats project rely on Webpack 🤔Kind of tangential thought as well (and quite a heavy piece of work). We have 3 ways to build the project (
build:compile,build:dist,build:package). This means the build runs 3 times on CI.Could things be split up differently so we build only once, and then gather the results of the build in whichever way we want to ship them (ie. a "build" and then a "packaging" step?