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 85f726a55b..0324ef6805 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 class CommitGraphWriter {
 	 */
 	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 class CommitGraphWriter {
 		} finally {
 			monitor.endTask();
 		}
-		return stats;
+		return Stats.from(bloomFilterChunks);
 	}
 
 	private static List<ChunkHeader> createCoreChunks(int hashsz, GraphCommits graphCommits)
@@ -415,12 +414,14 @@ public class CommitGraphWriter {
 		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 @@ public class CommitGraphWriter {
 			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 @@ public class CommitGraphWriter {
 				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 @@ public class CommitGraphWriter {
 
 		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 @@ public class CommitGraphWriter {
 	 */
 	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;
-- 
2.39.5