From 21e902dd7fa4ff53dc35fd7c48f8b5edc52f8eea Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Wed, 18 May 2022 13:31:30 +0100 Subject: [PATCH] Shortcut during git fetch for avoiding looping through all local refs The FetchProcess needs to verify that all the refs received point to objects that are reachable from the local refs, which could be very expensive but is needed to avoid missing objects exceptions because of broken chains. When the local repository has a lot of refs (e.g. millions) and the client is fetching a non-commit object (e.g. refs/sequences/changes in Gerrit) the reachability check on all local refs can be very expensive compared to the time to fetch the remote ref. Example for a 2M refs repository: - fetching a single non-commit object: 50ms - checking the reachability of local refs: 30s A ref pointing to a non-commit object doesn't have any parent or successor objects, hence would never need to have a reachability check done. Skipping the askForIsComplete() altogether would save the 30s time spent in an unnecessary phase. Signed-off-by: Luca Milanesio Change-Id: I09ac66ded45cede199ba30f9e71cc1055f00941b --- .../eclipse/jgit/transport/FetchProcess.java | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchProcess.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchProcess.java index 2cedd4b07..507795af5 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchProcess.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchProcess.java @@ -47,6 +47,7 @@ import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.RefDatabase; import org.eclipse.jgit.revwalk.ObjectWalk; +import org.eclipse.jgit.revwalk.RevObject; import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.util.StringUtils; @@ -378,11 +379,19 @@ private void updateFETCH_HEAD(FetchResult result) throws IOException { private boolean askForIsComplete() throws TransportException { try { try (ObjectWalk ow = new ObjectWalk(transport.local)) { - for (ObjectId want : askFor.keySet()) - ow.markStart(ow.parseAny(want)); - for (Ref ref : localRefs().values()) - ow.markUninteresting(ow.parseAny(ref.getObjectId())); - ow.checkConnectivity(); + boolean hasCommitObject = false; + for (ObjectId want : askFor.keySet()) { + RevObject obj = ow.parseAny(want); + ow.markStart(obj); + hasCommitObject |= obj.getType() == Constants.OBJ_COMMIT; + } + // Checking connectivity makes sense on commits only + if (hasCommitObject) { + for (Ref ref : localRefs().values()) { + ow.markUninteresting(ow.parseAny(ref.getObjectId())); + } + ow.checkConnectivity(); + } } return true; } catch (MissingObjectException e) {