From 2390a89f2875c71a3310e74a03cad5aba202d1ec Mon Sep 17 00:00:00 2001
From: Ivan Frade <ifrade@google.com>
Date: Fri, 8 Sep 2023 13:23:09 -0700
Subject: [PATCH] CommitGraphWriter: Decouple Stats from computing bloom
 filters

The public stats object is created only to be populated by the computation of
bloom filters.

Make the computation return its numbers with the results and copy them
to the stats when needed. This eliminates the side effects from the
computation and makes it easier to add more data to the stats later.

Change-Id: I7a5e55fc3a17f5a294edf3a3b725b2d9c0280a5a
---
 .../commitgraph/CommitGraphWriter.java        | 40 ++++++++++++++-----
 1 file changed, 31 insertions(+), 9 deletions(-)

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 85f726a55..0324ef680 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
@@ -118,13 +118,12 @@ public CommitGraphWriter(@NonNull GraphCommits graphCommits,
 	 */
 	public Stats write(@NonNull ProgressMonitor monitor,
 			@NonNull OutputStream commitGraphStream) throws IOException {
-		Stats stats = new Stats();
 		if (graphCommits.size() == 0) {
-			return stats;
+			return Stats.EMPTY;
 		}
 
 		BloomFilterChunks bloomFilterChunks = generateChangedPathFilters
-				? computeBloomFilterChunks(stats)
+				? computeBloomFilterChunks()
 				: null;
 		List<ChunkHeader> chunks = new ArrayList<>();
 		chunks.addAll(createCoreChunks(hashsz, graphCommits));
@@ -156,7 +155,7 @@ public Stats write(@NonNull ProgressMonitor monitor,
 		} finally {
 			monitor.endTask();
 		}
-		return stats;
+		return Stats.from(bloomFilterChunks);
 	}
 
 	private static List<ChunkHeader> createCoreChunks(int hashsz, GraphCommits graphCommits)
@@ -415,12 +414,14 @@ private static Optional<HashSet<ByteBuffer>> computeBloomFilterPaths(
 		return Optional.of(paths);
 	}
 
-	private BloomFilterChunks computeBloomFilterChunks(Stats stats)
+	private BloomFilterChunks computeBloomFilterChunks()
 			throws MissingObjectException, IncorrectObjectTypeException,
 			CorruptObjectException, IOException {
 
 		ByteArrayOutputStream index = new ByteArrayOutputStream();
 		ByteArrayOutputStream data = new ByteArrayOutputStream();
+		long filtersReused = 0;
+		long filtersComputed =0;
 
 		// Allocate scratch buffer for converting integers into
 		// big-endian bytes.
@@ -438,9 +439,9 @@ private BloomFilterChunks computeBloomFilterChunks(Stats stats)
 			for (RevCommit cmit : graphCommits) {
 				ChangedPathFilter cpf = cmit.getChangedPathFilter(rw);
 				if (cpf != null) {
-					stats.changedPathFiltersReused++;
+					filtersReused++;
 				} else {
-					stats.changedPathFiltersComputed++;
+					filtersComputed++;
 					Optional<HashSet<ByteBuffer>> paths = computeBloomFilterPaths(
 							graphCommits.getObjectReader(), cmit);
 					if (paths.isEmpty()) {
@@ -453,7 +454,7 @@ private BloomFilterChunks computeBloomFilterChunks(Stats stats)
 				NB.encodeInt32(scratch, 0, data.size() - dataHeaderSize);
 				index.write(scratch);
 			}
-			return new BloomFilterChunks(index, data);
+			return new BloomFilterChunks(index, data, filtersReused, filtersComputed);
 		}
 	}
 
@@ -503,10 +504,18 @@ private static class BloomFilterChunks {
 
 		final ByteArrayOutputStream data;
 
+		final long filtersReused;
+
+		final long filtersComputed;
+
 		BloomFilterChunks(ByteArrayOutputStream index,
-				ByteArrayOutputStream data) {
+				ByteArrayOutputStream data,
+				long filtersReused,
+				long filtersComputed) {
 			this.index = index;
 			this.data = data;
+			this.filtersReused = filtersReused;
+			this.filtersComputed = filtersComputed;
 		}
 	}
 
@@ -515,6 +524,19 @@ private static class BloomFilterChunks {
 	 */
 	public static class Stats {
 
+		static final Stats EMPTY = new Stats();
+
+		static final Stats from(@Nullable BloomFilterChunks bloomFilterChunks) {
+			Stats stats = new Stats();
+			if (bloomFilterChunks != null) {
+				stats.changedPathFiltersComputed = bloomFilterChunks.filtersComputed;
+				stats.changedPathFiltersReused = bloomFilterChunks.filtersReused;
+			}
+			return stats;
+		}
+
+		private Stats() {};
+
 		private long changedPathFiltersReused = 0;
 
 		private long changedPathFiltersComputed = 0;