Avoid looking at UNREACHABLE_GARBAGE for client have lines

Clients send a bunch of unknown objects to UploadPack on each round
of negotiation. Many of these are not known to the server, which
leads the implementation to be looking at indexes for garbage packs.

Disable examining the index of a garbage pack, allowing servers to
avoid reading them from disk during negotiation.

The effect of this change is the server will only ACK a have line
if the object was reachable during the last garbage collection,
or was recently added to the repository. For most repositories
there is no impact in this behavior change.

If a repository rewinds a branch, runs GC, and then resets the
branch back to where it was before, the now current tip is going to
be skipped by this change. A client that has the commit may wind up
getting a slightly larger data transfer from the server as an older
common ancestor will be chosen during negotiation. This is fixable
on the server side by running GC again to correct the layout of
objects in pack files.

Change-Id: Icd550359ef70fc7b701980f9b13d923fd13c744b

4 files changed, 39 insertions, 4 deletions

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectReader.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectReader.java
index 68fee612fb..42851498b4 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectReader.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectReader.java
@@ -437,6 +437,19 @@ public abstract class ObjectReader {
 	}
 
 	/**
+	 * Advise the reader to avoid unreachable objects.
+	 * <p>
+	 * While enabled the reader will skip over anything previously proven to be
+	 * unreachable. This may be dangerous in the face of concurrent writes.
+	 *
+	 * @param avoid
+	 *            true to avoid unreachable objects.
+	 */
+	public void setAvoidUnreachableObjects(boolean avoid) {
+		// Do nothing by default.
+	}
+
+	/**
 	 * An index that can be used to speed up ObjectWalks.
 	 *
 	 * @return the index or null if one does not exist.
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsPackFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsPackFile.java
index 80cced84e8..707488b039 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsPackFile.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsPackFile.java
@@ -45,9 +45,10 @@
 
 package org.eclipse.jgit.storage.dfs;
 
+import static org.eclipse.jgit.storage.dfs.DfsObjDatabase.PackSource.UNREACHABLE_GARBAGE;
 import static org.eclipse.jgit.storage.pack.PackExt.BITMAP_INDEX;
-import static org.eclipse.jgit.storage.pack.PackExt.PACK;
 import static org.eclipse.jgit.storage.pack.PackExt.INDEX;
+import static org.eclipse.jgit.storage.pack.PackExt.PACK;
 
 import java.io.BufferedInputStream;
 import java.io.EOFException;
@@ -276,8 +277,12 @@ public final class DfsPackFile {
 		}
 	}
 
+	final boolean isGarbage() {
+		return packDesc.getPackSource() == UNREACHABLE_GARBAGE;
+	}
+
 	PackBitmapIndex getBitmapIndex(DfsReader ctx) throws IOException {
-		if (invalid)
+		if (invalid || isGarbage())
 			return null;
 		DfsBlockCache.Ref<PackBitmapIndex> idxref = bitmapIndex;
 		if (idxref != null) {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsReader.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsReader.java
index 7d5fb5b2a5..401c483d4d 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsReader.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsReader.java
@@ -118,6 +118,8 @@ public final class DfsReader extends ObjectReader implements ObjectReuseAsIs {
 
 	private boolean wantReadAhead;
 
+	private boolean avoidUnreachable;
+
 	private List<ReadAheadTask.BlockFuture> pendingReadAhead;
 
 	DfsReader(DfsObjDatabase db) {
@@ -144,6 +146,11 @@ public final class DfsReader extends ObjectReader implements ObjectReuseAsIs {
 	}
 
 	@Override
+	public void setAvoidUnreachableObjects(boolean avoid) {
+		avoidUnreachable = avoid;
+	}
+
+	@Override
 	public BitmapIndex getBitmapIndex() throws IOException {
 		for (DfsPackFile pack : db.getPacks()) {
 			PackBitmapIndex bitmapIndex = pack.getBitmapIndex(this);
@@ -169,8 +176,11 @@ public final class DfsReader extends ObjectReader implements ObjectReuseAsIs {
 			throws IOException {
 		if (id.isComplete())
 			return Collections.singleton(id.toObjectId());
+		boolean noGarbage = avoidUnreachable;
 		HashSet<ObjectId> matches = new HashSet<ObjectId>(4);
 		for (DfsPackFile pack : db.getPacks()) {
+			if (noGarbage && pack.isGarbage())
+				continue;
 			pack.resolve(this, matches, id, 256);
 			if (256 <= matches.size())
 				break;
@@ -182,8 +192,9 @@ public final class DfsReader extends ObjectReader implements ObjectReuseAsIs {
 	public boolean has(AnyObjectId objectId) throws IOException {
 		if (last != null && last.hasObject(this, objectId))
 			return true;
+		boolean noGarbage = avoidUnreachable;
 		for (DfsPackFile pack : db.getPacks()) {
-			if (last == pack)
+			if (pack == last || (noGarbage && pack.isGarbage()))
 				continue;
 			if (pack.hasObject(this, objectId)) {
 				last = pack;
@@ -203,8 +214,9 @@ public final class DfsReader extends ObjectReader implements ObjectReuseAsIs {
 				return ldr;
 		}
 
+		boolean noGarbage = avoidUnreachable;
 		for (DfsPackFile pack : db.getPacks()) {
-			if (pack == last)
+			if (pack == last || (noGarbage && pack.isGarbage()))
 				continue;
 			ObjectLoader ldr = pack.get(this, objectId);
 			if (ldr != null) {
@@ -265,6 +277,7 @@ public final class DfsReader extends ObjectReader implements ObjectReuseAsIs {
 
 		int lastIdx = 0;
 		DfsPackFile lastPack = packList[lastIdx];
+		boolean noGarbage = avoidUnreachable;
 
 		OBJECT_SCAN: for (T t : objectIds) {
 			try {
@@ -281,6 +294,8 @@ public final class DfsReader extends ObjectReader implements ObjectReuseAsIs {
 				if (i == lastIdx)
 					continue;
 				DfsPackFile pack = packList[i];
+				if (noGarbage && pack.isGarbage())
+					continue;
 				try {
 					long p = pack.findOffset(this, t);
 					if (0 < p) {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
index f0ba0cdc5b..12f6886588 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
@@ -792,6 +792,7 @@ public class UploadPack {
 
 		sentReady = false;
 		int haveCnt = 0;
+		walk.getObjectReader().setAvoidUnreachableObjects(true);
 		AsyncRevObjectQueue q = walk.parseAny(peerHas, false);
 		try {
 			for (;;) {
@@ -838,6 +839,7 @@ public class UploadPack {
 			}
 		} finally {
 			q.release();
+			walk.getObjectReader().setAvoidUnreachableObjects(false);
 		}
 
 		int missCnt = peerHas.size() - haveCnt;