Use PrologMachineCopy in RulesCache to avoid consult/1

The predicate consult/1 is slow, parsing a Prolog source file from
disk and inserting it into the in-memory database can take up to a
full second for even a fairly simple rule script. Instead of doing
the consult for each new PrologEnvironment being created, run it
once as part of the RulesCache and reuse the same set of terms in
other machine executions.

Change-Id: I97ee963e4f44fa0a89a100575ef0216075f000c4

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

@@ -20,7 +20,7 @@ import com.google.inject.assistedinject.Assisted;
 
 import com.googlecode.prolog_cafe.lang.BufferingPrologControl;
 import com.googlecode.prolog_cafe.lang.Prolog;
-import com.googlecode.prolog_cafe.lang.PrologClassLoader;
+import com.googlecode.prolog_cafe.lang.PrologMachineCopy;
 import com.googlecode.prolog_cafe.lang.SystemException;
 import com.googlecode.prolog_cafe.lang.Term;
 
@@ -34,6 +34,8 @@ import java.util.EnumSet;
  * A single copy of the Prolog interpreter, for the current thread.
  */
 public class PrologEnvironment extends BufferingPrologControl {
+  static final int MAX_ARITY = 8;
+
   private static final String[] PACKAGE_LIST = {
       Prolog.BUILTIN,
       "gerrit",
@@ -43,21 +45,20 @@ public class PrologEnvironment extends BufferingPrologControl {
     /**
      * Construct a new Prolog interpreter.
      *
-     * @param cl ClassLoader to dynamically load predicates from.
+     * @param src the machine to template the new environment from.
      * @return the new interpreter.
      */
-    PrologEnvironment create(ClassLoader cl);
+    PrologEnvironment create(PrologMachineCopy src);
   }
 
   private final Injector injector;
   private boolean intialized;
 
   @Inject
-  PrologEnvironment(Injector i, @Assisted ClassLoader newCL) {
+  PrologEnvironment(Injector i, @Assisted PrologMachineCopy src) {
+    super(src);
     injector = i;
-    setPrologClassLoader(new PrologClassLoader(newCL));
+    setMaxArity(MAX_ARITY);
-    setMaxArity(8);
-    setMaxDatabaseSize(64);
     setEnabled(EnumSet.allOf(Prolog.Feature.class), false);
   }
 

gerrit-server/src/main/java/com/google/gerrit/rules/RulesCache.java

@@ -14,24 +14,43 @@
 
 package com.google.gerrit.rules;
 
+import static com.googlecode.prolog_cafe.lang.PrologMachineCopy.save;
+
+import com.google.gerrit.reviewdb.Project;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.SitePaths;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
 
+import com.googlecode.prolog_cafe.compiler.CompileException;
+import com.googlecode.prolog_cafe.lang.BufferingPrologControl;
+import com.googlecode.prolog_cafe.lang.JavaObjectTerm;
+import com.googlecode.prolog_cafe.lang.Prolog;
+import com.googlecode.prolog_cafe.lang.PrologClassLoader;
+import com.googlecode.prolog_cafe.lang.PrologMachineCopy;
+import com.googlecode.prolog_cafe.lang.SymbolTerm;
+
+import org.eclipse.jgit.errors.LargeObjectException;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
 import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.ObjectId;
-import org.slf4j.Logger;
+import org.eclipse.jgit.lib.ObjectLoader;
-import org.slf4j.LoggerFactory;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.util.RawParseUtils;
 
 import java.io.File;
+import java.io.IOException;
+import java.io.PushbackReader;
+import java.io.StringReader;
 import java.lang.ref.Reference;
 import java.lang.ref.ReferenceQueue;
 import java.lang.ref.WeakReference;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.net.URLClassLoader;
+import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -44,83 +63,155 @@ import java.util.Map;
  */
 @Singleton
 public class RulesCache {
-  private static final Logger log = LoggerFactory.getLogger(RulesCache.class);
+  /** Maximum size of a dynamic Prolog script, in bytes. */
+  private static final int SRC_LIMIT = 128 * 1024;
 
-  private final Map<ObjectId, LoaderRef> classLoaderCache =
+  /** Default size of the internal Prolog database within each interpreter. */
-    new HashMap<ObjectId, LoaderRef>();
+  private static final int DB_MAX = 64;
 
-  private final ReferenceQueue<ClassLoader> DEAD = new ReferenceQueue<ClassLoader>();
+  private final Map<ObjectId, MachineRef> machineCache =
+      new HashMap<ObjectId, MachineRef>();
 
-  private final File cacheDir;
+  private final ReferenceQueue<PrologMachineCopy> dead =
-  private final File rulesDir;
+      new ReferenceQueue<PrologMachineCopy>();
 
-  private final class LoaderRef extends WeakReference<ClassLoader> {
+  private static final class MachineRef extends WeakReference<PrologMachineCopy> {
     final ObjectId key;
 
-    LoaderRef(ObjectId key, ClassLoader loader) {
+    MachineRef(ObjectId key, PrologMachineCopy pcm,
-      super(loader, DEAD);
+        ReferenceQueue<PrologMachineCopy> queue) {
+      super(pcm, queue);
       this.key = key;
     }
   }
 
+  private final File cacheDir;
+  private final File rulesDir;
+  private final GitRepositoryManager gitMgr;
+  private final ClassLoader systemLoader;
+  private final PrologMachineCopy defaultMachine;
+
   @Inject
-  protected RulesCache (@GerritServerConfig Config config, SitePaths site) {
+  protected RulesCache(@GerritServerConfig Config config, SitePaths site,
+      GitRepositoryManager gm) {
     cacheDir = site.resolve(config.getString("cache", null, "directory"));
     rulesDir = cacheDir != null ? new File(cacheDir, "rules") : null;
+    gitMgr = gm;
+
+    systemLoader = getClass().getClassLoader();
+    defaultMachine = save(newEmptyMachine(systemLoader));
   }
 
   /**
-   * @return ClassLoader with compiled rules jar from rules.pl if it exists;
+   * Locate a cached Prolog machine state, or create one if not available.
-   *         null otherwise.
+   *
+   * @return a Prolog machine, after loading the specified rules.
+   * @throws CompileException the machine cannot be created.
    */
-  public synchronized ClassLoader getClassLoader(ObjectId rulesId) {
+  public synchronized PrologMachineCopy loadMachine(
-    if (rulesId == null || rulesDir == null) {
+      Project.NameKey project,
-      return null;
+      ObjectId rulesId)
+      throws CompileException {
+    if (project == null || rulesId == null) {
+      return defaultMachine;
     }
 
-    Reference<? extends ClassLoader> ref = classLoaderCache.get(rulesId);
+    Reference<? extends PrologMachineCopy> ref = machineCache.get(rulesId);
     if (ref != null) {
-      ClassLoader cl = ref.get();
+      PrologMachineCopy pmc = ref.get();
-      if (cl != null) {
+      if (pmc != null) {
-        return cl;
+        return pmc;
       }
-      classLoaderCache.remove(rulesId);
+
+      machineCache.remove(rulesId);
       ref.enqueue();
     }
 
-    cleanCache();
+    gc();
 
-    //read jar from (site)/cache/rules
+    PrologMachineCopy pcm = createMachine(project, rulesId);
-    //the included jar file should be in format:
+    MachineRef newRef = new MachineRef(rulesId, pcm, dead);
-    //rules-(rules.pl's sha1).jar
+    machineCache.put(rulesId, newRef);
-    File jarFile = new File(rulesDir, "rules-" + rulesId.getName() + ".jar");
+    return pcm;
-    if (!jarFile.isFile()) {
-      return null;
-    }
-
-    ClassLoader defaultLoader = getClass().getClassLoader();
-    URL url;
-    try {
-      url = jarFile.toURI().toURL();
-    } catch (MalformedURLException e) {
-      log.error("Path to rules jar is broken", e);
-      return null;
-    }
-
-    ClassLoader urlLoader = new URLClassLoader(new URL[]{url}, defaultLoader);
-
-    LoaderRef lRef = new LoaderRef(rulesId, urlLoader);
-    classLoaderCache.put(rulesId, lRef);
-    return urlLoader;
   }
 
-  private void cleanCache() {
+  private void gc() {
-    Reference<? extends ClassLoader> ref;
+    Reference<?> ref;
-    while ((ref = DEAD.poll()) != null) {
+    while ((ref = dead.poll()) != null) {
-      ObjectId key = ((LoaderRef) ref).key;
+      ObjectId key = ((MachineRef) ref).key;
-      if (classLoaderCache.get(key) == ref) {
+      if (machineCache.get(key) == ref) {
-        classLoaderCache.remove(key);
+        machineCache.remove(key);
       }
     }
   }
-}
+
+  private PrologMachineCopy createMachine(Project.NameKey project,
+      ObjectId rulesId) throws CompileException {
+    // If the rules are available as a complied JAR on local disk, prefer
+    // that over dynamic consult as the bytecode will be faster.
+    //
+    if (rulesDir != null) {
+      File jarFile = new File(rulesDir, "rules-" + rulesId.getName() + ".jar");
+      if (jarFile.isFile()) {
+        URL[] cp = new URL[] {toURL(jarFile)};
+        return save(newEmptyMachine(new URLClassLoader(cp, systemLoader)));
+      }
+    }
+
+    // Dynamically consult the rules into the machine's internal database.
+    //
+    String rules = read(project, rulesId);
+    BufferingPrologControl ctl = newEmptyMachine(systemLoader);
+    PushbackReader in = new PushbackReader(
+        new StringReader(rules),
+        Prolog.PUSHBACK_SIZE);
+
+    if (!ctl.execute(
+        Prolog.BUILTIN, "consult_stream",
+        SymbolTerm.intern("rules.pl"),
+        new JavaObjectTerm(in))) {
+      throw new CompileException("Cannot consult rules of " + project);
+    }
+    return save(ctl);
+  }
+
+  private String read(Project.NameKey project, ObjectId rulesId)
+      throws CompileException {
+    Repository git;
+    try {
+      git = gitMgr.openRepository(project);
+    } catch (RepositoryNotFoundException e) {
+      throw new CompileException("Cannot open repository " + project, e);
+    }
+    try {
+      ObjectLoader ldr = git.open(rulesId, Constants.OBJ_BLOB);
+      byte[] raw = ldr.getCachedBytes(SRC_LIMIT);
+      return RawParseUtils.decode(raw);
+    } catch (LargeObjectException e) {
+      throw new CompileException("rules of " + project + " are too large", e);
+    } catch (RuntimeException e) {
+      throw new CompileException("Cannot load rules of " + project, e);
+    } catch (IOException e) {
+      throw new CompileException("Cannot load rules of " + project, e);
+    } finally {
+      git.close();
+    }
+  }
+
+  private static BufferingPrologControl newEmptyMachine(ClassLoader cl) {
+    BufferingPrologControl ctl = new BufferingPrologControl();
+    ctl.setMaxArity(PrologEnvironment.MAX_ARITY);
+    ctl.setMaxDatabaseSize(DB_MAX);
+    ctl.setPrologClassLoader(new PrologClassLoader(cl));
+    ctl.setEnabled(EnumSet.allOf(Prolog.Feature.class), false);
+    return ctl;
+  }
+
+  private static URL toURL(File jarFile) throws CompileException {
+    try {
+      return jarFile.toURI().toURL();
+    } catch (MalformedURLException e) {
+      throw new CompileException("Cannot create URL for " + jarFile, e);
+    }
+  }
+}

gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java

@@ -75,7 +75,6 @@ public class ProjectConfig extends VersionedMetaData {
   private Map<AccountGroup.UUID, GroupReference> groupsByUUID;
   private Map<String, AccessSection> accessSections;
   private List<ValidationError> validationErrors;
-  private String prologRules;
   private ObjectId rulesId;
 
   public static ProjectConfig read(MetaDataUpdate update) throws IOException,
@@ -153,17 +152,6 @@ public class ProjectConfig extends VersionedMetaData {
     return groupsByUUID.get(uuid);
   }
 
-  /**
-   * @return the project's Prolog based rules.pl script,
-   *    if present in the branch. Null if there are no rules.
-   */
-  public String getPrologRules() {
-    if (prologRules.equals("")) {
-      return null;
-    }
-    return prologRules;
-  }
-
   /**
    * @return the project's rules.pl ObjectId, if present in the branch.
    *    Null if it doesn't exist.
@@ -212,7 +200,6 @@ public class ProjectConfig extends VersionedMetaData {
   protected void onLoad() throws IOException, ConfigInvalidException {
     Map<String, GroupReference> groupsByName = readGroupList();
 
-    prologRules = readUTF8("rules.pl");
     rulesId = getObjectId("rules.pl");
     Config rc = readConfig(PROJECT_CONFIG);
     project = new Project(projectName);

gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java

@@ -30,16 +30,12 @@ import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;
 
 import com.googlecode.prolog_cafe.compiler.CompileException;
-import com.googlecode.prolog_cafe.lang.JavaObjectTerm;
+import com.googlecode.prolog_cafe.lang.PrologMachineCopy;
-import com.googlecode.prolog_cafe.lang.Prolog;
-import com.googlecode.prolog_cafe.lang.SymbolTerm;
 
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Repository;
 
 import java.io.IOException;
-import java.io.PushbackReader;
-import java.io.StringReader;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -58,10 +54,13 @@ public class ProjectState {
   private final ProjectControl.AssistedFactory projectControlFactory;
   private final PrologEnvironment.Factory envFactory;
   private final GitRepositoryManager gitMgr;
+  private final RulesCache rulesCache;
 
   private final ProjectConfig config;
   private final Set<AccountGroup.UUID> localOwners;
-  private final ClassLoader ruleLoader;
+
+  /** Prolog rule state. */
+  private transient PrologMachineCopy rulesMachine;
 
   /** Last system time the configuration's revision was examined. */
   private transient long lastCheckTime;
@@ -80,12 +79,8 @@ public class ProjectState {
     this.projectControlFactory = projectControlFactory;
     this.envFactory = envFactory;
     this.gitMgr = gitMgr;
+    this.rulesCache = rulesCache;
     this.config = config;
-    if (rulesCache != null) {
-      ruleLoader = rulesCache.getClassLoader(config.getRulesId());
-    } else {
-      ruleLoader = null;
-    }
 
     HashSet<AccountGroup.UUID> groups = new HashSet<AccountGroup.UUID>();
     AccessSection all = config.getAccessSection(AccessSection.ALL);
@@ -133,26 +128,14 @@ public class ProjectState {
 
   /** @return Construct a new PrologEnvironment for the calling thread. */
   public PrologEnvironment newPrologEnvironment() throws CompileException {
-    if (ruleLoader != null) {
+    PrologMachineCopy pmc = rulesMachine;
-      return envFactory.create(ruleLoader);
+    if (pmc == null) {
+      pmc = rulesCache.loadMachine(
+          getProject().getNameKey(),
+          getConfig().getRulesId());
+      rulesMachine = pmc;
     }
-
+    return envFactory.create(pmc);
-    PrologEnvironment env = envFactory.create(getClass().getClassLoader());
-
-    //consult rules.pl at refs/meta/config branch for custom submit rules
-    String rules = getConfig().getPrologRules();
-    if (rules != null) {
-      PushbackReader in =
-          new PushbackReader(new StringReader(rules), Prolog.PUSHBACK_SIZE);
-      JavaObjectTerm streamObject = new JavaObjectTerm(in);
-      if (!env.execute(Prolog.BUILTIN, "consult_stream",
-          SymbolTerm.intern("rules.pl"), streamObject)) {
-        throw new CompileException("Cannot consult rules.pl " +
-            getProject().getName() + " " + getConfig().getRevision());
-      }
-    }
-
-    return env;
   }
 
   public Project getProject() {

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

@@ -18,8 +18,11 @@ import com.google.inject.Guice;
 import com.google.inject.Module;
 
 import com.googlecode.prolog_cafe.compiler.CompileException;
+import com.googlecode.prolog_cafe.lang.BufferingPrologControl;
 import com.googlecode.prolog_cafe.lang.JavaObjectTerm;
 import com.googlecode.prolog_cafe.lang.Prolog;
+import com.googlecode.prolog_cafe.lang.PrologClassLoader;
+import com.googlecode.prolog_cafe.lang.PrologMachineCopy;
 import com.googlecode.prolog_cafe.lang.StructureTerm;
 import com.googlecode.prolog_cafe.lang.SymbolTerm;
 import com.googlecode.prolog_cafe.lang.Term;
@@ -46,7 +49,8 @@ public abstract class PrologTestCase extends TestCase {
   private boolean hasSetup;
   private boolean hasTeardown;
   private List<Term> tests;
-  protected PrologEnvironment env;
+  private PrologMachineCopy machine;
+  private PrologEnvironment.Factory envFactory;
 
   protected void load(String pkg, String prologResource, Module... modules)
       throws CompileException, IOException {
@@ -54,18 +58,14 @@ public abstract class PrologTestCase extends TestCase {
     moduleList.add(new PrologModule());
     moduleList.addAll(Arrays.asList(modules));
 
-    PrologEnvironment.Factory factory =
+    envFactory = Guice.createInjector(moduleList)
-        Guice.createInjector(moduleList).getInstance(
+        .getInstance(PrologEnvironment.Factory.class);
-            PrologEnvironment.Factory.class);
+    PrologEnvironment env = envFactory.create(newMachine());
-    env = factory.create(getClass().getClassLoader());
+    consult(env, getClass(), prologResource);
-    env.setMaxDatabaseSize(16 * 1024);
-    env.setEnabled(Prolog.Feature.IO, true);
-
-    consult(getClass(), prologResource);
 
     this.pkg = pkg;
-    hasSetup = has("setup");
+    hasSetup = has(env, "setup");
-    hasTeardown = has("teardown");
+    hasTeardown = has(env, "teardown");
 
     StructureTerm head = new StructureTerm(":",
         SymbolTerm.intern(pkg),
@@ -76,10 +76,19 @@ public abstract class PrologTestCase extends TestCase {
       tests.add(pair[0]);
     }
     assertTrue("has tests", tests.size() > 0);
+    machine = PrologMachineCopy.save(env);
   }
 
-  protected void consult(Class<?> clazz, String prologResource)
+  private PrologMachineCopy newMachine() {
-      throws CompileException, IOException {
+    BufferingPrologControl ctl = new BufferingPrologControl();
+    ctl.setMaxDatabaseSize(16 * 1024);
+    ctl.setPrologClassLoader(new PrologClassLoader(getClass().getClassLoader()));
+    return PrologMachineCopy.save(ctl);
+  }
+
+  protected void consult(BufferingPrologControl env,
+      Class<?> clazz,
+      String prologResource) throws CompileException, IOException {
     InputStream in = clazz.getResourceAsStream(prologResource);
     if (in == null) {
       throw new FileNotFoundException(prologResource);
@@ -97,7 +106,7 @@ public abstract class PrologTestCase extends TestCase {
     }
   }
 
-  private boolean has(String name) {
+  private boolean has(BufferingPrologControl env, String name) {
     StructureTerm head = SymbolTerm.create(pkg, name, 0);
     return env.execute(Prolog.BUILTIN, "clause", head, new VariableTerm());
   }
@@ -107,17 +116,20 @@ public abstract class PrologTestCase extends TestCase {
     long start = System.currentTimeMillis();
 
     for (Term test : tests) {
+      PrologEnvironment env = envFactory.create(machine);
+      env.setEnabled(Prolog.Feature.IO, true);
+
       System.out.format("Prolog %-60s ...", removePackage(test));
       System.out.flush();
 
       if (hasSetup) {
-        call("setup");
+        call(env, "setup");
       }
 
       List<Term> all = env.all(Prolog.BUILTIN, "call", test);
 
       if (hasTeardown) {
-        call("teardown");
+        call(env, "teardown");
       }
 
       System.out.println(all.size() == 1 ? "OK" : "FAIL");
@@ -152,7 +164,7 @@ public abstract class PrologTestCase extends TestCase {
     assertEquals("No Errors", 0, errors);
   }
 
-  private void call(String name) {
+  private void call(BufferingPrologControl env, String name) {
     StructureTerm head = SymbolTerm.create(pkg, name, 0);
     if (!env.execute(Prolog.BUILTIN, "call", head)) {
       fail("Cannot invoke " + pkg + ":" + name);