From a1accedd9049d9bc79c93fb79513f39a6182bb67 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Fri, 6 Sep 2013 22:18:52 -0700 Subject: [PATCH] Load user's starred changes async to query Some database implementations are able to run a query in the background and only stall the application when the ResultSet is iterated and the next object has not yet been returned. If the caller is signed in and running a change query the JSON formatter will need to use the starred change collection. Begin loading the starred change collection for the caller before the change query is parsed and executed, giving the database time to load the starred changes in the background. This should reduce some search result latency for signed in users on gerrit-review, where the database is slow but ResultSet loads objects async to the application. Change-Id: Ic19c3242d2fc34f5e4417989667beb51c0d6260d --- .../google/gerrit/server/IdentifiedUser.java | 42 ++++++++++++++++--- .../server/query/change/QueryChanges.java | 24 ++++++++++- 2 files changed, 60 insertions(+), 6 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java b/gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java index e4209f0f4c..7ab256497b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java @@ -15,6 +15,7 @@ package com.google.gerrit.server; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Sets; import com.google.gerrit.common.data.AccountInfo; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountDiffPreference; @@ -34,6 +35,7 @@ import com.google.gerrit.server.config.AnonymousCowardName; import com.google.gerrit.server.config.AuthConfig; import com.google.gerrit.server.config.CanonicalWebUrl; import com.google.gwtorm.server.OrmException; +import com.google.gwtorm.server.ResultSet; import com.google.inject.Inject; import com.google.inject.OutOfScopeException; import com.google.inject.Provider; @@ -53,7 +55,6 @@ import java.net.URL; import java.util.Collection; import java.util.Collections; import java.util.Date; -import java.util.HashSet; import java.util.List; import java.util.Set; import java.util.TimeZone; @@ -195,6 +196,7 @@ public class IdentifiedUser extends CurrentUser { private Set emailAddresses; private GroupMembership effectiveGroups; private Set starredChanges; + private ResultSet starredQuery; private Collection notificationFilters; private CurrentUser realUser; @@ -296,11 +298,18 @@ public class IdentifiedUser extends CurrentUser { if (dbProvider == null) { throw new OutOfScopeException("Not in request scoped user"); } - final Set h = new HashSet(); + Set h = Sets.newHashSet(); try { - for (final StarredChange sc : dbProvider.get().starredChanges() - .byAccount(getAccountId())) { - h.add(sc.getChangeId()); + if (starredQuery != null) { + for (StarredChange sc : starredQuery) { + h.add(sc.getChangeId()); + } + starredQuery = null; + } else { + for (StarredChange sc : dbProvider.get().starredChanges() + .byAccount(getAccountId())) { + h.add(sc.getChangeId()); + } } } catch (OrmException e) { log.warn("Cannot query starred by user changes", e); @@ -310,6 +319,29 @@ public class IdentifiedUser extends CurrentUser { return starredChanges; } + public void asyncStarredChanges() { + if (starredChanges == null && dbProvider != null) { + try { + starredQuery = + dbProvider.get().starredChanges().byAccount(getAccountId()); + } catch (OrmException e) { + log.warn("Cannot query starred by user changes", e); + starredQuery = null; + starredChanges = Collections.emptySet(); + } + } + } + + public void abortStarredChanges() { + if (starredQuery != null) { + try { + starredQuery.close(); + } finally { + starredQuery = null; + } + } + } + @Override public Collection getNotificationFilters() { if (notificationFilters == null) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryChanges.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryChanges.java index 5084fff045..7ca24b2743 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryChanges.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryChanges.java @@ -20,12 +20,15 @@ import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.RestReadView; import com.google.gerrit.extensions.restapi.TopLevelResource; +import com.google.gerrit.server.CurrentUser; +import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.change.ChangeJson; import com.google.gerrit.server.change.ChangeJson.ChangeInfo; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.query.QueryParseException; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; +import com.google.inject.Provider; import org.kohsuke.args4j.Option; @@ -39,6 +42,7 @@ import java.util.regex.Pattern; public class QueryChanges implements RestReadView { private final ChangeJson json; private final QueryProcessor imp; + private final Provider user; private boolean reverse; private EnumSet options; @@ -80,9 +84,11 @@ public class QueryChanges implements RestReadView { @Inject QueryChanges(ChangeJson json, QueryProcessor qp, - ChangeControl.Factory cf) { + ChangeControl.Factory cf, + Provider user) { this.json = json; this.imp = qp; + this.user = user; options = EnumSet.noneOf(ListChangesOption.class); json.setChangeControlFactory(cf); @@ -127,6 +133,22 @@ public class QueryChanges implements RestReadView { throw new QueryParseException("limit of 10 queries"); } + IdentifiedUser self = null; + try { + if (user.get().isIdentifiedUser()) { + self = (IdentifiedUser) user.get(); + self.asyncStarredChanges(); + } + return query0(); + } finally { + if (self != null) { + self.abortStarredChanges(); + } + } + } + + private List> query0() throws OrmException, + QueryParseException { int cnt = queries.size(); BitSet more = new BitSet(cnt); List> data = Lists.newArrayListWithCapacity(cnt);