opendev/gerrit
Code Issues Proposed changes

Applies optimizations to GR-DIFF-SELECTION

Browse Source 
A major source of latency in creating comments in large diffs stems from
GR-DIFF-SELECTION applying selection-restricting classes to a parent of
the diff. When the diff contains a large number of lines (and thus a
large number of elements) applying a class forces a style recompute for
the substantial subtree.

This change optimizes this in two ways:

* **Initialize to the right side:** The selection-restricting class is
  initialized to the right side, before the diff is even completely
  built. This eliminates the need for a recomputation preceding the
  first comment add on the right side (the most-likely side to be
  interacted with).
* **Minimize the number of class modifications:** only add or remove
  classes when necessary. This eliminates recomputes for consecutive
  mousedown events that occur on the a common diff side.

On an i7 MBP, this eliminates ~120ms for the first comment on the right
and ~40ms for subsequent such comments.

Change-Id: Ibb8a7eca0398a5c7265dc1385967bce3dae5e5ef
This commit is contained in:
Wyatt Allen
2016-08-10 11:29:17 -07:00
parent f496436abe
commit 0490413eb8
1 changed files with 13 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.js
View File

@@ -26,6 +26,10 @@
'down': '_handleDown',
},
attached: function() {
this.classList.add('selected-right');
},
get diffBuilder() {
if (!this._cachedDiffBuilder) {
this._cachedDiffBuilder =
@@ -40,8 +44,15 @@
return;
}
var side = this.diffBuilder.getSideByLineEl(lineEl);
this.classList.remove('selected-right', 'selected-left');
this.classList.add('selected-' + side);
var targetClass = 'selected-' + side;
var alternateClass = 'selected-' + (side === 'left' ? 'right' : 'left');
if (this.classList.contains(alternateClass)) {
this.classList.remove(alternateClass);
}
if (!this.classList.contains(targetClass)) {
this.classList.add(targetClass);
}
},
_handleCopy: function(e) {