Merge changes Icd550359,If7aad533

* changes:
  Avoid looking at UNREACHABLE_GARBAGE for client have lines
  Simplify UploadPack by parsing wants separately from haves

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

@@ -436,6 +436,19 @@ public abstract class ObjectReader {
 		// Do nothing by default, most readers don't want or need advice.
 	}
 
+	/**
+	 * 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.
 	 *

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) {

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) {
@@ -143,6 +145,11 @@ public final class DfsReader extends ObjectReader implements ObjectReuseAsIs {
 		return new DfsReader(db);
 	}
 
+	@Override
+	public void setAvoidUnreachableObjects(boolean avoid) {
+		avoidUnreachable = avoid;
+	}
+
 	@Override
 	public BitmapIndex getBitmapIndex() throws IOException {
 		for (DfsPackFile pack : db.getPacks()) {
@@ -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) {

org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java

@@ -787,74 +787,24 @@ public class UploadPack {
 		preUploadHook.onBeginNegotiateRound(this, wantIds, peerHas.size());
 		if (peerHas.isEmpty())
 			return last;
+		if (wantAll.isEmpty() && !wantIds.isEmpty())
+			parseWants();
 
-		List<ObjectId> toParse = peerHas;
-		HashSet<ObjectId> peerHasSet = null;
-		boolean needMissing = false;
 		sentReady = false;
-
-		if (wantAll.isEmpty() && !wantIds.isEmpty()) {
-			// We have not yet parsed the want list. Parse it now.
-			peerHasSet = new HashSet<ObjectId>(peerHas);
-			int cnt = wantIds.size() + peerHasSet.size();
-			toParse = new ArrayList<ObjectId>(cnt);
-			toParse.addAll(wantIds);
-			toParse.addAll(peerHasSet);
-			needMissing = true;
-		}
-
-		Set<RevObject> notAdvertisedWants = null;
 		int haveCnt = 0;
-		AsyncRevObjectQueue q = walk.parseAny(toParse, needMissing);
+		walk.getObjectReader().setAvoidUnreachableObjects(true);
+		AsyncRevObjectQueue q = walk.parseAny(peerHas, false);
 		try {
 			for (;;) {
 				RevObject obj;
 				try {
 					obj = q.next();
 				} catch (MissingObjectException notFound) {
-					ObjectId id = notFound.getObjectId();
-					if (wantIds.contains(id)) {
-						String msg = MessageFormat.format(
-								JGitText.get().wantNotValid, id.name());
-						throw new PackProtocolException(msg, notFound);
-					}
 					continue;
 				}
 				if (obj == null)
 					break;
 
-				// If the object is still found in wantIds, the want
-				// list wasn't parsed earlier, and was done in this batch.
-				//
-				if (wantIds.remove(obj)) {
-					if (!advertised.contains(obj) && requestPolicy != RequestPolicy.ANY) {
-						if (notAdvertisedWants == null)
-							notAdvertisedWants = new HashSet<RevObject>();
-						notAdvertisedWants.add(obj);
-					}
-
-					if (!obj.has(WANT)) {
-						obj.add(WANT);
-						wantAll.add(obj);
-					}
-
-					if (!(obj instanceof RevCommit))
-						obj.add(SATISFIED);
-
-					if (obj instanceof RevTag) {
-						RevObject target = walk.peel(obj);
-						if (target instanceof RevCommit) {
-							if (!target.has(WANT)) {
-								target.add(WANT);
-								wantAll.add(target);
-							}
-						}
-					}
-
-					if (!peerHasSet.contains(obj))
-						continue;
-				}
-
 				last = obj;
 				haveCnt++;
 
@@ -889,25 +839,7 @@ public class UploadPack {
 			}
 		} finally {
 			q.release();
-		}
+			walk.getObjectReader().setAvoidUnreachableObjects(false);
-
-		// If the client asked for non advertised object, check our policy.
-		if (notAdvertisedWants != null && !notAdvertisedWants.isEmpty()) {
-			switch (requestPolicy) {
-			case ADVERTISED:
-			default:
-				throw new PackProtocolException(MessageFormat.format(
-						JGitText.get().wantNotValid,
-						notAdvertisedWants.iterator().next().name()));
-
-			case REACHABLE_COMMIT:
-				checkNotAdvertisedWants(notAdvertisedWants);
-				break;
-
-			case ANY:
-				// Allow whatever was asked for.
-				break;
-			}
 		}
 
 		int missCnt = peerHas.size() - haveCnt;
@@ -952,7 +884,61 @@ public class UploadPack {
 		return last;
 	}
 
-	private void checkNotAdvertisedWants(Set<RevObject> notAdvertisedWants)
+	private void parseWants() throws IOException {
+		AsyncRevObjectQueue q = walk.parseAny(wantIds, true);
+		try {
+			List<RevCommit> checkReachable = null;
+			RevObject obj;
+			while ((obj = q.next()) != null) {
+				if (!advertised.contains(obj)) {
+					switch (requestPolicy) {
+					case ADVERTISED:
+					default:
+						throw new PackProtocolException(MessageFormat.format(
+								JGitText.get().wantNotValid, obj));
+					case REACHABLE_COMMIT:
+						if (!(obj instanceof RevCommit)) {
+							throw new PackProtocolException(MessageFormat.format(
+								JGitText.get().wantNotValid, obj));
+						}
+						if (checkReachable == null)
+							checkReachable = new ArrayList<RevCommit>();
+						checkReachable.add((RevCommit) obj);
+						break;
+					case ANY:
+						break;
+					}
+				}
+				want(obj);
+
+				if (!(obj instanceof RevCommit))
+					obj.add(SATISFIED);
+				if (obj instanceof RevTag) {
+					obj = walk.peel(obj);
+					if (obj instanceof RevCommit)
+						want(obj);
+				}
+			}
+			if (checkReachable != null)
+				checkNotAdvertisedWants(checkReachable);
+			wantIds.clear();
+		} catch (MissingObjectException notFound) {
+			ObjectId id = notFound.getObjectId();
+			throw new PackProtocolException(MessageFormat.format(
+					JGitText.get().wantNotValid, id.name()), notFound);
+		} finally {
+			q.release();
+		}
+	}
+
+	private void want(RevObject obj) {
+		if (!obj.has(WANT)) {
+			obj.add(WANT);
+			wantAll.add(obj);
+		}
+	}
+
+	private void checkNotAdvertisedWants(List<RevCommit> notAdvertisedWants)
 			throws MissingObjectException, IncorrectObjectTypeException, IOException {
 		// Walk the requested commits back to the advertised commits.
 		// If any commit exists, a branch was deleted or rewound and
@@ -960,15 +946,8 @@ public class UploadPack {
 		// If the requested commit is merged into an advertised branch
 		// it will be marked UNINTERESTING and no commits return.
 
-		for (RevObject o : notAdvertisedWants) {
+		for (RevCommit c : notAdvertisedWants)
-			if (!(o instanceof RevCommit)) {
+			walk.markStart(c);
-				throw new PackProtocolException(MessageFormat.format(
-						JGitText.get().wantNotValid,
-						notAdvertisedWants.iterator().next().name()));
-			}
-			walk.markStart((RevCommit) o);
-		}
-
 		for (ObjectId id : advertised) {
 			try {
 				walk.markUninteresting(walk.parseCommit(id));