Coalesce requests within change view

Fetch more data with GET /change/detail instead of multiple separate
GET requests. Fetch independent resources in parallel. Fixed change
actions jumping around.

First page load to full page down by ~25% (1050ms vs 1400ms on localhost,
expect even more on prod server)

Timeline before and after:
http://imgur.com/a/oEFdD

Bug: Issue 4406
Change-Id: Ib7c3c160513cbb29aad89fedabfacbfd271abf22
This commit is contained in:
Viktar Donich
2016-11-01 14:12:07 -07:00
parent 7959720c79
commit ba69a1e192
7 changed files with 41 additions and 38 deletions

View File

@@ -76,7 +76,7 @@ limitations under the License.
on-tap="_handleActionTap"></gr-button>
</template>
</section>
<section hidden$="[[!_actionCount(_revisionActions.*, _additionalActions.*)]]">
<section hidden$="[[!_actionCount(revisionActions.*, _additionalActions.*)]]">
<template is="dom-repeat" items="[[_revisionActionValues]]" as="action">
<gr-button title$="[[action.title]]"
hidden$="[[_computeActionHidden(action.__key, _hiddenRevisionActions.*)]]"

View File

@@ -79,18 +79,18 @@
type: String,
value: '',
},
revisionActions: {
type: Object,
value: function() { return {}; },
},
_loading: {
type: Boolean,
value: true,
},
_revisionActions: {
type: Object,
value: function() { return {}; },
},
_revisionActionValues: {
type: Array,
computed: '_computeRevisionActionValues(_revisionActions.*, ' +
computed: '_computeRevisionActionValues(revisionActions.*, ' +
'primaryActionKeys.*, _additionalActions.*)',
},
_changeActionValues: {
@@ -121,7 +121,7 @@
],
observers: [
'_actionsChanged(actions.*, _revisionActions.*, _additionalActions.*)',
'_actionsChanged(actions.*, revisionActions.*, _additionalActions.*)',
],
ready: function() {
@@ -137,7 +137,7 @@
return this._getRevisionActions().then(function(revisionActions) {
if (!revisionActions) { return; }
this._revisionActions = revisionActions;
this.revisionActions = revisionActions;
this._loading = false;
}.bind(this)).catch(function(err) {
alert('Couldnt load revision actions. Check the console ' +
@@ -363,7 +363,7 @@
/* falls through */ // required by JSHint
default:
this._fireAction(this._prependSlash(key),
this._revisionActions[key], true);
this.revisionActions[key], true);
}
},
@@ -400,7 +400,7 @@
}
this.$.overlay.close();
el.hidden = true;
this._fireAction('/rebase', this._revisionActions.rebase, true, payload);
this._fireAction('/rebase', this.revisionActions.rebase, true, payload);
},
_handleCherrypickConfirm: function() {
@@ -418,7 +418,7 @@
el.hidden = true;
this._fireAction(
'/cherrypick',
this._revisionActions.cherrypick,
this.revisionActions.cherrypick,
true,
{
destination: el.branch,

View File

@@ -248,6 +248,7 @@ limitations under the License.
<gr-change-actions id="actions"
change="[[_change]]"
actions="[[_change.actions]]"
revision-actions="[[_currentRevisionActions]]"
change-num="[[_changeNum]]"
patch-num="[[_computeLatestPatchNum(_allPatchSets)]]"
commit-message="[[_latestCommitMessage]]"

View File

@@ -55,6 +55,7 @@
observer: '_changeChanged',
},
_commitInfo: Object,
_files: Object,
_changeNum: String,
_diffDrafts: {
type: Object,
@@ -77,6 +78,7 @@
type: Object,
observer: '_updateSelected',
},
_currentRevisionActions: Object,
_allPatchSets: {
type: Array,
computed: '_computeAllPatchSets(_change)',
@@ -681,7 +683,16 @@
if (!change.reviewer_updates) {
change.reviewer_updates = null;
}
var currentRevision = change.revisions &&
change.revisions[change.current_revision];
this._latestCommitMessage = currentRevision.commit.message;
this._change = change;
if (!this._patchRange || !this._patchRange.patchNum ||
this._patchRange.patchNum === currentRevision._number) {
this._commitInfo = currentRevision.commit;
this._currentRevisionActions = currentRevision.actions;
// TODO: Fetch and process files.
}
}.bind(this));
},
@@ -728,40 +739,25 @@
var detailCompletes = this._getChangeDetail().then(function() {
this._loading = false;
this._getProjectConfig();
}.bind(this));
this._getComments();
if (this._patchRange.patchNum) {
return this._reloadPatchNumDependentResources().then(function() {
return detailCompletes;
}).then(function() {
return this._reloadDetailDependentResources();
return Promise.all([
this._reloadPatchNumDependentResources(),
detailCompletes,
]).then(function() {
return this.$.actions.reload();
}.bind(this));
} else {
// The patch number is reliant on the change detail request.
return detailCompletes.then(function() {
return this._reloadPatchNumDependentResources();
}.bind(this)).then(function() {
return this._reloadDetailDependentResources();
this.$.fileList.reload();
}.bind(this));
}
},
/**
* Kicks off requests for resources that rely on the change detail
* (`this._change`) being loaded.
*/
_reloadDetailDependentResources: function() {
if (!this._change) { return Promise.resolve(); }
return this._getProjectConfig().then(function() {
return Promise.all([
this._getLatestCommitMessage(),
this.$.actions.reload(),
]);
}.bind(this));
},
/**
* Kicks off requests for resources that rely on the patch range
* (`this._patchRange`) being defined.

View File

@@ -102,7 +102,7 @@ limitations under the License.
return Promise.resolve({
change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
revisions: {
rev1: {_number: 1},
rev1: {_number: 1, commit: {}},
rev13: {_number: 13},
},
current_revision: 'rev1',
@@ -468,7 +468,12 @@ limitations under the License.
test('topic is coalesced to null', function(done) {
sandbox.stub(element, '_changeChanged');
sandbox.stub(element.$.restAPI, 'getChangeDetail', function() {
return Promise.resolve({id: '123456789', labels: {}});
return Promise.resolve({
id: '123456789',
labels: {},
current_revision: 'foo',
revisions: {foo: {commit: {}}},
});
});
element._getChangeDetail().then(function() {

View File

@@ -22,4 +22,3 @@ limitations under the License.
<dom-module id="gr-rest-api-interface">
<script src="gr-rest-api-interface.js"></script>
</dom-module>

View File

@@ -388,11 +388,13 @@
var options = this._listChangesOptionsToHex(
ListChangesOption.ALL_REVISIONS,
ListChangesOption.CHANGE_ACTIONS,
ListChangesOption.CURRENT_ACTIONS,
ListChangesOption.CURRENT_COMMIT,
ListChangesOption.DOWNLOAD_COMMANDS,
ListChangesOption.SUBMITTABLE
);
return this._getChangeDetail(changeNum, options, opt_errFn,
opt_cancelCondition);
return this._getChangeDetail(
changeNum, options, opt_errFn, opt_cancelCondition);
},
getDiffChangeDetail: function(changeNum, opt_errFn, opt_cancelCondition) {