Speed up clone/fetch with large number of refs

Instead of re-reading all refs after each update, execute
the deletes first, then read all refs once and perform
the check for conflicting ref names in memory.

Change-Id: I17d0b3ccc27f868c8497607d8e57bf7082e65ba3

org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefDirectoryTest.java

@@ -60,6 +60,8 @@ import static org.junit.Assert.fail;
 import java.io.File;
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
@@ -67,17 +69,20 @@ import java.util.concurrent.atomic.AtomicReference;
 import org.eclipse.jgit.events.ListenerHandle;
 import org.eclipse.jgit.events.RefsChangedEvent;
 import org.eclipse.jgit.events.RefsChangedListener;
-import org.eclipse.jgit.internal.storage.file.FileRepository;
-import org.eclipse.jgit.internal.storage.file.RefDirectory;
 import org.eclipse.jgit.junit.LocalDiskRepositoryTestCase;
 import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.lib.AnyObjectId;
+import org.eclipse.jgit.lib.BatchRefUpdate;
+import org.eclipse.jgit.lib.NullProgressMonitor;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Ref.Storage;
 import org.eclipse.jgit.lib.RefDatabase;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevTag;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.transport.ReceiveCommand;
+import org.eclipse.jgit.transport.ReceiveCommand.Type;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -1175,6 +1180,112 @@ public class RefDirectoryTest extends LocalDiskRepositoryTestCase {
 		assertEquals(1, changeCount.get());
 	}
 
+	@Test
+	public void testBatchRefUpdateSimpleNoForce() throws IOException {
+		writeLooseRef("refs/heads/master", A);
+		writeLooseRef("refs/heads/masters", B);
+		List<ReceiveCommand> commands = Arrays.asList(
+				newCommand(A, B, "refs/heads/master",
+						ReceiveCommand.Type.UPDATE),
+				newCommand(B, A, "refs/heads/masters",
+						ReceiveCommand.Type.UPDATE_NONFASTFORWARD));
+		BatchRefUpdate batchUpdate = refdir.newBatchUpdate();
+		batchUpdate.addCommand(commands);
+		batchUpdate
+				.execute(new RevWalk(diskRepo), NullProgressMonitor.INSTANCE);
+		Map<String, Ref> refs = refdir.getRefs(RefDatabase.ALL);
+		assertEquals(ReceiveCommand.Result.OK, commands.get(0).getResult());
+		assertEquals(ReceiveCommand.Result.REJECTED_NONFASTFORWARD, commands
+				.get(1).getResult());
+		assertEquals("[HEAD, refs/heads/master, refs/heads/masters]", refs
+				.keySet().toString());
+		assertEquals(B.getId(), refs.get("refs/heads/master").getObjectId());
+		assertEquals(B.getId(), refs.get("refs/heads/masters").getObjectId());
+	}
+
+	@Test
+	public void testBatchRefUpdateSimpleForce() throws IOException {
+		writeLooseRef("refs/heads/master", A);
+		writeLooseRef("refs/heads/masters", B);
+		List<ReceiveCommand> commands = Arrays.asList(
+				newCommand(A, B, "refs/heads/master",
+						ReceiveCommand.Type.UPDATE),
+				newCommand(B, A, "refs/heads/masters",
+						ReceiveCommand.Type.UPDATE_NONFASTFORWARD));
+		BatchRefUpdate batchUpdate = refdir.newBatchUpdate();
+		batchUpdate.setAllowNonFastForwards(true);
+		batchUpdate.addCommand(commands);
+		batchUpdate
+				.execute(new RevWalk(diskRepo), NullProgressMonitor.INSTANCE);
+		Map<String, Ref> refs = refdir.getRefs(RefDatabase.ALL);
+		assertEquals(ReceiveCommand.Result.OK, commands.get(0).getResult());
+		assertEquals(ReceiveCommand.Result.OK, commands.get(1).getResult());
+		assertEquals("[HEAD, refs/heads/master, refs/heads/masters]", refs
+				.keySet().toString());
+		assertEquals(B.getId(), refs.get("refs/heads/master").getObjectId());
+		assertEquals(A.getId(), refs.get("refs/heads/masters").getObjectId());
+	}
+
+	@Test
+	public void testBatchRefUpdateConflict() throws IOException {
+		writeLooseRef("refs/heads/master", A);
+		writeLooseRef("refs/heads/masters", B);
+		List<ReceiveCommand> commands = Arrays.asList(
+				newCommand(A, B, "refs/heads/master",
+						ReceiveCommand.Type.UPDATE),
+				newCommand(null, A, "refs/heads/master/x",
+						ReceiveCommand.Type.CREATE),
+				newCommand(null, A, "refs/heads", ReceiveCommand.Type.CREATE));
+		BatchRefUpdate batchUpdate = refdir.newBatchUpdate();
+		batchUpdate.setAllowNonFastForwards(true);
+		batchUpdate.addCommand(commands);
+		batchUpdate
+				.execute(new RevWalk(diskRepo), NullProgressMonitor.INSTANCE);
+		Map<String, Ref> refs = refdir.getRefs(RefDatabase.ALL);
+		assertEquals(ReceiveCommand.Result.OK, commands.get(0).getResult());
+		assertEquals(ReceiveCommand.Result.LOCK_FAILURE, commands.get(1)
+				.getResult());
+		assertEquals(ReceiveCommand.Result.LOCK_FAILURE, commands.get(2)
+				.getResult());
+		assertEquals("[HEAD, refs/heads/master, refs/heads/masters]", refs
+				.keySet().toString());
+		assertEquals(B.getId(), refs.get("refs/heads/master").getObjectId());
+		assertEquals(B.getId(), refs.get("refs/heads/masters").getObjectId());
+	}
+
+	@Test
+	public void testBatchRefUpdateConflictThanksToDelete() throws IOException {
+		writeLooseRef("refs/heads/master", A);
+		writeLooseRef("refs/heads/masters", B);
+		List<ReceiveCommand> commands = Arrays.asList(
+				newCommand(A, B, "refs/heads/master",
+						ReceiveCommand.Type.UPDATE),
+				newCommand(null, A, "refs/heads/masters/x",
+						ReceiveCommand.Type.CREATE),
+				newCommand(B, null, "refs/heads/masters",
+						ReceiveCommand.Type.DELETE));
+		BatchRefUpdate batchUpdate = refdir.newBatchUpdate();
+		batchUpdate.setAllowNonFastForwards(true);
+		batchUpdate.addCommand(commands);
+		batchUpdate
+				.execute(new RevWalk(diskRepo), NullProgressMonitor.INSTANCE);
+		Map<String, Ref> refs = refdir.getRefs(RefDatabase.ALL);
+		assertEquals(ReceiveCommand.Result.OK, commands.get(0).getResult());
+		assertEquals(ReceiveCommand.Result.OK, commands.get(1).getResult());
+		assertEquals(ReceiveCommand.Result.OK, commands.get(2).getResult());
+		assertEquals("[HEAD, refs/heads/master, refs/heads/masters/x]", refs
+				.keySet().toString());
+		assertEquals(A.getId(), refs.get("refs/heads/masters/x").getObjectId());
+	}
+
+	private static ReceiveCommand newCommand(RevCommit a, RevCommit b,
+			String string, Type update) {
+		ReceiveCommand ret = new ReceiveCommand(a != null ? a.getId() : null,
+				b != null ? b.getId() : null, string, update);
+		ret.setResult(ReceiveCommand.Result.NOT_ATTEMPTED);
+		return ret;
+	}
+
 	private void writeLooseRef(String name, AnyObjectId id) throws IOException {
 		writeLooseRef(name, id.name() + "\n");
 	}

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

