Improve browser TBT of merge request overview tab
What does this MR do?
For #331172 (closed)
During the investigation phase of my work on this issue, I found that, by far the longest blocking time was spent in Webpack, loading modules.
Loading some modules via Webpack was blocking for - on average - around 6000ms and sometimes resulting in the browser dropping frames on the scale of 2-5 seconds at a time.
Because we bundle all of our modules together, this was time mostly spent in eval
.
Fix
The correct fix is to loosen our reliance on Webpack and leverage asynchronous module loading in the browser via HTTP/2, import()
, and other features of modern JS instead of bundling (and chunking) the FE application into giant strings that are eval
ed by Webpack.
However, even starting down that path is a huge undertaking.
Patch
So, a patch is necessary.
In this case, the patch is: Eliminate as many modules from loading as we can.
On the Overview ("Discussions") tab of the MR page, the entire Diffs app is - sadly - loaded.
To be frank, the right approach here is to create an app that only loads the things that are necessary and skips the entirety of the Diffs app, but we're not there yet, either.
Instead, I identified the modules that Webpack spent the longest processing and chose to load that dynamically.
In this case, the longest blocking times were clearly in the load of the Virtual Scrolling modules.
In fact, there wasn't even a close second: the front end application spends 5 or more seconds loading the Virtual Scrolling modules, which accounts for basically all of the largest blocking segments during performance testing.
From what I understand, the Virtual Scrolling isn't used on the Overview/Discussions tab: it's only activated when viewing the full diff, not when viewing small chunks of diff associated to discussions.
I deferred loading the Virtual Scrolling modules until the Diffs tab is activated.
Results
Note these results are skewed because the local development environment is about 3-4x slower than production.
In my testing, the average Total Blocking Time before this change on master
was around 12000ms, or 12 seconds.
Exceptionally good runs clocked in around 9 seconds.
After this change, the average total blocking time was around 5000ms, or 5 seconds.
Exceptionally good runs clocked in around 3.5 seconds.
Taking the best case before, and the worst case after, I expect this change to result in a TBT improvement (reduction) of something like 4 seconds, or around 45%. I would expect the percent to be more accurate on production, as the entire TBT is lower than 4s there anyway, so we're dealing with scaled effects.
Screenshots or Screencasts (strongly suggested)
How to setup and validate locally (strongly suggested)
Does this MR meet the acceptance criteria?
Conformity
-
I have included changelog trailers, or none are needed. (Does this MR need a changelog?) -
I have added/updated documentation, or it's not needed. (Is documentation required?) -
I have properly separated EE content from FOSS, or this MR is FOSS only. (Where should EE code go?) -
I have added information for database reviewers in the MR description, or it's not needed. (Does this MR have database related changes?) -
I have self-reviewed this MR per code review guidelines. -
This MR does not harm performance, or I have asked a reviewer to help assess the performance impact. (Merge request performance guidelines) -
I have followed the style guides. -
This change is backwards compatible across updates, or this does not apply.
Availability and Testing
-
I have added/updated tests following the Testing Guide, or it's not needed. (Consider all test levels. See the Test Planning Process.) -
I have tested this MR in all supported browsers, or it's not needed. -
I have informed the Infrastructure department of a default or new setting change per definition of done, or it's not needed.