summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorShawn O. Pearce <spearce@spearce.org>2011-02-03 12:37:39 -0800
committerShawn O. Pearce <spearce@spearce.org>2011-02-04 09:10:23 -0800
commit2096c749c30c2420362859383bbdd1e307790569 (patch)
tree390cef109ded089b8d94ab12baeb4154c2dc5bf7
parenta3620cbbe144c9b666ce4472fd5e8ef1222bd641 (diff)
downloadjgit-2096c749c30c2420362859383bbdd1e307790569.tar.gz
jgit-2096c749c30c2420362859383bbdd1e307790569.zip
UploadPack: Avoid parsing want list on clone
If a client wants to perform a clone of the repository, it sends wants, but no haves. There is no point in parsing the want list within UploadPack, as there won't be a common merge base search. Instead just defer the parsing to PackWriter, which will do its own parsing and object enumeration. If the client does have a "have" set, defer parsing of the want list until the have list is also parsed, and parse them together in a single batch queue. This lets the underlying storage system use a larger lookup batch if there is significant latency involved when resolving an ObjectId to a RevObject. Change-Id: I9c30d34f8e344da05c8a2c041a6dc181d8e8bc19 Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
-rw-r--r--org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java106
1 files changed, 60 insertions, 46 deletions
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 81da24792e..4ff1c5c5a3 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
@@ -49,6 +49,7 @@ import java.io.InputStream;
import java.io.OutputStream;
import java.text.MessageFormat;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
@@ -143,6 +144,9 @@ public class UploadPack {
/** Capabilities requested by the client. */
private final Set<String> options = new HashSet<String>();
+ /** Raw ObjectIds the client has asked for, before validating them. */
+ private final Set<ObjectId> wantIds = new HashSet<ObjectId>();
+
/** Objects the client wants to obtain. */
private final List<RevObject> wantAll = new ArrayList<RevObject>();
@@ -335,7 +339,7 @@ public class UploadPack {
}
recvWants();
- if (wantAll.isEmpty())
+ if (wantIds.isEmpty())
return;
if (options.contains(OPTION_MULTI_ACK_DETAILED))
@@ -374,7 +378,6 @@ public class UploadPack {
}
private void recvWants() throws IOException {
- HashSet<ObjectId> wantIds = new HashSet<ObjectId>();
boolean isFirst = true;
for (;;) {
String line;
@@ -403,46 +406,6 @@ public class UploadPack {
wantIds.add(ObjectId.fromString(line.substring(5)));
isFirst = false;
}
-
- if (wantIds.isEmpty())
- return;
-
- AsyncRevObjectQueue q = walk.parseAny(wantIds, true);
- try {
- for (;;) {
- RevObject o;
- try {
- o = q.next();
- } catch (IOException error) {
- throw new PackProtocolException(MessageFormat.format(
- JGitText.get().notValid, error.getMessage()), error);
- }
- if (o == null)
- break;
- if (o.has(WANT)) {
- // Already processed, the client repeated itself.
-
- } else if (advertised.contains(o)) {
- o.add(WANT);
- wantAll.add(o);
-
- if (o instanceof RevTag) {
- o = walk.peel(o);
- if (o instanceof RevCommit) {
- if (!o.has(WANT)) {
- o.add(WANT);
- wantAll.add(o);
- }
- }
- }
- } else {
- throw new PackProtocolException(MessageFormat.format(
- JGitText.get().notValid, o.name()));
- }
- }
- } finally {
- q.release();
- }
}
private boolean negotiate() throws IOException {
@@ -489,20 +452,65 @@ public class UploadPack {
if (peerHas.isEmpty())
return last;
- // If both sides have the same object; let the client know.
- //
- AsyncRevObjectQueue q = walk.parseAny(peerHas, false);
+ List<ObjectId> toParse = peerHas;
+ HashSet<ObjectId> peerHasSet = null;
+ boolean needMissing = 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;
+ }
+
+ AsyncRevObjectQueue q = walk.parseAny(toParse, needMissing);
try {
for (;;) {
RevObject obj;
try {
obj = q.next();
} catch (MissingObjectException notFound) {
+ if (wantIds.contains(notFound.getObjectId())) {
+ throw new PackProtocolException(
+ MessageFormat.format(JGitText.get().notValid,
+ notFound.getMessage()), 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)) {
+ throw new PackProtocolException(MessageFormat.format(
+ JGitText.get().notValid, obj.name()));
+ }
+
+ if (!obj.has(WANT)) {
+ obj.add(WANT);
+ wantAll.add(obj);
+ }
+
+ 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;
if (obj.has(PEER_HAS))
continue;
@@ -512,6 +520,8 @@ public class UploadPack {
((RevCommit) obj).carry(PEER_HAS);
addCommonBase(obj);
+ // If both sides have the same object; let the client know.
+ //
switch (multiAck) {
case OFF:
if (commonBase.size() == 1)
@@ -631,6 +641,10 @@ public class UploadPack {
}
}
+ Collection<? extends ObjectId> want = wantAll;
+ if (want.isEmpty())
+ want = wantIds;
+
PackConfig cfg = packConfig;
if (cfg == null)
cfg = new PackConfig(db);
@@ -639,7 +653,7 @@ public class UploadPack {
pw.setUseCachedPacks(true);
pw.setDeltaBaseAsOffset(options.contains(OPTION_OFS_DELTA));
pw.setThin(options.contains(OPTION_THIN_PACK));
- pw.preparePack(pm, wantAll, commonBase);
+ pw.preparePack(pm, want, commonBase);
if (options.contains(OPTION_INCLUDE_TAG)) {
for (final Ref r : refs.values()) {
final RevObject o;