Version DiffSummary cache separately from PatchList cache

One of the reasons for the existence of the DiffSummary cache is that
it is conceptually simpler than the PatchList cache, and depends on
fewer implementation details that are likely to change. Prior to this
change, an update to the PatchList serialVersionUID will cause the
DiffSummary cache to flush, in turn slowing down the next full site
reindex.

We can avoid this by using a separate DiffSummaryKey, which is based
upon the current PatchListKey implementation, but is versioned
independently.

Change-Id: Ic5ab2db58842a2bf7ed266bb0b5b926421e2c145

gerrit-server/src/main/java/com/google/gerrit/server/patch/DiffSummary.java

@@ -30,7 +30,7 @@ import java.util.zip.DeflaterOutputStream;
 import java.util.zip.InflaterInputStream;
 
 public class DiffSummary implements Serializable {
-  private static final long serialVersionUID = PatchListKey.serialVersionUID;
+  private static final long serialVersionUID = DiffSummaryKey.serialVersionUID;
 
   private transient String[] paths;
 

gerrit-server/src/main/java/com/google/gerrit/server/patch/DiffSummaryKey.java

@@ -0,0 +1,117 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.patch;
+
+import static org.eclipse.jgit.lib.ObjectIdSerialization.readCanBeNull;
+import static org.eclipse.jgit.lib.ObjectIdSerialization.readNotNull;
+import static org.eclipse.jgit.lib.ObjectIdSerialization.writeCanBeNull;
+import static org.eclipse.jgit.lib.ObjectIdSerialization.writeNotNull;
+
+import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
+
+import org.eclipse.jgit.lib.ObjectId;
+
+import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
+import java.io.Serializable;
+import java.util.Objects;
+
+public class DiffSummaryKey implements Serializable {
+  public static final long serialVersionUID = 1L;
+
+  /** see PatchListKey#oldId */
+  private transient ObjectId oldId;
+
+  /** see PatchListKey#parentNum */
+  private transient Integer parentNum;
+
+  private transient ObjectId newId;
+  private transient Whitespace whitespace;
+
+  public static DiffSummaryKey fromPatchListKey(PatchListKey plk) {
+    return new DiffSummaryKey(plk.getOldId(), plk.getParentNum(),
+        plk.getNewId(), plk.getWhitespace());
+  }
+
+  private DiffSummaryKey(ObjectId oldId, Integer parentNum, ObjectId newId,
+      Whitespace whitespace) {
+    this.oldId = oldId;
+    this.parentNum = parentNum;
+    this.newId = newId;
+    this.whitespace = whitespace;
+  }
+
+  PatchListKey toPatchListKey() {
+    return new PatchListKey(oldId, parentNum, newId, whitespace);
+  }
+
+  @Override
+  public int hashCode() {
+    return Objects.hash(oldId, parentNum, newId, whitespace);
+  }
+
+  @Override
+  public boolean equals(final Object o) {
+    if (o instanceof DiffSummaryKey) {
+      DiffSummaryKey k = (DiffSummaryKey) o;
+      return Objects.equals(oldId, k.oldId)
+          && Objects.equals(parentNum, k.parentNum)
+          && Objects.equals(newId, k.newId)
+          && whitespace == k.whitespace;
+    }
+    return false;
+  }
+
+  @Override
+  public String toString() {
+    StringBuilder n = new StringBuilder();
+    n.append("DiffSummaryKey[");
+    n.append(oldId != null ? oldId.name() : "BASE");
+    n.append("..");
+    n.append(newId.name());
+    n.append(" ");
+    if (parentNum != null) {
+      n.append(parentNum);
+      n.append(" ");
+    }
+    n.append(whitespace.name());
+    n.append("]");
+    return n.toString();
+  }
+
+  private void writeObject(final ObjectOutputStream out) throws IOException {
+    writeCanBeNull(out, oldId);
+    out.writeInt(parentNum == null ? 0 : parentNum);
+    writeNotNull(out, newId);
+    Character c = PatchListKey.WHITESPACE_TYPES.get(whitespace);
+    if (c == null) {
+      throw new IOException("Invalid whitespace type: " + whitespace);
+    }
+    out.writeChar(c);
+  }
+
+  private void readObject(final ObjectInputStream in) throws IOException {
+    oldId = readCanBeNull(in);
+    int n = in.readInt();
+    parentNum = n == 0 ? null : Integer.valueOf(n);
+    newId = readNotNull(in);
+    char t = in.readChar();
+    whitespace = PatchListKey.WHITESPACE_TYPES.inverse().get(t);
+    if (whitespace == null) {
+      throw new IOException("Invalid whitespace type code: " + t);
+    }
+  }
+}

gerrit-server/src/main/java/com/google/gerrit/server/patch/DiffSummaryLoader.java

@@ -31,16 +31,16 @@ public class DiffSummaryLoader implements Callable<DiffSummary> {
   static final Logger log = LoggerFactory.getLogger(DiffSummaryLoader.class);
 
   public interface Factory {
-    DiffSummaryLoader create(PatchListKey key, Project.NameKey project);
+    DiffSummaryLoader create(DiffSummaryKey key, Project.NameKey project);
   }
 
   private final PatchListCache patchListCache;
-  private final PatchListKey key;
+  private final DiffSummaryKey key;
   private final Project.NameKey project;
 
   @AssistedInject
   DiffSummaryLoader(PatchListCache plc,
-      @Assisted PatchListKey k,
+      @Assisted DiffSummaryKey k,
       @Assisted Project.NameKey p) {
     patchListCache = plc;
     key = k;
@@ -49,7 +49,7 @@ public class DiffSummaryLoader implements Callable<DiffSummary> {
 
   @Override
   public DiffSummary call() throws Exception {
-    PatchList patchList = patchListCache.get(key, project);
+    PatchList patchList = patchListCache.get(key.toPatchListKey(), project);
     return toDiffSummary(patchList);
   }
 

gerrit-server/src/main/java/com/google/gerrit/server/patch/DiffSummaryWeigher.java

@@ -17,11 +17,12 @@ package com.google.gerrit.server.patch;
 import com.google.common.cache.Weigher;
 
 /** Computes memory usage for {@link DiffSummary} in bytes of memory used. */
-public class DiffSummaryWeigher implements Weigher<PatchListKey, DiffSummary> {
+public class DiffSummaryWeigher implements
+    Weigher<DiffSummaryKey, DiffSummary> {
 
   @Override
-  public int weigh(PatchListKey key, DiffSummary value) {
-    int size = 16 + 4 * 8 + 2 * 36; // Size of PatchListKey, 64 bit JVM
+  public int weigh(DiffSummaryKey key, DiffSummary value) {
+    int size = 16 + 4 * 8 + 2 * 36; // Size of DiffSummaryKey, 64 bit JVM
 
     // Size of the list of paths ...
     for (String p : value.getPaths()) {

gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java

@@ -58,7 +58,7 @@ public class PatchListCacheImpl implements PatchListCache {
             .weigher(IntraLineWeigher.class);
 
         factory(DiffSummaryLoader.Factory.class);
-        persist(DIFF_SUMMARY, PatchListKey.class, DiffSummary.class)
+        persist(DIFF_SUMMARY, DiffSummaryKey.class, DiffSummary.class)
             .maximumWeight(10 << 20)
             .weigher(DiffSummaryWeigher.class)
             .diskLimit(1 << 30);
@@ -71,7 +71,7 @@ public class PatchListCacheImpl implements PatchListCache {
 
   private final Cache<PatchListKey, PatchList> fileCache;
   private final Cache<IntraLineDiffKey, IntraLineDiff> intraCache;
-  private final Cache<PatchListKey, DiffSummary> diffSummaryCache;
+  private final Cache<DiffSummaryKey, DiffSummary> diffSummaryCache;
   private final PatchListLoader.Factory fileLoaderFactory;
   private final IntraLineLoader.Factory intraLoaderFactory;
   private final DiffSummaryLoader.Factory diffSummaryLoaderFactory;
@@ -81,7 +81,7 @@ public class PatchListCacheImpl implements PatchListCache {
   PatchListCacheImpl(
       @Named(FILE_NAME) Cache<PatchListKey, PatchList> fileCache,
       @Named(INTRA_NAME) Cache<IntraLineDiffKey, IntraLineDiff> intraCache,
-      @Named(DIFF_SUMMARY) Cache<PatchListKey, DiffSummary> diffSummaryCache,
+      @Named(DIFF_SUMMARY) Cache<DiffSummaryKey, DiffSummary> diffSummaryCache,
       PatchListLoader.Factory fileLoaderFactory,
       IntraLineLoader.Factory intraLoaderFactory,
       DiffSummaryLoader.Factory diffSummaryLoaderFactory,
@@ -103,7 +103,9 @@ public class PatchListCacheImpl implements PatchListCache {
       throws PatchListNotAvailableException {
     try {
       PatchList pl = fileCache.get(key, fileLoaderFactory.create(key, project));
-      diffSummaryCache.put(key, toDiffSummary(pl));
+      diffSummaryCache.put(
+          DiffSummaryKey.fromPatchListKey(key),
+          toDiffSummary(pl));
       return pl;
     } catch (ExecutionException e) {
       PatchListLoader.log.warn("Error computing " + key, e);
@@ -164,11 +166,14 @@ public class PatchListCacheImpl implements PatchListCache {
     Project.NameKey project = change.getProject();
     ObjectId b = ObjectId.fromString(patchSet.getRevision().get());
     Whitespace ws = Whitespace.IGNORE_NONE;
-    return getDiffSummary(PatchListKey.againstDefaultBase(b, ws), project);
+    return getDiffSummary(
+        DiffSummaryKey.fromPatchListKey(
+            PatchListKey.againstDefaultBase(b, ws)),
+        project);
   }
 
-  private DiffSummary getDiffSummary(PatchListKey key, Project.NameKey project)
-      throws PatchListNotAvailableException {
+  private DiffSummary getDiffSummary(DiffSummaryKey key,
+      Project.NameKey project) throws PatchListNotAvailableException {
     try {
       return diffSummaryCache.get(key,
           diffSummaryLoaderFactory.create(key, project));
@@ -183,4 +188,4 @@ public class PatchListCacheImpl implements PatchListCache {
       throw e;
     }
   }
-}
+}

gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListKey.java

@@ -92,6 +92,15 @@ public class PatchListKey implements Serializable {
     whitespace = ws;
   }
 
+  /** For use only by DiffSummaryKey. */
+  PatchListKey(ObjectId oldId, Integer parentNum, ObjectId newId,
+      Whitespace whitespace) {
+    this.oldId = oldId;
+    this.parentNum = parentNum;
+    this.newId = newId;
+    this.whitespace = whitespace;
+  }
+
   /** Old side commit, or null to assume ancestor or combined merge. */
   @Nullable
   public ObjectId getOldId() {