RulesCache: Replace manual weak cache with regular Guava cache

Previously, PrologMachineCopy instances were stored with weak references
in RulesCache's internal cache, which had no other size constraints.
Instances were copied as hard references to ProjectState on demand,
meaning we would only spend memory on PrologMachineCopy instances that
were active in a project we care about in the cache. This was an awkward
and non-obvious two-layer cache.

We can achieve very similar memory performance if we simply convert
RulesCache to a standard Guava cache with hard references, sized to the
number of projects in the cache. Typically, we will store up to one
PrologMachineCopy per project in the cache, and the project cache is
generally sized to the total number of projects in the site. We can let
them expire out of the RulesCache in normal LRU fashion.

Under this implementation, it's possible we will hold on to stale rules
longer than necessary, particularly if rules.pl changes frequently and
the project cache is sized much larger than the total number of
projects. This could be avoided if we convert RulesCache to a
Cache<Project.NameKey, Pair<ObjectId, PrologMachineCopy>> and revalidate
the ObjectId on load; but at this point, that's a premature
optimization.

Tie the configuration of the RulesCache to the ProjectCache, so that it
is sized appropriately. This means that it can't be configured
independently, but we still need to at least mention it in the
documentation, since it will show up in the list of caches exposed to
administrators at runtime.

Change-Id: I1d3e3c013c30dd6d8a2a5d12f45a8e276d6051cd

Documentation/config-gerrit.txt

@@ -942,6 +942,11 @@ in the database.  If a project record is updated or deleted, this
 cache should be flushed.  Newly inserted projects do not require
 a cache flush, as they will be read upon first reference.
 
+cache `"prolog_rules"`::
++
+Caches parsed `rules.pl` contents for each project. This cache uses the same
+size as the `projects` cache, and cannot be configured independently.
+
 cache `"sshkeys"`::
 +
 Caches unpacked versions of user SSH keys, so the internal SSH daemon

Documentation/rest-api-config.txt

@@ -446,6 +446,16 @@ The entries in the map are sorted by cache name.
         "mem": 99
       }
     },
