From 7baa5a157b467938e90df76f1a69c00f0494cb58 Mon Sep 17 00:00:00 2001
From: Ivan Frade <ifrade@google.com>
Date: Fri, 8 Sep 2023 08:35:47 -0700
Subject: [PATCH] DfsPackFile: Record index loads only in one place

Each index can be set in the reader from two locations: the dfs cache
callback or the code afterwards. The pack is emitting the load event
in both cases, when the reference is set. This is brittle (right now
it is missing events for BITMAP_INDEX and COMMIT_GRAPH).

Emit the index loaded event only once, after going through the cache
code. The fact that the reference was set in the callback or the main
code is irrelevant. Also, the reader is per-thread, so there shouldn't
be any concurrency involved triggering double counts.

Change-Id: I7f3d078a53741ecc1e81b96353ed8faa8fef3a49
---
 .../jgit/internal/storage/dfs/DfsPackFile.java   | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackFile.java
index 70c8d0433..7f985229e 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackFile.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackFile.java
@@ -182,8 +182,8 @@ private PackIndex idx(DfsReader ctx) throws IOException {
 			PackIndex idx = idxref.get();
 			if (index == null && idx != null) {
 				index = idx;
-				ctx.emitIndexLoad(desc, INDEX, idx);
 			}
+			ctx.emitIndexLoad(desc, INDEX, index);
 			return index;
 		} catch (IOException e) {
 			invalid = true;
@@ -229,8 +229,8 @@ public PackBitmapIndex getBitmapIndex(DfsReader ctx) throws IOException {
 		PackBitmapIndex bmidx = idxref.get();
 		if (bitmapIndex == null && bmidx != null) {
 			bitmapIndex = bmidx;
-			ctx.emitIndexLoad(desc, BITMAP_INDEX, bmidx);
 		}
+		ctx.emitIndexLoad(desc, BITMAP_INDEX, bitmapIndex);
 		return bitmapIndex;
 	}
 
@@ -268,8 +268,8 @@ public CommitGraph getCommitGraph(DfsReader ctx) throws IOException {
 		CommitGraph cg = cgref.get();
 		if (commitGraph == null && cg != null) {
 			commitGraph = cg;
-			ctx.emitIndexLoad(desc, COMMIT_GRAPH, cg);
 		}
+		ctx.emitIndexLoad(desc, COMMIT_GRAPH, commitGraph);
 		return commitGraph;
 	}
 
@@ -303,8 +303,8 @@ public PackReverseIndex getReverseIdx(DfsReader ctx) throws IOException {
 		PackReverseIndex revidx = revref.get();
 		if (reverseIndex == null && revidx != null) {
 			reverseIndex = revidx;
-			ctx.emitIndexLoad(desc, REVERSE_INDEX, revidx);
 		}
+		ctx.emitIndexLoad(desc, REVERSE_INDEX, reverseIndex);
 		return reverseIndex;
 	}
 
@@ -334,12 +334,15 @@ private PackObjectSizeIndex getObjectSizeIndex(DfsReader ctx)
 			PackObjectSizeIndex sizeIdx = sizeIdxRef.get();
 			if (objectSizeIndex == null && sizeIdx != null) {
 				objectSizeIndex = sizeIdx;
-				ctx.emitIndexLoad(desc, OBJECT_SIZE_INDEX, sizeIdx);
 			}
 		} finally {
 			objectSizeIndexLoadAttempted = true;
 		}
 
+		// Object size index is optional, it can be null and that's fine
+		if (objectSizeIndex != null) {
+			ctx.emitIndexLoad(desc, OBJECT_SIZE_INDEX, objectSizeIndex);
+		}
 		return objectSizeIndex;
 	}
 
@@ -1183,7 +1186,6 @@ private DfsBlockCache.Ref<PackIndex> loadPackIndex(
 			try (ReadableChannel rc = ctx.db.openFile(desc, INDEX)) {
 				PackIndex idx = PackIndex.read(alignTo8kBlocks(rc));
 				ctx.stats.readIdxBytes += rc.position();
-				ctx.emitIndexLoad(desc, INDEX, idx);
 				index = idx;
 				return new DfsBlockCache.Ref<>(
 						idxKey,
@@ -1211,7 +1213,6 @@ private DfsBlockCache.Ref<PackReverseIndex> loadReverseIdx(
 		long start = System.nanoTime();
 		PackReverseIndex revidx = PackReverseIndexFactory.computeFromIndex(idx);
 		reverseIndex = revidx;
-		ctx.emitIndexLoad(desc, REVERSE_INDEX, revidx);
 		ctx.stats.readReverseIdxMicros += elapsedMicros(start);
 		return new DfsBlockCache.Ref<>(
 				revKey,
@@ -1232,7 +1233,6 @@ private DfsBlockCache.Ref<PackObjectSizeIndex> loadObjectSizeIndex(
 				objectSizeIndex = PackObjectSizeIndexLoader
 						.load(Channels.newInputStream(rc));
 				size = rc.position();
-				ctx.emitIndexLoad(desc, OBJECT_SIZE_INDEX, objectSizeIndex);
 			} catch (IOException e) {
 				parsingError = e;
 			}