From f102abf035ae242d96368dadafaee8a339f72c7e Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Tue, 3 Nov 2015 21:54:48 -0800 Subject: [PATCH] Replace anonymous BitmapRevFilter subclasses with explicit classes This puts the code for include() in each RevFilter returned by newRevFilter in one place and should make the code easier to understand and modify. Change-Id: I590ac4604b61fc1ffe7aba2ed89f8139847ccac3 --- .../storage/pack/PackWriterBitmapWalker.java | 88 ++++++++++++++----- 1 file changed, 68 insertions(+), 20 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriterBitmapWalker.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriterBitmapWalker.java index cd18a9c5c..4a7585a9d 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriterBitmapWalker.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriterBitmapWalker.java @@ -110,7 +110,7 @@ final class PackWriterBitmapWalker { } if (marked) { - BitmapRevFilter filter = newRevFilter(seen, bitmapResult); + RevFilter filter = newRevFilter(seen, bitmapResult); walker.setRevFilter(filter); while (walker.next() != null) { @@ -141,41 +141,89 @@ final class PackWriterBitmapWalker { walker.reset(); } - static BitmapRevFilter newRevFilter( - final BitmapBuilder seen, final BitmapBuilder bitmapResult) { + static RevFilter newRevFilter(BitmapBuilder seen, BitmapBuilder bitmapResult) { if (seen != null) { - return new BitmapRevFilter() { - protected boolean load(RevCommit cmit) { - if (seen.contains(cmit)) - return false; - return bitmapResult.add(cmit, Constants.OBJ_COMMIT); - } - }; + return new AddUnseenToBitmapFilter(seen, bitmapResult); + } + return new AddToBitmapFilter(bitmapResult); + } + + /** + * A RevFilter that adds the visited commits to {@code bitmap} as a side + * effect. + *

+ * When the walk hits a commit that is part of {@code bitmap}'s + * BitmapIndex, that entire bitmap is ORed into {@code bitmap} and the + * commit and its parents are marked as SEEN so that the walk does not + * have to visit its ancestors. This ensures the walk is very short if + * there is good bitmap coverage. + */ + private static class AddToBitmapFilter extends RevFilter { + private final BitmapBuilder bitmap; + + AddToBitmapFilter(BitmapBuilder bitmap) { + this.bitmap = bitmap; } - return new BitmapRevFilter() { - @Override - protected boolean load(RevCommit cmit) { - return bitmapResult.add(cmit, Constants.OBJ_COMMIT); + + @Override + public final boolean include(RevWalk walker, RevCommit cmit) { + if (bitmap.add(cmit, Constants.OBJ_COMMIT)) { + return true; + } + for (RevCommit p : cmit.getParents()) { + p.add(RevFlag.SEEN); } - }; + return false; + } + + @Override + public final RevFilter clone() { + throw new UnsupportedOperationException(); + } + + @Override + public final boolean requiresCommitBody() { + return false; + } } - static abstract class BitmapRevFilter extends RevFilter { - protected abstract boolean load(RevCommit cmit); + /** + * A RevFilter that adds the visited commits to {@code bitmap} as a side + * effect. + *

+ * When the walk hits a commit that is part of {@code bitmap}'s + * BitmapIndex, that entire bitmap is ORed into {@code bitmap} and the + * commit and its parents are marked as SEEN so that the walk does not + * have to visit its ancestors. This ensures the walk is very short if + * there is good bitmap coverage. + *

+ * Commits named in {@code seen} are considered already seen. If one is + * encountered, that commit and its parents will be marked with the SEEN + * flag to prevent the walk from visiting its ancestors. + */ + private static class AddUnseenToBitmapFilter extends RevFilter { + private final BitmapBuilder seen; + private final BitmapBuilder bitmap; + + AddUnseenToBitmapFilter(BitmapBuilder seen, BitmapBuilder bitmapResult) { + this.seen = seen; + this.bitmap = bitmapResult; + } @Override public final boolean include(RevWalk walker, RevCommit cmit) { - if (load(cmit)) { + if (!seen.contains(cmit) && bitmap.add(cmit, Constants.OBJ_COMMIT)) { return true; } - for (RevCommit p : cmit.getParents()) + for (RevCommit p : cmit.getParents()) { p.add(RevFlag.SEEN); + } return false; } @Override public final RevFilter clone() { - return this; + throw new UnsupportedOperationException(); } @Override