From 7884873160727069944afc4e2b36046aa2cff550 Mon Sep 17 00:00:00 2001
From: Dariusz Luksza <dariusz.luksza@gmail.com>
Date: Mon, 18 Dec 2023 18:24:49 +0000
Subject: [PATCH] BasePackFetchConnection: Skip object/ref lookups if local
 repo is empty

When cloning repository some of the operations in
`BasePackFetchConnection` can be skipped. We don't need to advertise
packs, compute "wanted time" or wanted refs to send. All of those
operations will try to read objects from an empty repository which
always results in a missing object.

This saves 99.9% of `LooseObjects.open()` calls which dramatically
speeds up object negotiation in V2 protocol.

In testing on JGit (v6.8.0.202311291450-r) repository, which contains
564 refs, the number of calls to `LooseObjects.open()` was reduced from
1187 to 1.

Skipping a call to `markRefsAdvertised()` initially reduced be above
number to 623. Then assuming "0" "want time" an on empty repository
pushed the calls down to 312. Finally, skipping objects reachability on
empty repository set calls down to 1.

The last call is performed from `FetchProcess.asForIsComplete()` which
probably needs to stay in place.

Bug: jgit-5
Change-Id: I2480690726ea54d3b1593380bc8f8d15b4d6afc6
---
 .../transport/BasePackFetchConnection.java    | 29 +++++++++++++------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java
index e0a351772b..469a3d6015 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java
@@ -402,11 +402,14 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 	protected void doFetch(final ProgressMonitor monitor,
 			final Collection<Ref> want, final Set<ObjectId> have,
 			OutputStream outputStream) throws TransportException {
+		boolean hasObjects = !have.isEmpty();
 		try {
 			noProgress = monitor == NullProgressMonitor.INSTANCE;
 
-			markRefsAdvertised();
-			markReachable(want, have, maxTimeWanted(want));
+			if (hasObjects) {
+				markRefsAdvertised();
+			}
+			markReachable(want, have, maxTimeWanted(want, hasObjects));
 
 			if (TransferConfig.ProtocolVersion.V2
 					.equals(getProtocolVersion())) {
@@ -418,7 +421,7 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 				state = new TemporaryBuffer.Heap(Integer.MAX_VALUE);
 				pckState = new PacketLineOut(state);
 				try {
-					doFetchV2(monitor, want, outputStream);
+					doFetchV2(monitor, want, outputStream, hasObjects);
 				} finally {
 					clearState();
 				}
@@ -430,7 +433,7 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 				pckState = new PacketLineOut(state);
 			}
 			PacketLineOut output = statelessRPC ? pckState : pckOut;
-			if (sendWants(want, output)) {
+			if (sendWants(want, output, hasObjects)) {
 				boolean mayHaveShallow = depth != null || deepenSince != null || !deepenNots.isEmpty();
 				Set<ObjectId> shallowCommits = local.getObjectDatabase().getShallowCommits();
 				if (isCapableOf(GitProtocolConstants.CAPABILITY_SHALLOW)) {
@@ -457,7 +460,8 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 	}
 
 	private void doFetchV2(ProgressMonitor monitor, Collection<Ref> want,
-			OutputStream outputStream) throws IOException, CancelledException {
+			OutputStream outputStream, boolean hasObjects)
+			throws IOException, CancelledException {
 		sideband = true;
 		negotiateBegin();
 
@@ -479,7 +483,7 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 			pckState.writeString(capability);
 		}
 
-		if (!sendWants(want, pckState)) {
+		if (!sendWants(want, pckState, hasObjects)) {
 			// We already have everything we wanted.
 			return;
 		}
@@ -666,8 +670,12 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 		return local.getConfig().get(FetchConfig::new);
 	}
 
-	private int maxTimeWanted(Collection<Ref> wants) {
+	private int maxTimeWanted(Collection<Ref> wants, boolean hasObjects) {
 		int maxTime = 0;
+		if (!hasObjects) {
+			// we don't have any objects locally, we can immediately bail out
+			return maxTime;
+		}
 		for (Ref r : wants) {
 			try {
 				final RevObject obj = walk.parseAny(r.getObjectId());
@@ -769,7 +777,8 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 		}
 	}
 
-	private boolean sendWants(Collection<Ref> want, PacketLineOut p)
+	private boolean sendWants(Collection<Ref> want, PacketLineOut p,
+			boolean hasObjects)
 			throws IOException {
 		boolean first = true;
 		for (Ref r : want) {
@@ -778,7 +787,9 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 				continue;
 			}
 			// if depth is set we need to fetch the objects even if they are already available
-			if (transport.getDepth() == null) {
+			if (transport.getDepth() == null
+					// only check reachable objects when we have objects
+					&& hasObjects) {
 				try {
 					if (walk.parseAny(objectId).has(REACHABLE)) {
 						// We already have this object. Asking for it is
-- 
2.39.5