Implement a snapshotting RefDirectory for use in request scope

Introduce a SnapshottingRefDirectory class which allows users to get
a snapshot of the ref database and use it in a request scope (for
example a Gerrit query) instead of having to re-read packed-refs
several times in a request.

This can potentially be further improved to avoid scanning/reading a
loose ref several times in a request. This would especially help
repeated lookups of a packed ref, where we check for the existence of
a loose ref each time.

Change-Id: I634b92877f819f8bf36a3b9586bbc1815108189a
Signed-off-by: Kaushik Lingarkar <quic_kaushikl@quicinc.com>

org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefDirectoryTest.java

@@ -58,13 +58,13 @@
 public class RefDirectoryTest extends LocalDiskRepositoryTestCase {
 	private Repository diskRepo;
 
-	private TestRepository<Repository> repo;
+	TestRepository<Repository> repo;
 
-	private RefDirectory refdir;
+	RefDirectory refdir;
 
-	private RevCommit A;
+	RevCommit A;
 
-	private RevCommit B;
+	RevCommit B;
 
 	private RevTag v1_0;
 
@@ -1349,6 +1349,17 @@ public void testPackedRefsLockFailure() throws Exception {
 		assertEquals(Storage.LOOSE, ref.getStorage());
 	}
 
+	void writePackedRef(String name, AnyObjectId id) throws IOException {
+		writePackedRefs(id.name() + " " + name + "\n");
+	}
+
+	void writePackedRefs(String content) throws IOException {
+		File pr = new File(diskRepo.getDirectory(), "packed-refs");
+		write(pr, content);
+		FS fs = diskRepo.getFS();
+		fs.setLastModified(pr.toPath(), Instant.now().minusSeconds(3600));
+	}
+
 	private void writeLooseRef(String name, AnyObjectId id) throws IOException {
 		writeLooseRef(name, id.name() + "\n");
 	}
@@ -1357,17 +1368,6 @@ private void writeLooseRef(String name, String content) throws IOException {
 		write(new File(diskRepo.getDirectory(), name), content);
 	}
 
-	private void writePackedRef(String name, AnyObjectId id) throws IOException {
-		writePackedRefs(id.name() + " " + name + "\n");
-	}
-
-	private void writePackedRefs(String content) throws IOException {
-		File pr = new File(diskRepo.getDirectory(), "packed-refs");
-		write(pr, content);
-		FS fs = diskRepo.getFS();
-		fs.setLastModified(pr.toPath(), Instant.now().minusSeconds(3600));
-	}
-
 	private void deleteLooseRef(String name) {
 		File path = new File(diskRepo.getDirectory(), name);
 		assertTrue("deleted " + name, path.delete());

org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/SnapshottingRefDirectoryTest.java

@@ -0,0 +1,88 @@
+/*
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc.
+ * and other copyright owners as documented in the project's IP log.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Distribution License v. 1.0 which is available at
+ * https://www.eclipse.org/org/documents/edl-v10.php.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+package org.eclipse.jgit.internal.storage.file;
+
+import org.eclipse.jgit.lib.NullProgressMonitor;
+import org.eclipse.jgit.transport.ReceiveCommand;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.io.IOException;
+
+import static org.junit.Assert.assertEquals;
+
+public class SnapshottingRefDirectoryTest extends RefDirectoryTest {
+	private RefDirectory originalRefDirectory;
+
+	/** {@inheritDoc} */
+	@Before
+	@Override
+	public void setUp() throws Exception {
+		super.setUp();
+		originalRefDirectory = refdir;
+		refdir = refdir.createSnapshottingRefDirectory();
+	}
+
+	@Test
+	public void testSnapshot_CannotSeeExternalPackedRefsUpdates()
+			throws IOException {
+		String refName = "refs/heads/new";
+
+		writePackedRef(refName, A);
+		assertEquals(A, originalRefDirectory.exactRef(refName).getObjectId());
+		assertEquals(A, refdir.exactRef(refName).getObjectId());
+
+		writePackedRef(refName, B);
+		assertEquals(B, originalRefDirectory.exactRef(refName).getObjectId());
+		assertEquals(A, refdir.exactRef(refName).getObjectId());
+	}
+
+	@Test
+	public void testSnapshot_WriteThrough() throws IOException {
+		String refName = "refs/heads/new";
+
+		writePackedRef(refName, A);
+		assertEquals(A, originalRefDirectory.exactRef(refName).getObjectId());
+		assertEquals(A, refdir.exactRef(refName).getObjectId());
+
+		PackedBatchRefUpdate update = refdir.newBatchUpdate();
+		update.addCommand(new ReceiveCommand(A, B, refName));
+		update.execute(repo.getRevWalk(), NullProgressMonitor.INSTANCE);
+
+		assertEquals(B, originalRefDirectory.exactRef(refName).getObjectId());
+		assertEquals(B, refdir.exactRef(refName).getObjectId());
+	}
+
+	@Test
+	public void testSnapshot_IncludeExternalPackedRefsUpdatesWithWrites()
+			throws IOException {
+		String refA = "refs/heads/refA";
+		String refB = "refs/heads/refB";
+		writePackedRefs("" + //
+				A.name() + " " + refA + "\n" + //
+				A.name() + " " + refB + "\n");
+		assertEquals(A, refdir.exactRef(refA).getObjectId());
+		assertEquals(A, refdir.exactRef(refB).getObjectId());
+
+		writePackedRefs("" + //
+				B.name() + " " + refA + "\n" + //
+				A.name() + " " + refB + "\n");
+		PackedBatchRefUpdate update = refdir.newBatchUpdate();
+		update.addCommand(new ReceiveCommand(A, B, refB));
+		update.execute(repo.getRevWalk(), NullProgressMonitor.INSTANCE);
+
+		assertEquals(B, originalRefDirectory.exactRef(refA).getObjectId());
+		assertEquals(B, refdir.exactRef(refA).getObjectId());
+		assertEquals(B, originalRefDirectory.exactRef(refB).getObjectId());
+		assertEquals(B, refdir.exactRef(refB).getObjectId());
+	}
+}

org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java

@@ -179,6 +179,19 @@ public class RefDirectory extends RefDatabase {
 
 	private final TrustPackedRefsStat trustPackedRefsStat;
 
+	RefDirectory(RefDirectory refDb) {
+		parent = refDb.parent;
+		gitDir = refDb.gitDir;
+		refsDir = refDb.refsDir;
+		logsDir = refDb.logsDir;
+		logsRefsDir = refDb.logsRefsDir;
+		packedRefsFile = refDb.packedRefsFile;
+		looseRefs.set(refDb.looseRefs.get());
+		packedRefs.set(refDb.packedRefs.get());
+		trustFolderStat = refDb.trustFolderStat;
+		trustPackedRefsStat = refDb.trustPackedRefsStat;
+	}
+
 	RefDirectory(FileRepository db) {
 		final FS fs = db.getFS();
 		parent = db;
@@ -223,6 +236,15 @@ public File logFor(String name) {
 		return new File(logsDir, name);
 	}
 
+	/**
+	 * Create a cache of this {@link RefDirectory}.
+	 *
+	 * @return a cached RefDirectory.
+	 */
+	public SnapshottingRefDirectory createSnapshottingRefDirectory() {
+		return new SnapshottingRefDirectory(this);
+	}
+
 	/** {@inheritDoc} */
 	@Override
 	public void create() throws IOException {
@@ -575,18 +597,26 @@ public RefDirectoryUpdate newUpdate(String name, boolean detach)
 		else {
 			detachingSymbolicRef = detach && ref.isSymbolic();
 		}
-		RefDirectoryUpdate refDirUpdate = new RefDirectoryUpdate(this, ref);
+		RefDirectoryUpdate refDirUpdate = createRefDirectoryUpdate(ref);
 		if (detachingSymbolicRef)
 			refDirUpdate.setDetachingSymbolicRef();
 		return refDirUpdate;
 	}
 
+	RefDirectoryUpdate createRefDirectoryUpdate(Ref ref) {
+		return new RefDirectoryUpdate(this, ref);
+	}
+
 	/** {@inheritDoc} */
 	@Override
 	public RefDirectoryRename newRename(String fromName, String toName)
 			throws IOException {
 		RefDirectoryUpdate from = newUpdate(fromName, false);
 		RefDirectoryUpdate to = newUpdate(toName, false);
+		return createRefDirectoryRename(from, to);
+	}
+
+	RefDirectoryRename createRefDirectoryRename(RefDirectoryUpdate from, RefDirectoryUpdate to) {
 		return new RefDirectoryRename(from, to);
 	}
 
@@ -966,6 +996,13 @@ private PackedRefList readPackedRefs() throws IOException {
 		}
 	}
 
+	void compareAndSetPackedRefs(PackedRefList curList, PackedRefList newList) {
+		if (packedRefs.compareAndSet(curList, newList)
+				&& !curList.id.equals(newList.id)) {
+			modCnt.incrementAndGet();
+		}
+	}
+
 	private RefList<Ref> parsePackedRefs(BufferedReader br)
 			throws IOException {
 		RefList.Builder<Ref> all = new RefList.Builder<>();
@@ -1258,7 +1295,7 @@ RefDirectoryUpdate newTemporaryUpdate() throws IOException {
 		File tmp = File.createTempFile("renamed_", "_ref", refsDir); //$NON-NLS-1$ //$NON-NLS-2$
 		String name = Constants.R_REFS + tmp.getName();
 		Ref ref = new ObjectIdRef.Unpeeled(NEW, name, null);
-		return new RefDirectoryUpdate(this, ref);
+		return createRefDirectoryUpdate(ref);
 	}
 
 	/**

org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectoryRename.java

@@ -59,6 +59,15 @@ class RefDirectoryRename extends RefRename {
 		refdb = src.getRefDatabase();
 	}
 
+	/**
+	 * Get the ref directory associated with this rename.
+	 *
+	 * @return the ref directory.
+	 */
+	protected RefDirectory getRefDirectory() {
+		return refdb;
+	}
+
 	/** {@inheritDoc} */
 	@Override
 	protected Result doRename() throws IOException {

org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/SnapshottingRefDirectory.java

@@ -0,0 +1,258 @@
+/*
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc.
+ * and other copyright owners as documented in the project's IP log.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Distribution License v. 1.0 which is available at
+ * https://www.eclipse.org/org/documents/edl-v10.php.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+package org.eclipse.jgit.internal.storage.file;
+
+import org.eclipse.jgit.lib.ProgressMonitor;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.RefDatabase;
+import org.eclipse.jgit.lib.RefUpdate;
+import org.eclipse.jgit.revwalk.RevWalk;
+
+import java.io.IOException;
+import java.util.List;
+
+/**
+ * Snapshotting write-through cache of a {@link RefDirectory}.
+ * <p>
+ * This is intended to be short-term write-through snapshot based cache used in
+ * a request scope to avoid re-reading packed-refs on each read. A future
+ * improvement could also snapshot loose refs.
+ * <p>
+ * Only use this class when concurrent writes from other requests (not using the
+ * same instance of SnapshottingRefDirectory) generally need not be visible to
+ * the current request. The exception to this is when such writes would cause
+ * writes from this snapshot to fail due to their base ref value being
+ * outdated.
+ */
+class SnapshottingRefDirectory extends RefDirectory {
+	final RefDirectory refDb;
+
+	private volatile boolean isValid;
+
+	/**
+	 * Create a snapshotting write-through cache of a {@link RefDirectory}.
+	 *
+	 * @param refDb
+	 * 		a reference to the ref database
+	 */
+	SnapshottingRefDirectory(RefDirectory refDb) {
+		super(refDb);
+		this.refDb = refDb;
+	}
+
+	/**
+	 * Lazily initializes and returns a PackedRefList snapshot.
+	 * <p>
+	 * A newer snapshot will be returned when a ref update is performed using
+	 * this {@link SnapshottingRefDirectory}.
+	 */
+	@Override
+	PackedRefList getPackedRefs() throws IOException {
+		if (!isValid) {
+			synchronized (this) {
+				if (!isValid) {
+					refreshSnapshot();
+				}
+			}
+		}
+		return packedRefs.get();
+	}
+
+	/** {@inheritDoc} */
+	@Override
+	void delete(RefDirectoryUpdate update) throws IOException {
+		refreshSnapshot();
+		super.delete(update);
+	}
+
+	/** {@inheritDoc} */
+	@Override
+	public RefDirectoryUpdate newUpdate(String name, boolean detach)
+			throws IOException {
+		refreshSnapshot();
+		return super.newUpdate(name, detach);
+	}
+
+	/** {@inheritDoc} */
+	@Override
+	public PackedBatchRefUpdate newBatchUpdate() {
+		return new SnapshotPackedBatchRefUpdate(this);
+	}
+
+	/** {@inheritDoc} */
+	@Override
+	public PackedBatchRefUpdate newBatchUpdate(boolean shouldLockLooseRefs) {
+		return new SnapshotPackedBatchRefUpdate(this, shouldLockLooseRefs);
+	}
+
+	/** {@inheritDoc} */
+	@Override
+	RefDirectoryUpdate newTemporaryUpdate() throws IOException {
+		refreshSnapshot();
+		return super.newTemporaryUpdate();
+	}
+
+	@Override
+	RefDirectoryUpdate createRefDirectoryUpdate(Ref ref) {
+		return new SnapshotRefDirectoryUpdate(this, ref);
+	}
+
+	@Override
+	RefDirectoryRename createRefDirectoryRename(RefDirectoryUpdate from,
+			RefDirectoryUpdate to) {
+		return new SnapshotRefDirectoryRename(from, to);
+	}
+
+	synchronized void invalidateSnapshot() {
+		isValid = false;
+	}
+
+	/**
+	 * Refresh our snapshot by calling the underlying RefDirectory's
+	 * getPackedRefs().
+	 * <p>
+	 * Update the in-memory copy of the underlying RefDirectory's packed-refs to
+	 * avoid the overhead of re-reading packed-refs on each new snapshot as the
+	 * packed-refs of the underlying RefDirectory may not get updated if most
+	 * threads use this snapshot.
+	 *
+	 * @throws IOException
+	 */
+	private synchronized void refreshSnapshot() throws IOException {
+		compareAndSetPackedRefs(packedRefs.get(), refDb.getPackedRefs());
+		isValid = true;
+	}
+
+	@FunctionalInterface
+	private interface SupplierThrowsException<R, E extends Exception> {
+		R call() throws E;
+	}
+
+	@FunctionalInterface
+	private interface FunctionThrowsException<A, R, E extends Exception> {
+		R apply(A a) throws E;
+	}
+
+	@FunctionalInterface
+	private interface TriConsumerThrowsException<A1, A2, A3, E extends Exception> {
+		void accept(A1 a1, A2 a2, A3 a3) throws E;
+	}
+
+	private static <T> T invalidateSnapshotOnError(
+			SupplierThrowsException<T, IOException> f, RefDatabase refDb)
+			throws IOException {
+		return invalidateSnapshotOnError(a -> f.call(), null, refDb);
+	}
+
+	private static <A, R> R invalidateSnapshotOnError(
+			FunctionThrowsException<A, R, IOException> f, A a,
+			RefDatabase refDb) throws IOException {
+		try {
+			return f.apply(a);
+		} catch (IOException e) {
+			((SnapshottingRefDirectory) refDb).invalidateSnapshot();
+			throw e;
+		}
+	}
+
+	private static <A1, A2, A3> void invalidateSnapshotOnError(
+			TriConsumerThrowsException<A1, A2, A3, IOException> f, A1 a1, A2 a2,
+			A3 a3, RefDatabase refDb) throws IOException {
+		try {
+			f.accept(a1, a2, a3);
+		} catch (IOException e) {
+			((SnapshottingRefDirectory) refDb).invalidateSnapshot();
+			throw e;
+		}
+	}
+
+	private static class SnapshotRefDirectoryUpdate extends RefDirectoryUpdate {
+		SnapshotRefDirectoryUpdate(RefDirectory r, Ref ref) {
+			super(r, ref);
+		}
+
+		@Override
+		public Result forceUpdate() throws IOException {
+			return invalidateSnapshotOnError(() -> super.forceUpdate(),
+					getRefDatabase());
+		}
+
+		@Override
+		public Result update() throws IOException {
+			return invalidateSnapshotOnError(() -> super.update(),
+					getRefDatabase());
+		}
+
+		@Override
+		public Result update(RevWalk walk) throws IOException {
+			return invalidateSnapshotOnError(rw -> super.update(rw), walk,
+					getRefDatabase());
+		}
+
+		@Override
+		public Result delete() throws IOException {
+			return invalidateSnapshotOnError(() -> super.delete(),
+					getRefDatabase());
+		}
+
+		@Override
+		public Result delete(RevWalk walk) throws IOException {
+			return invalidateSnapshotOnError(rw -> super.delete(rw), walk,
+					getRefDatabase());
+		}
+
+		@Override
+		public Result link(String target) throws IOException {
+			return invalidateSnapshotOnError(t -> super.link(t), target,
+					getRefDatabase());
+		}
+	}
+
+	private static class SnapshotRefDirectoryRename extends RefDirectoryRename {
+		SnapshotRefDirectoryRename(RefDirectoryUpdate src,
+				RefDirectoryUpdate dst) {
+			super(src, dst);
+		}
+
+		@Override
+		public RefUpdate.Result rename() throws IOException {
+			return invalidateSnapshotOnError(() -> super.rename(),
+					getRefDirectory());
+		}
+	}
+
+	private static class SnapshotPackedBatchRefUpdate
+			extends PackedBatchRefUpdate {
+		SnapshotPackedBatchRefUpdate(RefDirectory refdb) {
+			super(refdb);
+		}
+
+		SnapshotPackedBatchRefUpdate(RefDirectory refdb,
+				boolean shouldLockLooseRefs) {
+			super(refdb, shouldLockLooseRefs);
+		}
+
+		@Override
+		public void execute(RevWalk walk, ProgressMonitor monitor,
+				List<String> options) throws IOException {
+			invalidateSnapshotOnError((rw, m, o) -> super.execute(rw, m, o),
+					walk, monitor, options, getRefDatabase());
+		}
+
+		@Override
+		public void execute(RevWalk walk, ProgressMonitor monitor)
+				throws IOException {
+			invalidateSnapshotOnError((rw, m, a3) -> super.execute(rw, m), walk,
+					monitor, null, getRefDatabase());
+		}
+	}
+}

org.eclipse.jgit/src/org/eclipse/jgit/lib/BatchRefUpdate.java

@@ -521,6 +521,15 @@ public void execute(RevWalk walk, ProgressMonitor monitor,
 		monitor.endTask();
 	}
 
+	/**
+	 * Get the ref database associated with this update.
+	 *
+	 * @return the ref database.
+	 */
+	protected RefDatabase getRefDatabase() {
+		return refdb;
+	}
+
 	private static boolean isMissing(RevWalk walk, ObjectId id)
 			throws IOException {
 		if (id.equals(ObjectId.zeroId())) {