From 51d6c63fe1602a9e812dc3610c10d7e6e31eeafd Mon Sep 17 00:00:00 2001
From: Martin Fick <mfick@nvidia.com>
Date: Tue, 17 Dec 2024 17:30:17 -0800
Subject: Pack: separate an open/close accounting lock

Previously the open/close accounting code used whole Pack object
synchronization for locking. Unfortunately, there are other unrelated
methods which use whole Pack object synchronization also, mostly to
avoid concurrent loading of these independent indices, and they do not
touch or need to coordinate with the open/close accounting data. During
heavy load when a new file appears after repacking the readFully()
threads could uselessly block on threads reading the reverse index.
These threads could have been reading from the Pack file instead of
waiting for the reverse index to be read. Use a new lock to make this
locking more fine grained to prevent the readFully() calling threads
from getting blocked in beginWindowCache() while the reverse index or
bitmaps are being loaded.

Change-Id: I7ac9067ca10cd6d6be0ab25148d99da3ace7ba36
Signed-off-by: Martin Fick <mfick@nvidia.com>
---
 .../eclipse/jgit/internal/storage/file/Pack.java   | 56 ++++++++++++++--------
 1 file changed, 35 insertions(+), 21 deletions(-)

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java
index 8d2a86386f..1a7b5de1d7 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java
@@ -95,6 +95,9 @@ public class Pack implements Iterable<PackIndex.MutableEntry> {
 
 	private RandomAccessFile fd;
 
+	/** For managing open/close accounting of {@link #fd}. */
+	private final Object activeLock = new Object();
+
 	/** Serializes reads performed against {@link #fd}. */
 	private final Object readLock = new Object();
 
@@ -645,37 +648,48 @@ public class Pack implements Iterable<PackIndex.MutableEntry> {
 			throw new EOFException();
 	}
 
-	private synchronized void beginCopyAsIs()
+	private void beginCopyAsIs()
 			throws StoredObjectRepresentationNotAvailableException {
-		if (++activeCopyRawData == 1 && activeWindows == 0) {
-			try {
-				doOpen();
-			} catch (IOException thisPackNotValid) {
-				throw new StoredObjectRepresentationNotAvailableException(
-						thisPackNotValid);
+		synchronized (activeLock) {
+			if (++activeCopyRawData == 1 && activeWindows == 0) {
+				try {
+					doOpen();
+				} catch (IOException thisPackNotValid) {
+					throw new StoredObjectRepresentationNotAvailableException(
+							thisPackNotValid);
+				}
 			}
 		}
 	}
 
-	private synchronized void endCopyAsIs() {
-		if (--activeCopyRawData == 0 && activeWindows == 0)
-			doClose();
+	private void endCopyAsIs() {
+		synchronized (activeLock) {
+			if (--activeCopyRawData == 0 && activeWindows == 0) {
+				doClose();
+			}
+		}
 	}
 
-	synchronized boolean beginWindowCache() throws IOException {
-		if (++activeWindows == 1) {
-			if (activeCopyRawData == 0)
-				doOpen();
-			return true;
+	boolean beginWindowCache() throws IOException {
+		synchronized (activeLock) {
+			if (++activeWindows == 1) {
+				if (activeCopyRawData == 0) {
+					doOpen();
+				}
+				return true;
+			}
+			return false;
 		}
-		return false;
 	}
 
-	synchronized boolean endWindowCache() {
-		final boolean r = --activeWindows == 0;
-		if (r && activeCopyRawData == 0)
-			doClose();
-		return r;
+	boolean endWindowCache() {
+		synchronized (activeLock) {
+			boolean r = --activeWindows == 0;
+			if (r && activeCopyRawData == 0) {
+				doClose();
+			}
+			return r;
+		}
 	}
 
 	private void doOpen() throws IOException {
-- 
cgit v1.2.3