Skip to content

Improve browser TBT of merge request overview tab

Thomas Randolph requested to merge tor/perf/mr-overview-discussions into master

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 evaled 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

Availability and Testing

Edited by Phil Hughes

Merge request reports