From 77aec62141cd22067f853eb0e0f7a2df757e3155 Mon Sep 17 00:00:00 2001 From: Jonathan Tan Date: Mon, 8 May 2023 13:51:28 -0700 Subject: [PATCH] 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 --- .../commitgraph/CommitGraphWriterTest.java | 36 +++++++++++ .../commitgraph/CommitGraphWriter.java | 63 +++++++++++++++---- .../org/eclipse/jgit/revwalk/RevCommit.java | 2 +- .../org/eclipse/jgit/revwalk/RevCommitCG.java | 2 +- 4 files changed, 89 insertions(+), 14 deletions(-) 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 e7f49496ad..79276ff564 100644 --- 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 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 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 f1b4f55299..eb9dfb4d9c 100644 --- 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 chunks = createChunks(); + List 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 createChunks() throws MissingObjectException, + private List createChunks(Stats stats) + throws MissingObjectException, IncorrectObjectTypeException, CorruptObjectException, IOException { List 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> 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> 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 316aeda4f4..e0bdf3eaf9 100644 --- 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 bfc0c59aad..8c8100320a 100644 --- 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; } } -- 2.39.5