Merge branch 'stable-7.3'HEADmaster

* stable-7.3:
  Use volatiles for bitmap and revIndex in Pack
  Fix performance regression in Pack.idx()
  Use LocalObjectToPack representation more
  Use representation from LocalObjectToPack if possible
  Avoid conditional in LocalObjectRepresentation.wasDeltaAttempted

Change-Id: Ibbc5b439b721a6d0267b6c70c0370f7a9fe7f74c

3 files changed, 80 insertions, 13 deletions

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/LocalObjectRepresentation.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/LocalObjectRepresentation.java
index 3f3d78c734..af571622b4 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/LocalObjectRepresentation.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/LocalObjectRepresentation.java
@@ -22,6 +22,11 @@ class LocalObjectRepresentation extends StoredObjectRepresentation {
 			public int getFormat() {
 				return PACK_WHOLE;
 			}
+
+			@Override
+			public boolean wasDeltaAttempted() {
+				return true;
+			}
 		};
 		r.pack = pack;
 		r.offset = offset;
@@ -81,5 +86,10 @@ class LocalObjectRepresentation extends StoredObjectRepresentation {
 		public int getFormat() {
 			return PACK_DELTA;
 		}
+
+		@Override
+		public boolean wasDeltaAttempted() {
+			return true;
+		}
 	}
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java
index 3a6de4e8e2..dc25bfde40 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java
@@ -70,6 +70,11 @@ import org.eclipse.jgit.util.FileUtils;
  * considered.
  */
 public class ObjectDirectory extends FileObjectDatabase {
+	@FunctionalInterface
+	private interface TriFunctionThrowsException<A, A2, A3, R, E extends Exception> {
+		R apply(A a, A2 a2, A3 a3) throws E;
+	}
+
 	/** Maximum number of candidates offered as resolutions of abbreviation. */
 	private static final int RESOLVE_ABBREV_LIMIT = 256;
 
@@ -348,9 +353,13 @@ public class ObjectDirectory extends FileObjectDatabase {
 	@Override
 	ObjectLoader openObject(WindowCursor curs, AnyObjectId objectId)
 			throws IOException {
-		ObjectLoader ldr = openObjectWithoutRestoring(curs, objectId);
-		if (ldr == null && restoreFromSelfOrAlternate(objectId, null)) {
+		ObjectLoader ldr = getFromLocalObjectToPack(curs, objectId,
+				(p, c, l) -> p.load(c, l.offset));
+		if (ldr == null) {
 			ldr = openObjectWithoutRestoring(curs, objectId);
+			if (ldr == null && restoreFromSelfOrAlternate(objectId, null)) {
+				ldr = openObjectWithoutRestoring(curs, objectId);
+			}
 		}
 		return ldr;
 	}
@@ -419,11 +428,16 @@ public class ObjectDirectory extends FileObjectDatabase {
 		return loose.open(curs, id);
 	}
 
+	@SuppressWarnings("boxing")
 	@Override
 	long getObjectSize(WindowCursor curs, AnyObjectId id) throws IOException {
-		long sz = getObjectSizeWithoutRestoring(curs, id);
-		if (0 > sz && restoreFromSelfOrAlternate(id, null)) {
+		Long sz = getFromLocalObjectToPack(curs, id,
+				(p, c, l) -> p.getObjectSize(c, l));
+		if (sz == null) {
 			sz = getObjectSizeWithoutRestoring(curs, id);
+			if (sz < 0 && restoreFromSelfOrAlternate(id, null)) {
+				sz = getObjectSizeWithoutRestoring(curs, id);
+			}
 		}
 		return sz;
 	}
@@ -480,6 +494,24 @@ public class ObjectDirectory extends FileObjectDatabase {
 		return -1;
 	}
 
+	private <R> R getFromLocalObjectToPack(WindowCursor curs,
+			AnyObjectId objectId,
+			TriFunctionThrowsException<Pack, WindowCursor, LocalObjectToPack, R, IOException> func) {
+		if (objectId instanceof LocalObjectToPack) {
+			LocalObjectToPack lotp = (LocalObjectToPack) objectId;
+			Pack pack = lotp.pack;
+			if (pack != null) {
+				try {
+					return func.apply(pack, curs, lotp);
+				} catch (IOException e) {
+					// lotp potentially repacked, continue as if lotp not
+					// provided
+				}
+			}
+		}
+		return null;
+	}
+
 	@Override
 	void selectObjectRepresentation(PackWriter packer, ObjectToPack otp,
 			WindowCursor curs) throws IOException {
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 f2f54947af..8988b41b55 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
@@ -124,15 +124,16 @@ public class Pack implements Iterable<PackIndex.MutableEntry> {
 
 	private byte[] packChecksum;
 
-	private Optionally<PackIndex> loadedIdx = Optionally.empty();
+	private volatile Optionally<PackIndex> loadedIdx = Optionally.empty();
 
-	private Optionally<PackReverseIndex> reverseIdx = Optionally.empty();
+	private volatile Optionally<PackReverseIndex> reverseIdx = Optionally.empty();
 
 	private volatile PackObjectSizeIndex loadedObjSizeIdx;
 
 	private volatile boolean attemptLoadObjSizeIdx;
 
-	private Optionally<PackBitmapIndex> bitmapIdx = Optionally.empty();
+	private volatile Optionally<PackBitmapIndex> bitmapIdx = Optionally.empty();
+
 
 	/**
 	 * Objects we have tried to read, and discovered to be corrupt.
@@ -168,7 +169,15 @@ public class Pack implements Iterable<PackIndex.MutableEntry> {
 		length = Long.MAX_VALUE;
 	}
 
-	private synchronized PackIndex idx() throws IOException {
+	private PackIndex idx() throws IOException {
+		Optional<PackIndex> optional = loadedIdx.getOptional();
+		if (optional.isPresent()) {
+			return optional.get();
+		}
+		return memoizeIdxIfNeeded();
+	}
+
+	private synchronized PackIndex memoizeIdxIfNeeded() throws IOException {
 		Optional<PackIndex> optional = loadedIdx.getOptional();
 		if (optional.isPresent()) {
 			return optional.get();
@@ -420,9 +429,9 @@ public class Pack implements Iterable<PackIndex.MutableEntry> {
 	}
 
 	private synchronized void closeIndices() {
-		loadedIdx.clear();
-		reverseIdx.clear();
-		bitmapIdx.clear();
+		loadedIdx = Optionally.empty();
+		reverseIdx = Optionally.empty();
+		bitmapIdx = Optionally.empty();
 	}
 
 	/**
@@ -1290,7 +1299,15 @@ public class Pack implements Iterable<PackIndex.MutableEntry> {
 		return getReverseIdx().findNextOffset(startOffset, maxOffset);
 	}
 
-	synchronized PackBitmapIndex getBitmapIndex() throws IOException {
+	PackBitmapIndex getBitmapIndex() throws IOException {
+		Optional<PackBitmapIndex> optional = bitmapIdx.getOptional();
+		if (optional.isPresent()) {
+			return optional.get();
+		}
+		return memoizeBitmapIndexIfNeeded();
+	}
+
+	private synchronized PackBitmapIndex memoizeBitmapIndexIfNeeded() throws IOException {
 		if (invalid || bitmapIdxFile == null) {
 			return null;
 		}
@@ -1325,7 +1342,15 @@ public class Pack implements Iterable<PackIndex.MutableEntry> {
 		this.bitmapIdxFile = bitmapIndexFile;
 	}
 
-	private synchronized PackReverseIndex getReverseIdx() throws IOException {
+	private PackReverseIndex getReverseIdx() throws IOException {
+		Optional<PackReverseIndex> optional = reverseIdx.getOptional();
+		if (optional.isPresent()) {
+			return optional.get();
+		}
+		return memoizeReverseIdxIfNeeded();
+	}
+
+	private synchronized PackReverseIndex memoizeReverseIdxIfNeeded() throws IOException {
 		if (invalid) {
 			throw new PackInvalidException(packFile, invalidatingCause);
 		}