@@ -53,9 +53,11 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 
 import org.eclipse.jgit.internal.JGitText;
+import org.eclipse.jgit.lib.RefUpdate.Result;
 import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.transport.ReceiveCommand;
 
@@ -242,33 +244,86 @@ public class BatchRefUpdate {
 	 * @param walk
 	 *            a RevWalk to parse tags in case the storage system wants to
 	 *            store them pre-peeled, a common performance optimization.
-	 * @param update
+	 * @param monitor
 	 *            progress monitor to receive update status on.
 	 * @throws IOException
 	 *             the database is unable to accept the update. Individual
 	 *             command status must be tested to determine if there is a
 	 *             partial failure, or a total failure.
 	 */
-	public void execute(RevWalk walk, ProgressMonitor update)
+	public void execute(RevWalk walk, ProgressMonitor monitor)
 			throws IOException {
-		update.beginTask(JGitText.get().updatingReferences, commands.size());
+		monitor.beginTask(JGitText.get().updatingReferences, commands.size());
+		List<ReceiveCommand> commands2 = new ArrayList<ReceiveCommand>(
+				commands.size());
+		List<String> namesToCheck = new ArrayList<String>(commands.size());
+		// First delete refs. This may free the name space for some of the
+		// updates.
 		for (ReceiveCommand cmd : commands) {
 			try {
-				update.update(1);
-
 				if (cmd.getResult() == NOT_ATTEMPTED) {
 					cmd.updateType(walk);
-					RefUpdate ru = newUpdate(cmd);
 					switch (cmd.getType()) {
+					case CREATE:
+						namesToCheck.add(cmd.getRefName());
+						commands2.add(cmd);
+						break;
+					case UPDATE:
+					case UPDATE_NONFASTFORWARD:
+						commands2.add(cmd);
+						break;
 					case DELETE:
-						cmd.setResult(ru.delete(walk));
+						RefUpdate rud = newUpdate(cmd);
-						continue;
+						monitor.update(1);
+						cmd.setResult(rud.delete(walk));
+					}
+				}
+			} catch (IOException err) {
+				cmd.setResult(
+						REJECTED_OTHER_REASON,
+						MessageFormat.format(JGitText.get().lockError,
+								err.getMessage()));
+			}
+		}
+		if (!commands2.isEmpty()) {
+			// What part of the name space is already taken
+			Collection<String> takenNames = new HashSet<String>(refdb.getRefs(
+					RefDatabase.ALL).keySet());
+			Collection<String> takenPrefixes = getTakenPrefixes(takenNames);
 
-					case CREATE:
+			// Now to the update that may require more room in the name space
+			for (ReceiveCommand cmd : commands2) {
+				try {
+					monitor.update(1);
+
+					if (cmd.getResult() == NOT_ATTEMPTED) {
+						cmd.updateType(walk);
+						RefUpdate ru = newUpdate(cmd);
+						SWITCH: switch (cmd.getType()) {
+						case DELETE:
+							// Performed in the first phase
+							break;
 						case UPDATE:
 						case UPDATE_NONFASTFORWARD:
+							monitor.update(1);
+							RefUpdate ruu = newUpdate(cmd);
+							cmd.setResult(ruu.update(walk));
+							break;
+						case CREATE:
+							for (String prefix : getPrefixes(cmd.getRefName())) {
+								if (takenNames.contains(prefix)) {
+									cmd.setResult(Result.LOCK_FAILURE);
+									break SWITCH;
+								}
+							}
+							if (takenPrefixes.contains(cmd.getRefName())) {
+								cmd.setResult(Result.LOCK_FAILURE);
+								break SWITCH;
+							}
+							ru.setCheckConflicting(false);
+							addRefToPrefixes(takenPrefixes, cmd.getRefName());
+							takenNames.add(cmd.getRefName());
 							cmd.setResult(ru.update(walk));
-						continue;
 						}
 					}
 				} catch (IOException err) {
@@ -276,7 +331,33 @@ public class BatchRefUpdate {
 							JGitText.get().lockError, err.getMessage()));
 				}
 			}
-		update.endTask();
+		}
+		monitor.endTask();
+	}
+
+	private static Collection<String> getTakenPrefixes(
+			final Collection<String> names) {
+		Collection<String> ref = new HashSet<String>();
+		for (String name : names)
+			ref.addAll(getPrefixes(name));
+		return ref;
+	}
+
+	private static void addRefToPrefixes(Collection<String> prefixes,
+			String name) {
+		for (String prefix : getPrefixes(name)) {
+			prefixes.add(prefix);
+		}
+	}
+
+	static Collection<String> getPrefixes(String s) {
+		Collection<String> ret = new HashSet<String>();
+		int p1 = s.indexOf('/');
+		while (p1 > 0) {
+			ret.add(s.substring(0, p1));
+			p1 = s.indexOf('/', p1 + 1);
+		}
+		return ret;
 	}
 
 	/**

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

@@ -178,6 +178,8 @@ public abstract class RefUpdate {
 	 */
 	private boolean detachingSymbolicRef;
 
