diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefDirectoryTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefDirectoryTest.java index f2b4b484b..619e585a9 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefDirectoryTest.java +++ b/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 repo; + TestRepository 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()); diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/SnapshottingRefDirectoryTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/SnapshottingRefDirectoryTest.java new file mode 100644 index 000000000..c3dafe4aa --- /dev/null +++ b/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()); + } +} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java index 72f1d5afa..cc9db5f77 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java +++ b/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 parsePackedRefs(BufferedReader br) throws IOException { RefList.Builder 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); } /** diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectoryRename.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectoryRename.java index 2c0ade681..d07299e45 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectoryRename.java +++ b/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 { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/SnapshottingRefDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/SnapshottingRefDirectory.java new file mode 100644 index 000000000..0b9748096 --- /dev/null +++ b/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}. + *

+ * 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. + *

+ * 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. + *

+ * 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(). + *

+ * 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 call() throws E; + } + + @FunctionalInterface + private interface FunctionThrowsException { + R apply(A a) throws E; + } + + @FunctionalInterface + private interface TriConsumerThrowsException { + void accept(A1 a1, A2 a2, A3 a3) throws E; + } + + private static T invalidateSnapshotOnError( + SupplierThrowsException f, RefDatabase refDb) + throws IOException { + return invalidateSnapshotOnError(a -> f.call(), null, refDb); + } + + private static R invalidateSnapshotOnError( + FunctionThrowsException f, A a, + RefDatabase refDb) throws IOException { + try { + return f.apply(a); + } catch (IOException e) { + ((SnapshottingRefDirectory) refDb).invalidateSnapshot(); + throw e; + } + } + + private static void invalidateSnapshotOnError( + TriConsumerThrowsException 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 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()); + } + } +} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/BatchRefUpdate.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/BatchRefUpdate.java index e2bebfefd..21d77f47c 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/BatchRefUpdate.java +++ b/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())) {