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 70c8d04338..7f985229e5 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 @@ public final class DfsPackFile extends BlockBasedFile {
 			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 final class DfsPackFile extends BlockBasedFile {
 		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 final class DfsPackFile extends BlockBasedFile {
 		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 final class DfsPackFile extends BlockBasedFile {
 		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 @@ public final class DfsPackFile extends BlockBasedFile {
 			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 @@ public final class DfsPackFile extends BlockBasedFile {
 			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 @@ public final class DfsPackFile extends BlockBasedFile {
 		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 @@ public final class DfsPackFile extends BlockBasedFile {
 				objectSizeIndex = PackObjectSizeIndexLoader
 						.load(Channels.newInputStream(rc));
 				size = rc.position();
-				ctx.emitIndexLoad(desc, OBJECT_SIZE_INDEX, objectSizeIndex);
 			} catch (IOException e) {
 				parsingError = e;
 			}
-- 
2.39.5