+	private boolean checkConflicting = true;
+
 	/**
 	 * Construct a new update operation for the reference.
 	 * <p>
@@ -564,7 +566,7 @@ public abstract class RefUpdate {
 	public Result link(String target) throws IOException {
 		if (!target.startsWith(Constants.R_REFS))
 			throw new IllegalArgumentException(MessageFormat.format(JGitText.get().illegalArgumentNotA, Constants.R_REFS));
-		if (getRefDatabase().isNameConflicting(getName()))
+		if (checkConflicting && getRefDatabase().isNameConflicting(getName()))
 			return Result.LOCK_FAILURE;
 		try {
 			if (!tryLock(false))
@@ -599,7 +601,7 @@ public abstract class RefUpdate {
 		RevObject oldObj;
 
 		// don't make expensive conflict check if this is an existing Ref
-		if (oldValue == null && getRefDatabase().isNameConflicting(getName()))
+		if (oldValue == null && checkConflicting && getRefDatabase().isNameConflicting(getName()))
 			return Result.LOCK_FAILURE;
 		try {
 			if (!tryLock(true))
@@ -631,6 +633,16 @@ public abstract class RefUpdate {
 		}
 	}
 
+	/**
+	 * Enable/disable the check for conflicting ref names. By default conflicts
+	 * are checked explicitly.
+	 *
+	 * @param check
+	 */
+	public void setCheckConflicting(boolean check) {
+		checkConflicting = check;
+	}
+
 	private static RevObject safeParse(final RevWalk rw, final AnyObjectId id)
 			throws IOException {
 		try {