From a5504b0ca0f02ad618be993ed98d1c9022cca336 Mon Sep 17 00:00:00 2001 From: Alice Kober-Sotzek Date: Wed, 14 Jun 2017 15:01:33 +0200 Subject: [PATCH] Fix bug in computation of hunks due to rebase Combining the optimization which omits files from the diff result containing only hunks due to rebase with the recursive calling of the diff computation didn't work well. The logic assumed that all differences between the parent commits were returned. Due to the optimization, only some of them were returned and hence only some hunks due to rebase could be marked. As the optimization was only applied under specific circumstances, this issue didn't occur for every diff between patch sets. In particular, it was observable when a change was based on another change. To resolve this issue, a parameter is introduced which influences whether optimizations might be applied. All code paths from external calls continue to apply those optimizations if applicable. When the differences between parent commits is computed for the hunks due to rebase, the unmodified differences between their trees is explicitly requested. As the recursion is thus cut short, the expensive call to RevWalk#isMergedInto can be removed from the check which determines whether the edits due to rebase are computed. Even though the DiffSummaryCache is supposed to be fairly independent from changes in PatchList and PatchListKey, its implementation is coupled to them. As the change in the diffing logic shouldn't influence the DiffSummaryCache, this change tries to work around it. Change-Id: I9bf31dec18d6dd357c17c640fa0a940808e97269 --- .../api/revision/RevisionDiffIT.java | 56 +++++++++- .../server/change/PatchListCacheIT.java | 2 +- .../gerrit/server/change/FileInfoJson.java | 2 +- .../gerrit/server/patch/DiffSummaryKey.java | 5 +- .../server/patch/PatchListCacheImpl.java | 4 +- .../gerrit/server/patch/PatchListEntry.java | 1 + .../gerrit/server/patch/PatchListKey.java | 56 ++++++++-- .../gerrit/server/patch/PatchListLoader.java | 104 +++++++----------- .../gerrit/server/patch/PatchListWeigher.java | 3 +- .../server/patch/PatchScriptFactory.java | 2 +- 10 files changed, 151 insertions(+), 84 deletions(-) diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java index 0ae78f693a..48030eb3a2 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java @@ -887,17 +887,28 @@ public class RevisionDiffIT extends AbstractDaemonTest { assertThat(changedFiles.get(renamedFilePath)).linesDeleted().isEqualTo(1); } + /* + * change PS B + * | + * change PS A commit4 + * | | + * commit2 commit3 + * | / + * commit1 -------- + */ @Test - public void rebaseHunksWhenRebasingOnAnotherChangeAreIdentified() throws Exception { + public void rebaseHunksWhenRebasingOnAnotherChangeOrPatchSetAreIdentified() throws Exception { ObjectId commit2 = addCommit(commit1, FILE_NAME, FILE_CONTENT.replace("Line 5\n", "Line five\n")); rebaseChangeOn(changeId, commit2); String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision; - ObjectId commit3 = - addCommit(commit1, FILE_NAME, FILE_CONTENT.replace("Line 35\n", "Line thirty five\n")); + String commit3FileContent = FILE_CONTENT.replace("Line 35\n", "Line thirty five\n"); + ObjectId commit3 = addCommit(commit1, FILE_NAME, commit3FileContent); + ObjectId commit4 = + addCommit(commit3, FILE_NAME, commit3FileContent.replace("Line 60\n", "Line sixty\n")); - rebaseChangeOn(changeId, commit3); + rebaseChangeOn(changeId, commit4); Function contentModification = fileContent -> fileContent.replace("Line 20\n", "Line twenty\n"); addModifiedPatchSet(changeId, FILE_NAME, contentModification); @@ -916,7 +927,11 @@ public class RevisionDiffIT extends AbstractDaemonTest { assertThat(diffInfo).content().element(5).linesOfA().containsExactly("Line 35"); assertThat(diffInfo).content().element(5).linesOfB().containsExactly("Line thirty five"); assertThat(diffInfo).content().element(5).isDueToRebase(); - assertThat(diffInfo).content().element(6).commonLines().hasSize(65); + assertThat(diffInfo).content().element(6).commonLines().hasSize(24); + assertThat(diffInfo).content().element(7).linesOfA().containsExactly("Line 60"); + assertThat(diffInfo).content().element(7).linesOfB().containsExactly("Line sixty"); + assertThat(diffInfo).content().element(7).isDueToRebase(); + assertThat(diffInfo).content().element(8).commonLines().hasSize(40); Map changedFiles = gApi.changes().id(changeId).current().files(previousPatchSetId); @@ -924,6 +939,37 @@ public class RevisionDiffIT extends AbstractDaemonTest { assertThat(changedFiles.get(FILE_NAME)).linesDeleted().isEqualTo(1); } + /* + * change PS B + * | + * change PS A commit4 + * | | + * commit2 commit3 + * | / + * commit1 -------- + */ + @Test + public void unrelatedFileWhenRebasingOnAnotherChangeOrPatchSetIsIgnored() throws Exception { + ObjectId commit2 = + addCommit(commit1, FILE_NAME, FILE_CONTENT.replace("Line 5\n", "Line five\n")); + rebaseChangeOn(changeId, commit2); + String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision; + + ObjectId commit3 = + addCommit(commit1, FILE_NAME2, FILE_CONTENT2.replace("2nd line\n", "Second line\n")); + ObjectId commit4 = + addCommit(commit3, FILE_NAME, FILE_CONTENT.replace("Line 60\n", "Line sixty\n")); + + rebaseChangeOn(changeId, commit4); + Function contentModification = + fileContent -> fileContent.replace("Line 20\n", "Line twenty\n"); + addModifiedPatchSet(changeId, FILE_NAME, contentModification); + + Map changedFiles = + gApi.changes().id(changeId).current().files(previousPatchSetId); + assertThat(changedFiles.keySet()).containsExactly(COMMIT_MSG, FILE_NAME); + } + @Test public void rebaseHunksWhenReversingPatchSetOrderAreIdentified() throws Exception { ObjectId commit2 = diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/PatchListCacheIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/PatchListCacheIT.java index 05acd5bdb7..3964253121 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/PatchListCacheIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/PatchListCacheIT.java @@ -234,7 +234,7 @@ public class PatchListCacheIT extends AbstractDaemonTest { } private PatchListKey getKey(ObjectId revisionIdA, ObjectId revisionIdB) { - return new PatchListKey(revisionIdA, revisionIdB, Whitespace.IGNORE_NONE); + return PatchListKey.againstCommit(revisionIdA, revisionIdB, Whitespace.IGNORE_NONE); } private ObjectId getCurrentRevisionId(String changeId) throws Exception { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/FileInfoJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/FileInfoJson.java index b25b588bb5..6ccd460055 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/FileInfoJson.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/FileInfoJson.java @@ -55,7 +55,7 @@ public class FileInfoJson { Map toFileInfoMap(Change change, ObjectId objectId, @Nullable PatchSet base) throws PatchListNotAvailableException { ObjectId a = (base == null) ? null : ObjectId.fromString(base.getRevision().get()); - return toFileInfoMap(change, new PatchListKey(a, objectId, Whitespace.IGNORE_NONE)); + return toFileInfoMap(change, PatchListKey.againstCommit(a, objectId, Whitespace.IGNORE_NONE)); } Map toFileInfoMap(Change change, RevId revision, int parent) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/DiffSummaryKey.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/DiffSummaryKey.java index 8ce4dd33fe..0a02e36a78 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/DiffSummaryKey.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/DiffSummaryKey.java @@ -19,6 +19,7 @@ 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.common.base.Preconditions; import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace; import java.io.IOException; import java.io.ObjectInputStream; @@ -40,6 +41,7 @@ public class DiffSummaryKey implements Serializable { private transient Whitespace whitespace; public static DiffSummaryKey fromPatchListKey(PatchListKey plk) { + Preconditions.checkArgument(plk.getAlgorithm() == PatchListKey.Algorithm.OPTIMIZED_DIFF); return new DiffSummaryKey( plk.getOldId(), plk.getParentNum(), plk.getNewId(), plk.getWhitespace()); } @@ -52,7 +54,8 @@ public class DiffSummaryKey implements Serializable { } PatchListKey toPatchListKey() { - return new PatchListKey(oldId, parentNum, newId, whitespace); + return new PatchListKey( + oldId, parentNum, newId, whitespace, PatchListKey.Algorithm.OPTIMIZED_DIFF); } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java index edff29389a..4a6e01faec 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java @@ -101,7 +101,9 @@ public class PatchListCacheImpl implements PatchListCache { throws PatchListNotAvailableException { try { PatchList pl = fileCache.get(key, fileLoaderFactory.create(key, project)); - diffSummaryCache.put(DiffSummaryKey.fromPatchListKey(key), toDiffSummary(pl)); + if (key.getAlgorithm() == PatchListKey.Algorithm.OPTIMIZED_DIFF) { + diffSummaryCache.put(DiffSummaryKey.fromPatchListKey(key), toDiffSummary(pl)); + } return pl; } catch (ExecutionException e) { PatchListLoader.log.warn("Error computing " + key, e); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListEntry.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListEntry.java index c99d0d7f46..96f66f6f8a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListEntry.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListEntry.java @@ -159,6 +159,7 @@ public class PatchListEntry { size += stringSize(newName); size += header.length; size += (8 + 16 + 4 * 4) * edits.size(); + size += (8 + 16 + 4 * 4) * editsDueToRebase.size(); return size; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListKey.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListKey.java index 6fbdd482c7..abf4d76f74 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListKey.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListKey.java @@ -32,7 +32,17 @@ import org.eclipse.jgit.lib.AnyObjectId; import org.eclipse.jgit.lib.ObjectId; public class PatchListKey implements Serializable { - public static final long serialVersionUID = 25L; + public static final long serialVersionUID = 26L; + + public enum Algorithm { + PURE_TREE_DIFF, + OPTIMIZED_DIFF + } + + private static final ImmutableBiMap ALGORITHM_TYPES = + ImmutableBiMap.of( + Algorithm.PURE_TREE_DIFF, 'T', + Algorithm.OPTIMIZED_DIFF, 'O'); public static final ImmutableBiMap WHITESPACE_TYPES = ImmutableBiMap.of( @@ -43,14 +53,25 @@ public class PatchListKey implements Serializable { static { checkState(WHITESPACE_TYPES.size() == Whitespace.values().length); + checkState(ALGORITHM_TYPES.size() == Algorithm.values().length); } public static PatchListKey againstDefaultBase(AnyObjectId newId, Whitespace ws) { - return new PatchListKey(null, newId, ws); + return new PatchListKey(null, newId, ws, Algorithm.OPTIMIZED_DIFF); } public static PatchListKey againstParentNum(int parentNum, AnyObjectId newId, Whitespace ws) { - return new PatchListKey(parentNum, newId, ws); + return new PatchListKey(parentNum, newId, ws, Algorithm.OPTIMIZED_DIFF); + } + + public static PatchListKey againstCommit( + AnyObjectId otherCommitId, AnyObjectId newId, Whitespace whitespace) { + return new PatchListKey(otherCommitId, newId, whitespace, Algorithm.OPTIMIZED_DIFF); + } + + public static PatchListKey againstCommitWithPureTreeDiff( + AnyObjectId otherCommitId, AnyObjectId newId, Whitespace whitespace) { + return new PatchListKey(otherCommitId, newId, whitespace, Algorithm.PURE_TREE_DIFF); } /** @@ -75,25 +96,34 @@ public class PatchListKey implements Serializable { private transient ObjectId newId; private transient Whitespace whitespace; + private transient Algorithm algorithm; - public PatchListKey(AnyObjectId a, AnyObjectId b, Whitespace ws) { + private PatchListKey(AnyObjectId a, AnyObjectId b, Whitespace ws, Algorithm algorithm) { oldId = a != null ? a.copy() : null; newId = b.copy(); whitespace = ws; + this.algorithm = algorithm; } - private PatchListKey(int parentNum, AnyObjectId b, Whitespace ws) { + private PatchListKey(int parentNum, AnyObjectId b, Whitespace ws, Algorithm algorithm) { this.parentNum = Integer.valueOf(parentNum); newId = b.copy(); whitespace = ws; + this.algorithm = algorithm; } /** For use only by DiffSummaryKey. */ - PatchListKey(ObjectId oldId, Integer parentNum, ObjectId newId, Whitespace whitespace) { + PatchListKey( + ObjectId oldId, + Integer parentNum, + ObjectId newId, + Whitespace whitespace, + Algorithm algorithm) { this.oldId = oldId; this.parentNum = parentNum; this.newId = newId; this.whitespace = whitespace; + this.algorithm = algorithm; } /** Old side commit, or null to assume ancestor or combined merge. */ @@ -117,9 +147,13 @@ public class PatchListKey implements Serializable { return whitespace; } + public Algorithm getAlgorithm() { + return algorithm; + } + @Override public int hashCode() { - return Objects.hash(oldId, parentNum, newId, whitespace); + return Objects.hash(oldId, parentNum, newId, whitespace, algorithm); } @Override @@ -129,7 +163,8 @@ public class PatchListKey implements Serializable { return Objects.equals(oldId, k.oldId) && Objects.equals(parentNum, k.parentNum) && Objects.equals(newId, k.newId) - && whitespace == k.whitespace; + && whitespace == k.whitespace + && algorithm == k.algorithm; } return false; } @@ -147,6 +182,8 @@ public class PatchListKey implements Serializable { n.append(" "); } n.append(whitespace.name()); + n.append(" "); + n.append(algorithm.name()); n.append("]"); return n.toString(); } @@ -160,6 +197,7 @@ public class PatchListKey implements Serializable { throw new IOException("Invalid whitespace type: " + whitespace); } out.writeChar(c); + out.writeChar(ALGORITHM_TYPES.get(algorithm)); } private void readObject(ObjectInputStream in) throws IOException { @@ -172,5 +210,7 @@ public class PatchListKey implements Serializable { if (whitespace == null) { throw new IOException("Invalid whitespace type code: " + t); } + char algorithmCharacter = in.readChar(); + algorithm = ALGORITHM_TYPES.inverse().get(algorithmCharacter); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java index 25325b7a31..b178165e75 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java @@ -147,15 +147,6 @@ public class PatchListLoader implements Callable { return save ? repo.newObjectInserter() : new InMemoryInserter(repo); } - /** - * Computes the {@code PatchList} for a given {@code PatchListKey}. - * - *

