CommitGraphWriter: reuse changed path filters

Teach CommitGraphWriter to reuse changed path filters that have been
read from the commit graph file whenever possible.

Change-Id: I1acbfa1613ca7198386a49209028886af360ddb6
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/commitgraph/CommitGraphWriterTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/commitgraph/CommitGraphWriterTest.java
index e7f49496adb6c33a7e8a8ed7fd53a373671d1e80..79276ff5646c0192cc2156c93b4a012c030b1820 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/commitgraph/CommitGraphWriterTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/commitgraph/CommitGraphWriterTest.java
@@ -23,8 +23,10 @@ import java.util.Set;
 
 import org.eclipse.jgit.dircache.DirCacheEntry;
 import org.eclipse.jgit.internal.storage.file.FileRepository;
+import org.eclipse.jgit.internal.storage.file.GC;
 import org.eclipse.jgit.junit.RepositoryTestCase;
 import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.ConfigConstants;
 import org.eclipse.jgit.lib.NullProgressMonitor;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.revwalk.RevBlob;
@@ -316,6 +318,40 @@ public class CommitGraphWriterTest extends RepositoryTestCase {
                assertThat(changedPaths, containsInAnyOrder("-1,"));
        }
 
+       @Test
+       public void testReuseBloomFilters() throws Exception {
+               RevBlob emptyBlob = tr.blob(new byte[] {});
+               RevCommit root = tr.commit(tr.tree(tr.file("foo.txt", emptyBlob),
+                               tr.file("onedir/twodir/bar.txt", emptyBlob)));
+               tr.branch("master").update(root);
+
+               db.getConfig().setBoolean(ConfigConstants.CONFIG_CORE_SECTION, null,
+                               ConfigConstants.CONFIG_COMMIT_GRAPH, true);
+               db.getConfig().setBoolean(ConfigConstants.CONFIG_GC_SECTION, null,
+                               ConfigConstants.CONFIG_KEY_WRITE_COMMIT_GRAPH, true);
+               GC gc = new GC(db);
+               gc.gc().get();
+
+               RevCommit tip = tr.commit(tr.tree(tr.file("foo-new.txt", emptyBlob),
+                               tr.file("onedir/twodir/bar-new.txt", emptyBlob)), root);
+
+               Set<ObjectId> wants = Collections.singleton(tip);
+               NullProgressMonitor m = NullProgressMonitor.INSTANCE;
+               GraphCommits graphCommits = GraphCommits.fromWalk(m, wants, walk);
+               writer = new CommitGraphWriter(graphCommits);
+               CommitGraphWriter.Stats stats = writer.write(m, os);
+
+               assertEquals(1, stats.getChangedPathFiltersReused());
+               assertEquals(1, stats.getChangedPathFiltersComputed());
+
+               // Expected strings are the same as in
+               // #testChangedPathFilterRootAndNested
+               HashSet<String> changedPaths = changedPathStrings(os.toByteArray());
+               assertThat(changedPaths, containsInAnyOrder(
+                               "109,-33,2,60,20,79,-11,116,",
+                               "119,69,63,-8,0,"));
+       }
+
        RevCommit commit(RevCommit... parents) throws Exception {
                return tr.commit(parents);
        }

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/commitgraph/CommitGraphWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/commitgraph/CommitGraphWriter.java
index f1b4f552993aad949199fccc4a4f1110f2c5dbdc..eb9dfb4d9c7a59cdd359c9e07416f2d407d1e0d5 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/commitgraph/CommitGraphWriter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/commitgraph/CommitGraphWriter.java
@@ -93,16 +93,18 @@ public class CommitGraphWriter {
         *            output stream of commit-graph data. The stream should be
         *            buffered by the caller. The caller is responsible for closing
         *            the stream.
+        * @return statistics gathered during the run
         * @throws IOException
         *             if an error occurred
         */
-       public void write(@NonNull ProgressMonitor monitor,
+       public Stats write(@NonNull ProgressMonitor monitor,
                        @NonNull OutputStream commitGraphStream) throws IOException {
+               Stats stats = new Stats();
                if (graphCommits.size() == 0) {
-                       return;
+                       return stats;
                }
 
-               List<ChunkHeader> chunks = createChunks();
+               List<ChunkHeader> chunks = createChunks(stats);
                long writeCount = 256 + 2 * graphCommits.size()
                                + graphCommits.getExtraEdgeCnt();
                monitor.beginTask(
@@ -122,9 +124,11 @@ public class CommitGraphWriter {
                } finally {
                        monitor.endTask();
                }
+               return stats;
        }
 
-       private List<ChunkHeader> createChunks() throws MissingObjectException,
+       private List<ChunkHeader> createChunks(Stats stats)
+                       throws MissingObjectException,
                        IncorrectObjectTypeException, CorruptObjectException, IOException {
                List<ChunkHeader> chunks = new ArrayList<>();
                chunks.add(new ChunkHeader(CHUNK_ID_OID_FANOUT, GRAPH_FANOUT_SIZE));
@@ -136,7 +140,7 @@ public class CommitGraphWriter {
                        chunks.add(new ChunkHeader(CHUNK_ID_EXTRA_EDGE_LIST,
                                        graphCommits.getExtraEdgeCnt() * 4));
                }
-               BloomFilterChunks bloomFilterChunks = computeBloomFilterChunks();
+               BloomFilterChunks bloomFilterChunks = computeBloomFilterChunks(stats);
                chunks.add(new ChunkHeader(CHUNK_ID_BLOOM_FILTER_INDEX,
                                bloomFilterChunks.index));
                chunks.add(new ChunkHeader(CHUNK_ID_BLOOM_FILTER_DATA,
@@ -363,7 +367,7 @@ public class CommitGraphWriter {
                return Optional.of(paths);
        }
 
-       private BloomFilterChunks computeBloomFilterChunks()
+       private BloomFilterChunks computeBloomFilterChunks(Stats stats)
                        throws MissingObjectException, IncorrectObjectTypeException,
                        CorruptObjectException, IOException {
 
@@ -383,13 +387,18 @@ public class CommitGraphWriter {
                int dataHeaderSize = data.size();
 
                for (RevCommit cmit : graphCommits) {
-                       Optional<HashSet<ByteBuffer>> paths = computeBloomFilterPaths(
-                                       graphCommits.getObjectReader(), cmit);
-                       ChangedPathFilter cpf;
-                       if (paths.isEmpty()) {
-                               cpf = ChangedPathFilter.FULL;
+                       ChangedPathFilter cpf = cmit.getChangedPathFilter();
+                       if (cpf != null) {
+                               stats.changedPathFiltersReused++;
                        } else {
-                               cpf = ChangedPathFilter.fromPaths(paths.get());
+                               stats.changedPathFiltersComputed++;
+                               Optional<HashSet<ByteBuffer>> paths = computeBloomFilterPaths(
+                                               graphCommits.getObjectReader(), cmit);
+                               if (paths.isEmpty()) {
+                                       cpf = ChangedPathFilter.FULL;
+                               } else {
+                                       cpf = ChangedPathFilter.fromPaths(paths.get());
+                               }
                        }
                        cpf.writeTo(data);
                        NB.encodeInt32(scratch, 0, data.size() - dataHeaderSize);
@@ -450,4 +459,34 @@ public class CommitGraphWriter {
                        this.data = data;
                }
        }
+
+       /**
+        * Statistics collected during a single commit graph write.
+        */
+       public static class Stats {
+
+               private long changedPathFiltersReused = 0;
+
+               private long changedPathFiltersComputed = 0;
+
+               /**
+                * Returns the number of existing changed path filters that were reused
+                * when writing, for statistical purposes.
+                *
+                * @return count of changed path filters
+                */
+               public long getChangedPathFiltersReused() {
+                       return changedPathFiltersReused;
+               }
+
+               /**
+                * Returns the number of changed path filters that were computed from
+                * scratch, for statistical purposes.
+                *
+                * @return count of changed path filters
+                */
+               public long getChangedPathFiltersComputed() {
+                       return changedPathFiltersComputed;
+               }
+       }
 }

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java
index 316aeda4f45dac94ddc09f8396a2f2fca6869093..e0bdf3eaf9d72353c737409e86616b11d5b207b7 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java
@@ -697,7 +697,7 @@ public class RevCommit extends RevObject {
         * @return the changed path filter
         * @since 6.7
         */
-       ChangedPathFilter getChangedPathFilter() {
+       public ChangedPathFilter getChangedPathFilter() {
                return null;
        }
 

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommitCG.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommitCG.java
index bfc0c59aad30aa4c9041e9db4c174fe4d5215661..8c8100320ae8f69815f3dcee5b6017fc630b4778 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommitCG.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommitCG.java
@@ -110,7 +110,7 @@ class RevCommitCG extends RevCommit {
 
        /** {@inheritDoc} */
        @Override
-       ChangedPathFilter getChangedPathFilter() {
+       public ChangedPathFilter getChangedPathFilter() {
                return changedPathFilter;
        }
 }