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>

org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/commitgraph/CommitGraphWriterTest.java

@@ -23,8 +23,10 @@
 
 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 void testChangedPathFilterManyChanges() throws Exception {
 		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);
 	}

org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/commitgraph/CommitGraphWriter.java

@@ -93,16 +93,18 @@ public CommitGraphWriter(@NonNull GraphCommits graphCommits) {
 	 *            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 void write(@NonNull ProgressMonitor monitor,
 		} 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 @@ private List<ChunkHeader> createChunks() throws MissingObjectException,
 			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 @@ private static Optional<HashSet<ByteBuffer>> computeBloomFilterPaths(
 		return Optional.of(paths);
 	}
 
-	private BloomFilterChunks computeBloomFilterChunks()
+	private BloomFilterChunks computeBloomFilterChunks(Stats stats)
 			throws MissingObjectException, IncorrectObjectTypeException,
 			CorruptObjectException, IOException {
 
@@ -383,13 +387,18 @@ private BloomFilterChunks computeBloomFilterChunks()
 		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 @@ private static class BloomFilterChunks {
 			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;
+		}
+	}
 }

org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java

@@ -697,7 +697,7 @@ int getGeneration() {
 	 * @return the changed path filter
 	 * @since 6.7
 	 */
-	ChangedPathFilter getChangedPathFilter() {
+	public ChangedPathFilter getChangedPathFilter() {
 		return null;
 	}
 

org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommitCG.java

@@ -110,7 +110,7 @@ int getGeneration() {
 
 	/** {@inheritDoc} */
 	@Override
-	ChangedPathFilter getChangedPathFilter() {
+	public ChangedPathFilter getChangedPathFilter() {
 		return changedPathFilter;
 	}
 }