Warning: This method may reset the specified {@code RevWalk}. Don't call it in the - * middle of a walk. - * - * @throws IOException if the repository can't be accessed - * @throws PatchListNotAvailableException if the {@code PatchList} can't be computed - */ private PatchList readPatchList(Repository repo, RevWalk rw, ObjectInserter ins) throws IOException, PatchListNotAvailableException { ObjectReader reader = rw.getObjectReader(); @@ -205,13 +196,11 @@ public class PatchListLoader implements Callable { comparisonType)); } Multimap editsDueToRebasePerFilePath = - getEditsDueToRebasePerFilePath(rw, b, aCommit); + key.getAlgorithm() == PatchListKey.Algorithm.OPTIMIZED_DIFF + ? getEditsDueToRebasePerFilePath(aCommit, b) + : ImmutableMultimap.of(); for (DiffEntry diffEntry : diffEntries) { - String filePath = diffEntry.getNewPath(); - if (diffEntry.getChangeType() == ChangeType.DELETE) { - filePath = diffEntry.getOldPath(); - } - Set editsDueToRebase = ImmutableSet.copyOf(editsDueToRebasePerFilePath.get(filePath)); + Set editsDueToRebase = getEditsDueToRebase(editsDueToRebasePerFilePath, diffEntry); Optional patchListEntry = getPatchListEntry(reader, df, diffEntry, aTree, bTree, editsDueToRebase); patchListEntry.ifPresent(entries::add); @@ -240,16 +229,7 @@ public class PatchListLoader implements Callable { * *

    *
  • If {@code commitA} is an ancestor of {@code commitB} (or the other way around), {@code - * commitA} (or {@code commitB}) is used instead of its parent in {@link - * #getEditsDueToRebasePerFilePath(RevCommit, RevCommit)}. - *
  • The method {@link #arePatchSetCommitsWithRebaseInBetween(RevWalk, RevCommit, RevCommit)} - * is adjusted to only short-circuit this method if {@code commitA} is the parent of {@code - * commitB} (or the other way around). - *
  • A flag is added to {@link PatchListKey} indicating whether this method should be called. - * As this method calls {@link PatchListCache#get(PatchListKey, Project.NameKey)} (which - * will end up in this method again), we have to make sure that this method doesn't recurse - * until a parent/child pair of commits (or the root of the history) is reached. Introducing - * a flag would be the simplest approach but there certainly are other ways, too. + * commitA} (or {@code commitB}) is used instead of its parent in this method. *
  • Special handling for merge commits is added. If only one of them is a merge commit, the * whole computation has to be done between the single parent and all parents of the merge * commit. If both of them are merge commits, all combinations of parents have to be @@ -257,56 +237,24 @@ public class PatchListLoader implements Callable { * (or just for specific types of merge commits). *
* - * @param revWalk a {@code RevWalk} for the repository * @param commitA the commit defining {@code treeA} * @param commitB the commit defining {@code treeB} * @return the {@code Edit}s per file path they modify in {@code treeB} - * @throws IOException if the repository can't be accessed * @throws PatchListNotAvailableException if the {@code Edit}s can't be identified */ private Multimap getEditsDueToRebasePerFilePath( - RevWalk revWalk, RevCommit commitB, RevCommit commitA) - throws IOException, PatchListNotAvailableException { - if (!arePatchSetCommitsWithRebaseInBetween(revWalk, commitA, commitB)) { + RevCommit commitA, RevCommit commitB) throws PatchListNotAvailableException { + if (commitA == null + || isRootOrMergeCommit(commitA) + || isRootOrMergeCommit(commitB) + || areParentChild(commitA, commitB) + || haveCommonParent(commitA, commitB)) { return ImmutableMultimap.of(); } - return getEditsDueToRebasePerFilePath(commitA, commitB); - } - /** - * Indicates whether {@code commitA} and {@code commitB} represent two patch sets separated by a - * rebase provided the below-mentioned assumption is met. - * - *

Warning: This method assumes that commitA and commitB are either a parent and child - * commit or represent two patch sets which belong to the same change. No checks are made to - * confirm this assumption! - * - * @throws IOException if the repository can't be accessed - */ - private boolean arePatchSetCommitsWithRebaseInBetween( - RevWalk revWalk, RevCommit commitA, RevCommit commitB) throws IOException { - return key.getOldId() != null - && commitB.getParentCount() == 1 - && commitA != null - && commitA.getParentCount() == 1 - && !ObjectId.equals(commitB.getParent(0), commitA.getParent(0)) - && !revWalk.isMergedInto(commitA, commitB) - && !revWalk.isMergedInto(commitB, commitA); - } - - /** - * Determines all {@code Edit}s which were introduced by a rebase. The {@code Edit}s are expressed - * as differences between {@code treeA} of {@code commitA} and {@code treeB} of {@code commitB}. - * - * @param commitA the commit defining {@code treeA} - * @param commitB the commit defining {@code treeB} - * @return the {@code Edit}s per file path they modify in {@code treeB} - * @throws PatchListNotAvailableException if the {@code Edit}s can't be determined - */ - private Multimap getEditsDueToRebasePerFilePath( - RevCommit commitA, RevCommit commitB) throws PatchListNotAvailableException { PatchListKey parentDiffKey = - new PatchListKey(commitA.getParent(0), commitB.getParent(0), key.getWhitespace()); + PatchListKey.againstCommitWithPureTreeDiff( + commitA.getParent(0), commitB.getParent(0), key.getWhitespace()); PatchList parentPatchList = patchListCache.get(parentDiffKey, project); PatchListKey oldKey = PatchListKey.againstDefaultBase(key.getOldId(), key.getWhitespace()); PatchList oldPatchList = patchListCache.get(oldKey, project); @@ -319,6 +267,32 @@ public class PatchListLoader implements Callable { return editTransformer.getEditsPerFilePath(); } + private static boolean isRootOrMergeCommit(RevCommit commit) { + return commit.getParentCount() != 1; + } + + private static boolean areParentChild(RevCommit commitA, RevCommit commitB) { + return ObjectId.equals(commitA.getParent(0), commitB) + || ObjectId.equals(commitB.getParent(0), commitA); + } + + private static boolean haveCommonParent(RevCommit commitA, RevCommit commitB) { + return ObjectId.equals(commitA.getParent(0), commitB.getParent(0)); + } + + private static Set getEditsDueToRebase( + Multimap editsDueToRebasePerFilePath, DiffEntry diffEntry) { + if (editsDueToRebasePerFilePath.isEmpty()) { + return ImmutableSet.of(); + } + + String filePath = diffEntry.getNewPath(); + if (diffEntry.getChangeType() == ChangeType.DELETE) { + filePath = diffEntry.getOldPath(); + } + return ImmutableSet.copyOf(editsDueToRebasePerFilePath.get(filePath)); + } + private Optional getPatchListEntry( ObjectReader objectReader, DiffFormatter diffFormatter, diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListWeigher.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListWeigher.java index f40eac6518..942d0e0903 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListWeigher.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListWeigher.java @@ -23,7 +23,8 @@ public class PatchListWeigher implements Weigher { int size = 16 + 4 * 8 - + 2 * 36 // Size of PatchListKey, 64 bit JVM + + 2 * 36 + + 8 // Size of PatchListKey, 64 bit JVM + 16 + 3 * 8 + 3 * 4 diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptFactory.java index 84a080afea..081ba7aee1 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptFactory.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptFactory.java @@ -235,7 +235,7 @@ public class PatchScriptFactory implements Callable { private PatchListKey keyFor(Whitespace whitespace) { if (parentNum < 0) { - return new PatchListKey(aId, bId, whitespace); + return PatchListKey.againstCommit(aId, bId, whitespace); } return PatchListKey.againstParentNum(parentNum + 1, bId, whitespace); }