Cache IdentifiedUser across PrologEnvironments When running multiple rules for the same change, the set of reviewers is identical as ChangeControl goes up the project tree and runs rules in parent projects. Recycle all of the IdentifiedUser objects that were made in one invocation for the next one, to avoid looking them up again in AccountCache. Change-Id: I4031a3f8f9eb0328f0abbbfce1f38eb67b4877d3
diff --git a/gerrit-server/src/main/java/com/google/gerrit/rules/StoredValues.java b/gerrit-server/src/main/java/com/google/gerrit/rules/StoredValues.java index 63a898b..b9f476b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/rules/StoredValues.java +++ b/gerrit-server/src/main/java/com/google/gerrit/rules/StoredValues.java
@@ -16,12 +16,16 @@ import static com.google.gerrit.rules.StoredValue.create; +import com.google.common.collect.Maps; +import com.google.gerrit.reviewdb.client.Account; +import com.google.gerrit.reviewdb.client.AccountDiffPreference.Whitespace; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSetInfo; import com.google.gerrit.reviewdb.client.Project; -import com.google.gerrit.reviewdb.client.AccountDiffPreference.Whitespace; import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.AnonymousUser; +import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.patch.PatchList; import com.google.gerrit.server.patch.PatchListCache; @@ -38,6 +42,8 @@ import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Repository; +import java.util.Map; + public final class StoredValues { public static final StoredValue<ReviewDb> REVIEW_DB = create(ReviewDb.class); public static final StoredValue<Change> CHANGE = create(Change.class); @@ -105,6 +111,23 @@ } }; + public static final StoredValue<AnonymousUser> ANONYMOUS_USER = + new StoredValue<AnonymousUser>() { + @Override + protected AnonymousUser createValue(Prolog engine) { + PrologEnvironment env = (PrologEnvironment) engine.control; + return env.getInjector().getInstance(AnonymousUser.class); + } + }; + + public static final StoredValue<Map<Account.Id, IdentifiedUser>> USERS = + new StoredValue<Map<Account.Id, IdentifiedUser>>() { + @Override + protected Map<Account.Id, IdentifiedUser> createValue(Prolog engine) { + return Maps.newHashMap(); + } + }; + private StoredValues() { } } \ No newline at end of file
diff --git a/gerrit-server/src/main/java/gerrit/PRED_current_user_2.java b/gerrit-server/src/main/java/gerrit/PRED_current_user_2.java index 9005180..1359de1 100644 --- a/gerrit-server/src/main/java/gerrit/PRED_current_user_2.java +++ b/gerrit-server/src/main/java/gerrit/PRED_current_user_2.java
@@ -20,16 +20,12 @@ import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.rules.PrologEnvironment; import com.google.gerrit.rules.StoredValues; -import com.google.gerrit.server.AnonymousUser; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; -import com.google.inject.Provider; import com.google.inject.util.Providers; -import com.googlecode.prolog_cafe.lang.HashtableOfTerm; import com.googlecode.prolog_cafe.lang.IllegalTypeException; import com.googlecode.prolog_cafe.lang.IntegerTerm; -import com.googlecode.prolog_cafe.lang.InternalException; import com.googlecode.prolog_cafe.lang.JavaObjectTerm; import com.googlecode.prolog_cafe.lang.Operation; import com.googlecode.prolog_cafe.lang.PInstantiationException; @@ -40,6 +36,8 @@ import com.googlecode.prolog_cafe.lang.SymbolTerm; import com.googlecode.prolog_cafe.lang.Term; +import java.util.Map; + /** * Loads a CurrentUser object for a user identity. * <p> @@ -54,7 +52,6 @@ private static final long serialVersionUID = 1L; private static final SymbolTerm user = intern("user", 1); private static final SymbolTerm anonymous = intern("anonymous"); - private static final SymbolTerm current_user = intern("current_user"); PRED_current_user_2(Term a1, Term a2, Operation n) { arg1 = a1; @@ -72,24 +69,14 @@ throw new PInstantiationException(this, 1); } - HashtableOfTerm userHash = userHash(engine); - Term userTerm = userHash.get(a1); - if (userTerm != null && userTerm.isJavaObject()) { - if (!(((JavaObjectTerm) userTerm).object() instanceof CurrentUser)) { - userTerm = createUser(engine, a1, userHash); - } - } else { - userTerm = createUser(engine, a1, userHash); - } - - if (!a2.unify(userTerm, engine.trail)) { + if (!a2.unify(createUser(engine, a1), engine.trail)) { return engine.fail(); } return cont; } - public Term createUser(Prolog engine, Term key, HashtableOfTerm userHash) { + public Term createUser(Prolog engine, Term key) { if (!key.isStructure() || key.arity() != 1 || !((StructureTerm) key).functor().equals(user)) { @@ -99,50 +86,30 @@ Term idTerm = key.arg(0); CurrentUser user; if (idTerm.isInteger()) { + Map<Account.Id, IdentifiedUser> cache = StoredValues.USERS.get(engine); Account.Id accountId = new Account.Id(((IntegerTerm) idTerm).intValue()); - - final ReviewDb db = StoredValues.REVIEW_DB.getOrNull(engine); - IdentifiedUser.GenericFactory userFactory = userFactory(engine); - if (db != null) { - user = userFactory.create(Providers.of(db), accountId); - } else { - user = userFactory.create(accountId); + user = cache.get(accountId); + if (user == null) { + ReviewDb db = StoredValues.REVIEW_DB.getOrNull(engine); + IdentifiedUser.GenericFactory userFactory = userFactory(engine); + IdentifiedUser who; + if (db != null) { + who = userFactory.create(Providers.of(db), accountId); + } else { + who = userFactory.create(accountId); + } + cache.put(accountId, who); + user = who; } - } else if (idTerm.equals(anonymous)) { - user = anonymousUser(engine); + user = StoredValues.ANONYMOUS_USER.get(engine); } else { throw new IllegalTypeException(this, 1, "user(int)", key); } - Term userTerm = new JavaObjectTerm(user); - userHash.put(key, userTerm); - return userTerm; - } - - private static HashtableOfTerm userHash(Prolog engine) { - Term userHash = engine.getHashManager().get(current_user); - if (userHash == null) { - HashtableOfTerm users = new HashtableOfTerm(); - engine.getHashManager().put(current_user, new JavaObjectTerm(userHash)); - return users; - } - - if (userHash.isJavaObject()) { - Object obj = ((JavaObjectTerm) userHash).object(); - if (obj instanceof HashtableOfTerm) { - return (HashtableOfTerm) obj; - } - } - - throw new InternalException(current_user + " is not HashtableOfTerm"); - } - - private static AnonymousUser anonymousUser(Prolog engine) { - PrologEnvironment env = (PrologEnvironment) engine.control; - return env.getInjector().getInstance(AnonymousUser.class); + return new JavaObjectTerm(user); } private static IdentifiedUser.GenericFactory userFactory(Prolog engine) {