+    "prolog_rules": {
+      "type": "MEM",
+      "entries": {
+        "mem": 35
+      },
+      "average_get": "103.0ms",
+      "hit_ratio": {
+        "mem": 99
+      }
+    },
     "quota-repo_size": {
       "type": "DISK",
       "entries": {
@@ -516,6 +526,7 @@ The cache names are lexicographically sorted.
     "plugin_resources",
     "project_list",
     "projects",
+    "prolog_rules",
     "quota-repo_size",
     "sshkeys",
     "web_sessions"

java/com/google/gerrit/server/project/ProjectCacheImpl.java

@@ -52,7 +52,8 @@ import org.slf4j.LoggerFactory;
 public class ProjectCacheImpl implements ProjectCache {
   private static final Logger log = LoggerFactory.getLogger(ProjectCacheImpl.class);
 
-  private static final String CACHE_NAME = "projects";
+  public static final String CACHE_NAME = "projects";
+
   private static final String CACHE_LIST = "project_list";
 
   public static Module module() {

java/com/google/gerrit/server/rules/PrologModule.java

@@ -22,6 +22,7 @@ public class PrologModule extends FactoryModule {
   @Override
   protected void configure() {
     install(new EnvironmentModule());
+    install(new RulesCache.Module());
     bind(PrologEnvironment.Args.class);
     factory(PrologRuleEvaluator.Factory.class);
 

java/com/google/gerrit/server/rules/RulesCache.java

@@ -17,15 +17,19 @@ package com.google.gerrit.server.rules;
 import static com.googlecode.prolog_cafe.lang.PrologMachineCopy.save;
 
 import com.google.common.base.Joiner;
+import com.google.common.cache.Cache;
 import com.google.common.collect.ImmutableList;
 import com.google.gerrit.extensions.registration.DynamicSet;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.cache.CacheModule;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.SitePaths;
 import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.project.ProjectCacheImpl;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
+import com.google.inject.name.Named;
 import com.googlecode.prolog_cafe.exceptions.CompileException;
 import com.googlecode.prolog_cafe.exceptions.SyntaxException;
 import com.googlecode.prolog_cafe.exceptions.TermException;
@@ -42,9 +46,6 @@ import java.io.IOException;
 import java.io.PushbackReader;
 import java.io.Reader;
 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;
@@ -52,9 +53,8 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.EnumSet;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
+import java.util.concurrent.ExecutionException;
 import org.eclipse.jgit.errors.LargeObjectException;
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.Constants;
@@ -71,17 +71,19 @@ import org.eclipse.jgit.util.RawParseUtils;
  */
 @Singleton
 public class RulesCache {
+  public static class Module extends CacheModule {
+    @Override
+    protected void configure() {
+      cache(RulesCache.CACHE_NAME, ObjectId.class, PrologMachineCopy.class)
+          // This cache is auxiliary to the project cache, so size it the same.
+          .configKey(ProjectCacheImpl.CACHE_NAME);
+    }
+  }
+
   private static final ImmutableList<String> PACKAGE_LIST =
       ImmutableList.of(Prolog.BUILTIN, "gerrit");
 
-  private static final class MachineRef extends WeakReference<PrologMachineCopy> {
-    final ObjectId key;
-
-    MachineRef(ObjectId key, PrologMachineCopy pcm, ReferenceQueue<PrologMachineCopy> queue) {
-      super(pcm, queue);
-      this.key = key;
-    }
-  }
+  static final String CACHE_NAME = "prolog_rules";
 
   private final boolean enableProjectRules;
   private final int maxDbSize;
@@ -92,15 +94,15 @@ public class RulesCache {
   private final DynamicSet<PredicateProvider> predicateProviders;
   private final ClassLoader systemLoader;
   private final PrologMachineCopy defaultMachine;
-  private final Map<ObjectId, MachineRef> machineCache = new HashMap<>();
-  private final ReferenceQueue<PrologMachineCopy> dead = new ReferenceQueue<>();
+  private final Cache<ObjectId, PrologMachineCopy> machineCache;
 
   @Inject
   protected RulesCache(
       @GerritServerConfig Config config,
       SitePaths site,
       GitRepositoryManager gm,
-      DynamicSet<PredicateProvider> predicateProviders) {
+      DynamicSet<PredicateProvider> predicateProviders,
+      @Named(CACHE_NAME) Cache<ObjectId, PrologMachineCopy> machineCache) {
     maxDbSize = config.getInt("rules", null, "maxPrologDatabaseSize", 256);
     maxSrcBytes = config.getInt("rules", null, "maxSourceBytes", 128 << 10);
     enableProjectRules = config.getBoolean("rules", null, "enable", true) && maxSrcBytes > 0;
@@ -108,6 +110,7 @@ public class RulesCache {
     rulesDir = cacheDir != null ? cacheDir.resolve("rules") : null;
     gitMgr = gm;
     this.predicateProviders = predicateProviders;
+    this.machineCache = machineCache;
 
     systemLoader = getClass().getClassLoader();
     defaultMachine = save(newEmptyMachine(systemLoader));
@@ -129,23 +132,14 @@ public class RulesCache {
       return defaultMachine;
     }
 
-    Reference<? extends PrologMachineCopy> ref = machineCache.get(rulesId);
-    if (ref != null) {
-      PrologMachineCopy pmc = ref.get();
-      if (pmc != null) {
-        return pmc;
+    try {
+      return machineCache.get(rulesId, () -> createMachine(project, rulesId));
+    } catch (ExecutionException e) {
+      if (e.getCause() instanceof CompileException) {
+        throw new CompileException(e.getCause().getMessage(), e);
       }
-
-      machineCache.remove(rulesId);
-      ref.enqueue();
+      throw new CompileException("Error while consulting rules from " + project, e);
     }
-
-    gc();
-
-    PrologMachineCopy pcm = createMachine(project, rulesId);
-    MachineRef newRef = new MachineRef(rulesId, pcm, dead);
-    machineCache.put(rulesId, newRef);
-    return pcm;
   }
 
   public PrologMachineCopy loadMachine(String name, Reader in) throws CompileException {
@@ -156,16 +150,6 @@ public class RulesCache {
     return pmc;
   }
 
-  private void gc() {
-    Reference<?> ref;
-    while ((ref = dead.poll()) != null) {
-      ObjectId key = ((MachineRef) ref).key;
-      if (machineCache.get(key) == ref) {
-        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