opendev/gerrit
Code Issues Proposed changes

Cache recent project configs globally in the client

Browse Source 
It is repeating a lot of work to GET /projects/X/config on every
change/patch set/diff load. Instead, just cache a few recent configs.
Don't worry about invalidation for now; the info that we're caching
shouldn't change very often, and if it does, the user just has to
reload the app. (This would not be safe if we were caching permissions
on the client side, but it seems unlikely we would do that.)

Change-Id: I7550311cf32b66498b2998897ccac55434f04a1e
This commit is contained in:
Dave Borowitz
2013-04-08 16:57:02 -07:00
parent 3203293510
commit 7239d86673
6 changed files with 104 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeScreen.java
View File

@@ -18,8 +18,7 @@ import com.google.gerrit.client.Dispatcher;
import com.google.gerrit.client.FormatUtil;
import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.account.AccountInfo;
import com.google.gerrit.client.projects.ConfigInfo;
import com.google.gerrit.client.projects.ProjectApi;
import com.google.gerrit.client.projects.ConfigInfoCache;
import com.google.gerrit.client.rpc.CallbackGroup;
import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.client.ui.CommentLinkProcessor;
@@ -270,12 +269,12 @@ public class ChangeScreen extends Screen
// start an async get at the source of every call that might trigger a
// value change.
CallbackGroup cbs = new CallbackGroup();
ProjectApi.config(event.getValue().getChange().getProject())
.get(cbs.add(new GerritCallback<ConfigInfo>() {
ConfigInfoCache.get(
event.getValue().getChange().getProject(),
cbs.add(new GerritCallback<ConfigInfoCache.Entry>() {
@Override
public void onSuccess(ConfigInfo result) {
commentLinkProcessor =
new CommentLinkProcessor(result.commentlinks());
public void onSuccess(ConfigInfoCache.Entry result) {
commentLinkProcessor = result.getCommentLinkProcessor();
}
@Override

12
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java
View File

@@ -20,8 +20,7 @@ import com.google.gerrit.client.changes.ChangeInfo.LabelInfo;
import com.google.gerrit.client.patches.AbstractPatchContentTable;
import com.google.gerrit.client.patches.CommentEditorContainer;
import com.google.gerrit.client.patches.CommentEditorPanel;
import com.google.gerrit.client.projects.ConfigInfo;
import com.google.gerrit.client.projects.ProjectApi;
import com.google.gerrit.client.projects.ConfigInfoCache;
import com.google.gerrit.client.rpc.CallbackGroup;
import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.client.rpc.Natives;
@@ -181,12 +180,11 @@ public class PublishCommentScreen extends AccountScreen implements
private void preDisplay(final PatchSetPublishDetail pubDetail,
final ScreenLoadCallback<PatchSetPublishDetail> origCb) {
ProjectApi.config(pubDetail.getChange().getProject())
.get(new AsyncCallback<ConfigInfo>() {
ConfigInfoCache.get(pubDetail.getChange().getProject(),
new AsyncCallback<ConfigInfoCache.Entry>() {
@Override
public void onSuccess(ConfigInfo result) {
commentLinkProcessor =
new CommentLinkProcessor(result.commentlinks());
public void onSuccess(ConfigInfoCache.Entry result) {
commentLinkProcessor = result.getCommentLinkProcessor();
display(pubDetail);
}

12
gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScreen.java
View File

@@ -21,8 +21,7 @@ import com.google.gerrit.client.RpcStatus;
import com.google.gerrit.client.changes.CommitMessageBlock;
import com.google.gerrit.client.changes.PatchTable;
import com.google.gerrit.client.changes.Util;
import com.google.gerrit.client.projects.ConfigInfo;
import com.google.gerrit.client.projects.ProjectApi;
import com.google.gerrit.client.projects.ConfigInfoCache;
import com.google.gerrit.client.rpc.CallbackGroup;
import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.client.rpc.ScreenLoadCallback;
@@ -392,12 +391,11 @@ public abstract class PatchScreen extends Screen implements
if (commentLinkProcessor == null) {
// Fetch config in parallel if we haven't previously.
CallbackGroup cb = new CallbackGroup();
ProjectApi.config(patchSetDetail.getProject())
.get(cb.add(new AsyncCallback<ConfigInfo>() {
ConfigInfoCache.get(patchSetDetail.getProject(),
cb.add(new AsyncCallback<ConfigInfoCache.Entry>() {
@Override
public void onSuccess(ConfigInfo result) {
commentLinkProcessor =
new CommentLinkProcessor(result.commentlinks());
public void onSuccess(ConfigInfoCache.Entry result) {
commentLinkProcessor = result.getCommentLinkProcessor();
contentTable.setCommentLinkProcessor(commentLinkProcessor);
}

2
gerrit-gwtui/src/main/java/com/google/gerrit/client/projects/ConfigInfo.java
View File

@@ -47,7 +47,7 @@ public class ConfigInfo extends JavaScriptObject {
private final native NativeMap<CommentLinkInfo> commentlinks0()
/*-{ return this.commentlinks; }-*/;
public final List<FindReplace> commentlinks() {
final List<FindReplace> commentlinks() {
JsArray<CommentLinkInfo> cls = commentlinks0().values();
List<FindReplace> commentLinks = new ArrayList<FindReplace>(cls.length());
for (int i = 0; i < cls.length(); i++) {

86
gerrit-gwtui/src/main/java/com/google/gerrit/client/projects/ConfigInfoCache.java Normal file
View File

@@ -0,0 +1,86 @@
// Copyright (C) 2013 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.client.projects;
import com.google.gerrit.client.ui.CommentLinkProcessor;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gwt.core.client.GWT;
import com.google.gwt.user.client.rpc.AsyncCallback;
import java.util.LinkedHashMap;
import java.util.Map;
/** Cache of {@link ConfigInfo} objects by project name. */
public class ConfigInfoCache {
private static final int LIMIT = 25;
private static final ConfigInfoCache instance =
GWT.create(ConfigInfoCache.class);
public static class Entry {
private final ConfigInfo info;
private CommentLinkProcessor commentLinkProcessor;
private Entry(ConfigInfo info) {
this.info = info;
}
public CommentLinkProcessor getCommentLinkProcessor() {
if (commentLinkProcessor == null) {
commentLinkProcessor = new CommentLinkProcessor(info.commentlinks());
}
return commentLinkProcessor;
}
}
public static void get(Project.NameKey name, AsyncCallback<Entry> cb) {
instance.getImpl(name, cb);
}
private final LinkedHashMap<String, Entry> cache;
protected ConfigInfoCache() {
cache = new LinkedHashMap<String, Entry>(LIMIT) {
private static final long serialVersionUID = 1L;
@Override
protected boolean removeEldestEntry(
Map.Entry<String, ConfigInfoCache.Entry> e) {
return size() > LIMIT;
}
};
}
private void getImpl(final Project.NameKey name,
final AsyncCallback<Entry> cb) {
Entry e = cache.get(name.get());
if (e != null) {
cb.onSuccess(e);
return;
}
ProjectApi.config(name).get(new AsyncCallback<ConfigInfo>() {
@Override
public void onSuccess(ConfigInfo result) {
Entry e = new Entry(result);
cache.put(name.get(), e);
cb.onSuccess(e);
}
@Override
public void onFailure(Throwable caught) {
cb.onFailure(caught);
}
});
}
}

2
gerrit-gwtui/src/main/java/com/google/gerrit/client/projects/ProjectApi.java
View File

@@ -33,7 +33,7 @@ public class ProjectApi {
.put(input, asyncCallback);
}
public static RestApi config(Project.NameKey name) {
static RestApi config(Project.NameKey name) {
return new RestApi("/projects/").id(name.get()).view("config");
}