opendev/gerrit
Code Issues Proposed changes

Make PrologEnvironment ~6500% faster

Browse Source 
Instead of passing the Injector use a singleton Args object that wraps
up all of the singletons needed in prolog predicates.

This offers less flexibility to plugin predicates but gets rid of a
potential Guice problem.  The claimed performance difference may not
exist, but a warning message is removed on startup.

Change-Id: Iceba5d4fb7f75eb867c6a968e5f43912e2213837
This commit is contained in:
Shawn Pearce
2013-08-05 16:40:04 -07:00
parent 242a604b9f
commit 9b24f90849
7 changed files with 89 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

69
gerrit-server/src/main/java/com/google/gerrit/rules/PrologEnvironment.java
View File

@@ -14,8 +14,15 @@
package com.google.gerrit.rules;
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.PatchListCache;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.project.ProjectCache;
import com.google.inject.Inject;
import com.google.inject.Injector;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import com.google.inject.assistedinject.Assisted;
import com.googlecode.prolog_cafe.lang.BufferingPrologControl;
@@ -56,23 +63,22 @@ public class PrologEnvironment extends BufferingPrologControl {
PrologEnvironment create(PrologMachineCopy src);
}
private final Injector injector;
private final Args args;
private final Map<StoredValue<Object>, Object> storedValues;
private List<Runnable> cleanup;
@Inject
PrologEnvironment(Injector i, @Assisted PrologMachineCopy src) {
PrologEnvironment(Args a, @Assisted PrologMachineCopy src) {
super(src);
injector = i;
setMaxArity(MAX_ARITY);
setEnabled(EnumSet.allOf(Prolog.Feature.class), false);
args = a;
storedValues = new HashMap<StoredValue<Object>, Object>();
cleanup = new LinkedList<Runnable>();
}
/** Get the global Guice Injector that configured the environment. */
public Injector getInjector() {
return injector;
public Args getArgs() {
return args;
}
/**
@@ -139,4 +145,53 @@ public class PrologEnvironment extends BufferingPrologControl {
i.remove();
}
}
@Singleton
public static class Args {
private final ProjectCache projectCache;
private final GitRepositoryManager repositoryManager;
private final PatchListCache patchListCache;
private final PatchSetInfoFactory patchSetInfoFactory;
private final IdentifiedUser.GenericFactory userFactory;
private final Provider<AnonymousUser> anonymousUser;
@Inject
Args(ProjectCache projectCache,
GitRepositoryManager repositoryManager,
PatchListCache patchListCache,
PatchSetInfoFactory patchSetInfoFactory,
IdentifiedUser.GenericFactory userFactory,
Provider<AnonymousUser> anonymousUser) {
this.projectCache = projectCache;
this.repositoryManager = repositoryManager;
this.patchListCache = patchListCache;
this.patchSetInfoFactory = patchSetInfoFactory;
this.userFactory = userFactory;
this.anonymousUser = anonymousUser;
}
public ProjectCache getProjectCache() {
return projectCache;
}
public GitRepositoryManager getGitRepositoryManager() {
return repositoryManager;
}
public PatchListCache getPatchListCache() {
return patchListCache;
}
public PatchSetInfoFactory getPatchSetInfoFactory() {
return patchSetInfoFactory;
}
public IdentifiedUser.GenericFactory getUserFactory() {
return userFactory;
}
public AnonymousUser getAnonymousUser() {
return anonymousUser.get();
}
}
}

12
gerrit-server/src/main/java/com/google/gerrit/rules/PrologModule.java
View File

@@ -20,7 +20,15 @@ import com.google.gerrit.server.config.FactoryModule;
public class PrologModule extends FactoryModule {
@Override
protected void configure() {
DynamicSet.setOf(binder(), PredicateProvider.class);
factory(PrologEnvironment.Factory.class);
install(new EnviromentModule());
bind(PrologEnvironment.Args.class);
}
static class EnviromentModule extends FactoryModule {
@Override
protected void configure() {
DynamicSet.setOf(binder(), PredicateProvider.class);
factory(PrologEnvironment.Factory.class);
}
}
}

9
gerrit-server/src/main/java/com/google/gerrit/rules/StoredValues.java
View File

@@ -60,7 +60,7 @@ public final class StoredValues {
PatchSet ps = StoredValues.PATCH_SET.get(engine);
PrologEnvironment env = (PrologEnvironment) engine.control;
PatchSetInfoFactory patchInfoFactory =
env.getInjector().getInstance(PatchSetInfoFactory.class);
env.getArgs().getPatchSetInfoFactory();
try {
return patchInfoFactory.get(change, ps);
} catch (PatchSetInfoNotAvailableException e) {
@@ -74,7 +74,7 @@ public final class StoredValues {
public PatchList createValue(Prolog engine) {
PrologEnvironment env = (PrologEnvironment) engine.control;
PatchSetInfo psInfo = StoredValues.PATCH_SET_INFO.get(engine);
PatchListCache plCache = env.getInjector().getInstance(PatchListCache.class);
PatchListCache plCache = env.getArgs().getPatchListCache();
Change change = StoredValues.CHANGE.get(engine);
Project.NameKey projectKey = change.getProject();
ObjectId a = null;
@@ -95,8 +95,7 @@ public final class StoredValues {
@Override
public Repository createValue(Prolog engine) {
PrologEnvironment env = (PrologEnvironment) engine.control;
GitRepositoryManager gitMgr =
env.getInjector().getInstance(GitRepositoryManager.class);
GitRepositoryManager gitMgr = env.getArgs().getGitRepositoryManager();
Change change = StoredValues.CHANGE.get(engine);
Project.NameKey projectKey = change.getProject();
final Repository repo;
@@ -122,7 +121,7 @@ public final class StoredValues {
@Override
protected AnonymousUser createValue(Prolog engine) {
PrologEnvironment env = (PrologEnvironment) engine.control;
return env.getInjector().getInstance(AnonymousUser.class);
return env.getArgs().getAnonymousUser();
}
};

3
gerrit-server/src/main/java/gerrit/PRED_current_user_2.java
View File

@@ -112,7 +112,6 @@ class PRED_current_user_2 extends Predicate.P2 {
}
private static IdentifiedUser.GenericFactory userFactory(Prolog engine) {
PrologEnvironment env = (PrologEnvironment) engine.control;
return env.getInjector().getInstance(IdentifiedUser.GenericFactory.class);
return ((PrologEnvironment) engine.control).getArgs().getUserFactory();
}
}

3
gerrit-server/src/main/java/gerrit/PRED_get_legacy_label_types_1.java
View File

@@ -17,7 +17,6 @@ package gerrit;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.rules.PrologEnvironment;
import com.google.gerrit.rules.StoredValues;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.googlecode.prolog_cafe.lang.IntegerTerm;
@@ -55,7 +54,7 @@ class PRED_get_legacy_label_types_1 extends Predicate.P1 {
Term a1 = arg1.dereference();
PrologEnvironment env = (PrologEnvironment) engine.control;
ProjectState state = env.getInjector().getInstance(ProjectCache.class)
ProjectState state = env.getArgs().getProjectCache()
.get(StoredValues.CHANGE.get(engine).getDest().getParentKey());
if (state == null) {
return engine.fail();

10
gerrit-server/src/test/java/com/google/gerrit/rules/GerritCommonTest.java
View File

@@ -49,7 +49,15 @@ public class GerritCommonTest extends PrologTestCase {
load("gerrit", "gerrit_common_test.pl", new AbstractModule() {
@Override
protected void configure() {
bind(ProjectCache.class).toInstance(projects);
bind(PrologEnvironment.Args.class).toInstance(
new PrologEnvironment.Args(
projects,
null,
null,
null,
null,
null
));
}
});
}

2
gerrit-server/src/test/java/com/google/gerrit/rules/PrologTestCase.java
View File

@@ -55,7 +55,7 @@ public abstract class PrologTestCase extends TestCase {
protected void load(String pkg, String prologResource, Module... modules)
throws CompileException, IOException {
ArrayList<Module> moduleList = new ArrayList<Module>();
moduleList.add(new PrologModule());
moduleList.add(new PrologModule.EnviromentModule());
moduleList.addAll(Arrays.asList(modules));
envFactory = Guice.createInjector(moduleList)