Improved shared group decomposition
Shared groups were broken down in I8018535ee7 to improve perceived
performance using a diff of Go's go_spec.html as a benchmark. However,
the total render time became slower with this change because the
decomposed groups thrashed the layout.
With this change, shared groups are decomposed into two groups to
achieve the same perceived responsiveness, but balance that with total
render time and avoid needless layouts.
The Maximum group size in increased to 120 to further reduce the number
of groups.
Some performance numbers for the go_spec.html benchmark diff and the
config_dump.json diff indicated in the linked issue.
+------------------+--------------+
| config_dump.json | go_spec.html |
+-----------------------+------------------+--------------+
| Content Render Before | 83877.2 ms | 7815.2 ms |
+-----------------------+------------------+--------------+
| Content Render After | 17492.0 ms | 3363.4 ms |
+-----------------------+------------------+--------------+
| Speedup Factor | ~4.8 | ~2.3 |
+-----------------------+------------------+--------------+
Further performance improvements to follow.
Bug: Issue 5778
Change-Id: I97751b7b78b821a794374cbfeecb16d59d5e1c4c
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js
index 62e8915..30d01cb 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js
@@ -36,11 +36,11 @@
* The maximum size for an addition or removal chunk before it is broken down
* into a series of chunks that are this size at most.
*
- * Note: The value of 70 is chosen so that it is larger than the default
+ * Note: The value of 120 is chosen so that it is larger than the default
* _asyncThreshold of 64, but feel free to tune this constant to your
* performance needs.
*/
- var MAX_GROUP_SIZE = 70;
+ var MAX_GROUP_SIZE = 120;
Polymer({
is: 'gr-diff-processor',
@@ -364,8 +364,11 @@
if (this.context === -1) {
var newContent = [];
content.forEach(function(group) {
- if (group.ab) {
- newContent.push.apply(newContent, this._breakdownGroup(group));
+ if (group.ab && group.ab.length > MAX_GROUP_SIZE * 2) {
+ // Split large shared groups in two, where the first is the maximum
+ // group size.
+ newContent.push({ab: group.ab.slice(0, MAX_GROUP_SIZE)});
+ newContent.push({ab: group.ab.slice(MAX_GROUP_SIZE)});
} else {
newContent.push(group);
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor_test.html b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor_test.html
index 687b3dd..1a5ccc5 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor_test.html
@@ -337,15 +337,16 @@
});
test('breaks-down shared chunks w/ whole-file', function() {
+ var size = 120 * 2 + 5;
var lineNums = {left: 0, right: 0};
var content = [{
- ab: _.times(75, function() { return '' + Math.random(); }),
+ ab: _.times(size, function() { return '' + Math.random(); }),
}];
element.context = -1;
var result = element._splitCommonGroupsWithComments(content, lineNums);
assert.equal(result.length, 2);
- assert.deepEqual(result[0].ab, content[0].ab.slice(0, 5));
- assert.deepEqual(result[1].ab, content[0].ab.slice(5));
+ assert.deepEqual(result[0].ab, content[0].ab.slice(0, 120));
+ assert.deepEqual(result[1].ab, content[0].ab.slice(120));
});
test('does not break-down shared chunks w/ context', function() {
