Page MenuHomePhabricator

Measure potential performance regression between the old and new VE template dialog sidebar
Closed, ResolvedPublic2 Estimated Story Points

Description

To get a better understanding on the severity of our changes regarding the performance lets compare the unchanged sidebar vs the new sidebar.

Event Timeline

I took the extremely scientific approach of opening the dialog several times and recording how long it took. If anything, there's a small improvement rather than a regression. One follow-up detail to look at is that the dialog is never destroyed, so there might be some memory leakage or penalty for a previous dialog instance having lots of parameters.

Test template had 320 unknown parameters, created as shown in the parent task.

Code change used to measure timing:

diff --git a/modules/ve-mw/ui/dialogs/ve.ui.MWTemplateDialog.js b/modules/ve-mw/ui/dialogs/ve.ui.MWTemplateDialog.js
index 6a6a87c0a..272b137e9 100644
--- a/modules/ve-mw/ui/dialogs/ve.ui.MWTemplateDialog.js
+++ b/modules/ve-mw/ui/dialogs/ve.ui.MWTemplateDialog.js
@@ -470,6 +470,8 @@ ve.ui.MWTemplateDialog.prototype.getActionProcess = function ( action ) {
  */
 ve.ui.MWTemplateDialog.prototype.getSetupProcess = function ( data ) {
        data = data || {};
+       this.loadTiming = new Date().getTime();
+
        return ve.ui.MWTemplateDialog.super.prototype.getSetupProcess.call( this, data )
                .next( function () {
                        var promise,
@@ -586,6 +588,7 @@ ve.ui.MWTemplateDialog.prototype.getSetupProcess = function ( data ) {
                                }
 
                                dialog.bookletLayout.autoFocus = true;
+                               console.log("loadTiming", new Date().getTime() - dialog.loadTiming)
                        } );
                }, this );
 };

Timings taken with the old sidebar, avg 4.5s:

4107
4160
4211
4601
4686
4464
4771
4834
4804
4628

New sidebar: mean 3.7s

2875
2990
3449
4131
3958
3147
2940
4267
4098
4035
4333
3723
4484
4008
3609
awight renamed this task from Look into the performance regression compared to the unchanged sidebar to Measure potential performance regression compared to the unchanged sidebar.Nov 24 2021, 11:30 PM
awight renamed this task from Measure potential performance regression compared to the unchanged sidebar to Measure potential performance regression between the old and new VE template dialog sidebar.

Here is a more detailed view utilizing Chromes profiler as well as https://stackoverflow.com/a/46780568. Old sidebar at the top, new sidebar at the bottom.

Screenshot from 2021-11-25 16-12-28.png (601×1 px, 113 KB)

Screenshot from 2021-11-25 16-12-55.png (621×1 px, 140 KB)

Findings:

  • The two points you found to measure that code's runtime give a pretty good result. Roughly speaking from clicking the edit button to when the dialog can be seen. I marked the relevant section in both flamegraphs. Look for "templates-measure".
  • With my example template this "templates-measure" time is 1.7s with the old vs. 1.5s with the new sidebar.
  • The large third chunk I described in T296335 is not measured with this method. You can see it in the second flamegraph on the right. The editor is blocked for a while there, but can be seen already.
  • Chrome reports the "total blocking time" with 795ms with the old vs. 1087ms with the new sidebar. I would classify this as a performance regression.
  • When I apply all patches that are currently listed in T296386 (I didn't make an extra screenshot for this, it looks similar to the first) our custom "templates-measure" time goes down to 1.2s and Chrome's "total blocking time" down to 547ms.
awight claimed this task.
awight moved this task from Tech Review to Done on the WMDE-TechWish-Sprint-2021-11-24 board.

Closing the investigation. Further work to improve performance is in progress under T296335: Performance: VisualEditor's template dialog is extremely slow on large templates/transclusions.