Use BatchRefUpdate for tracking refs in FetchProcess

If there are a lot of references to modify, using BatchRefUpdate can
save time if the underlying storage is able to combine these updates
together. This should speed up initial clone or fetch into an empty
repository, as some projects can have hundreds of release tags, or
hundreds of branch heads.

Change-Id: Iee9af8d5fa19080077d88357c18853540936e940

org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/TransportTest.java

@@ -56,6 +56,7 @@ import java.util.Collections;
 import java.util.List;
 
 import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.SampleDataRepositoryTestCase;
 import org.junit.After;
 import org.junit.Before;
@@ -209,7 +210,7 @@ public class TransportTest extends SampleDataRepositoryTestCase {
 		assertEquals("refs/remotes/test/a", tru.getLocalName());
 		assertEquals("refs/heads/a", tru.getRemoteName());
 		assertEquals(db.resolve("refs/heads/a"), tru.getNewObjectId());
-		assertNull(tru.getOldObjectId());
+		assertEquals(ObjectId.zeroId(), tru.getOldObjectId());
 	}
 
 	@Test

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

@@ -44,6 +44,11 @@
 
 package org.eclipse.jgit.transport;
 
+import static org.eclipse.jgit.transport.ReceiveCommand.Result.NOT_ATTEMPTED;
+import static org.eclipse.jgit.transport.ReceiveCommand.Result.OK;
+import static org.eclipse.jgit.transport.ReceiveCommand.Result.REJECTED_NONFASTFORWARD;
+import static org.eclipse.jgit.transport.ReceiveCommand.Type.UPDATE_NONFASTFORWARD;
+
 import java.io.File;
 import java.io.IOException;
 import java.io.OutputStreamWriter;
@@ -63,12 +68,13 @@ import org.eclipse.jgit.errors.MissingObjectException;
 import org.eclipse.jgit.errors.NotSupportedException;
 import org.eclipse.jgit.errors.TransportException;
 import org.eclipse.jgit.internal.JGitText;
+import org.eclipse.jgit.lib.BatchRefUpdate;
 import org.eclipse.jgit.lib.BatchingProgressMonitor;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ProgressMonitor;
 import org.eclipse.jgit.lib.Ref;
-import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.lib.RefDatabase;
 import org.eclipse.jgit.revwalk.ObjectWalk;
 import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.storage.file.LockFile;
@@ -97,6 +103,8 @@ class FetchProcess {
 
 	private FetchConnection conn;
 
+	private Map<String, Ref> localRefs;
+
 	FetchProcess(final Transport t, final Collection<RefSpec> f) {
 		transport = t;
 		toFetch = f;
@@ -108,6 +116,7 @@ class FetchProcess {
 		localUpdates.clear();
 		fetchHeadUpdates.clear();
 		packLocks.clear();
+		localRefs = null;
 
 		try {
 			executeImp(monitor, result);
@@ -183,27 +192,40 @@ class FetchProcess {
 			closeConnection(result);
 		}
 
+		BatchRefUpdate batch = transport.local.getRefDatabase()
+				.newBatchUpdate()
+				.setAllowNonFastForwards(true)
+				.setRefLogMessage("fetch", true);
 		final RevWalk walk = new RevWalk(transport.local);
 		try {
 			if (monitor instanceof BatchingProgressMonitor) {
 				((BatchingProgressMonitor) monitor).setDelayStart(
 						250, TimeUnit.MILLISECONDS);
 			}
-			monitor.beginTask(JGitText.get().updatingReferences, localUpdates.size());
 			if (transport.isRemoveDeletedRefs())
-				deleteStaleTrackingRefs(result, walk);
+				deleteStaleTrackingRefs(result, batch);
 			for (TrackingRefUpdate u : localUpdates) {
-				try {
+				result.add(u);
-					monitor.update(1);
+				batch.addCommand(u.asReceiveCommand());
-					u.update(walk);
-					result.add(u);
-				} catch (IOException err) {
-					throw new TransportException(MessageFormat.format(JGitText
-							.get().failureUpdatingTrackingRef,
-							u.getLocalName(), err.getMessage()), err);
-				}
 			}
-			monitor.endTask();
+			for (ReceiveCommand cmd : batch.getCommands()) {
+				cmd.updateType(walk);
+				if (cmd.getType() == UPDATE_NONFASTFORWARD
+						&& cmd instanceof TrackingRefUpdate.Command
+						&& !((TrackingRefUpdate.Command) cmd).canForceUpdate())
+					cmd.setResult(REJECTED_NONFASTFORWARD);
+			}
+			if (transport.isDryRun()) {
+				for (ReceiveCommand cmd : batch.getCommands()) {
+					if (cmd.getResult() == NOT_ATTEMPTED)
+						cmd.setResult(OK);
+				}
+			} else
+				batch.execute(walk, monitor);
+		} catch (IOException err) {
+			throw new TransportException(MessageFormat.format(
+					JGitText.get().failureUpdatingTrackingRef,
+					getFirstFailedRefName(batch), err.getMessage()), err);
 		} finally {
 			walk.release();
 		}
@@ -320,7 +342,7 @@ class FetchProcess {
 			try {
 				for (final ObjectId want : askFor.keySet())
 					ow.markStart(ow.parseAny(want));
-				for (final Ref ref : transport.local.getAllRefs().values())
+				for (final Ref ref : localRefs().values())
 					ow.markUninteresting(ow.parseAny(ref.getObjectId()));
 				ow.checkConnectivity();
 			} finally {
@@ -354,7 +376,7 @@ class FetchProcess {
 
 	private Collection<Ref> expandAutoFollowTags() throws TransportException {
 		final Collection<Ref> additionalTags = new ArrayList<Ref>();
-		final Map<String, Ref> haveRefs = transport.local.getAllRefs();
+		final Map<String, Ref> haveRefs = localRefs();
 		for (final Ref r : conn.getRefs()) {
 			if (!isTag(r))
 				continue;
@@ -385,7 +407,7 @@ class FetchProcess {
 	}
 
 	private void expandFetchTags() throws TransportException {
-		final Map<String, Ref> haveRefs = transport.local.getAllRefs();
+		final Map<String, Ref> haveRefs = localRefs();
 		for (final Ref r : conn.getRefs()) {
 			if (!isTag(r))
 				continue;
@@ -404,17 +426,10 @@ class FetchProcess {
 			throws TransportException {
 		final ObjectId newId = src.getObjectId();
 		if (spec.getDestination() != null) {
-			try {
+			final TrackingRefUpdate tru = createUpdate(spec, newId);
-				final TrackingRefUpdate tru = createUpdate(spec, newId);
+			if (newId.equals(tru.getOldObjectId()))
-				if (newId.equals(tru.getOldObjectId()))
+				return;
-					return;
+			localUpdates.add(tru);
-				localUpdates.add(tru);
-			} catch (IOException err) {
-				// Bad symbolic ref? That is the most likely cause.
-				//
-				throw new TransportException( MessageFormat.format(
-						JGitText.get().cannotResolveLocalTrackingRefForUpdating, spec.getDestination()), err);
-			}
 		}
 
 		askFor.put(newId, src);
@@ -427,21 +442,41 @@ class FetchProcess {
 		fetchHeadUpdates.add(fhr);
 	}
 
-	private TrackingRefUpdate createUpdate(final RefSpec spec,
+	private TrackingRefUpdate createUpdate(RefSpec spec, ObjectId newId)
-			final ObjectId newId) throws IOException {
+			throws TransportException {
-		return new TrackingRefUpdate(transport.local, spec, newId, "fetch");
+		Ref ref = localRefs().get(spec.getDestination());
+		ObjectId oldId = ref != null && ref.getObjectId() != null
+				? ref.getObjectId()
+				: ObjectId.zeroId();
+		return new TrackingRefUpdate(
+				spec.isForceUpdate(),
+				spec.getSource(),
+				spec.getDestination(),
+				oldId,
+				newId);
+	}
+
+	private Map<String, Ref> localRefs() throws TransportException {
+		if (localRefs == null) {
+			try {
+				localRefs = transport.local.getRefDatabase()
+						.getRefs(RefDatabase.ALL);
+			} catch (IOException err) {
+				throw new TransportException(JGitText.get().cannotListRefs, err);
+			}
+		}
+		return localRefs;
 	}
 
-	private void deleteStaleTrackingRefs(final FetchResult result,
+	private void deleteStaleTrackingRefs(FetchResult result,
-			final RevWalk walk) throws TransportException {
+			BatchRefUpdate batch) throws IOException {
-		final Repository db = transport.local;
+		for (final Ref ref : localRefs().values()) {
-		for (final Ref ref : db.getAllRefs().values()) {
 			final String refname = ref.getName();
 			for (final RefSpec spec : toFetch) {
 				if (spec.matchDestination(refname)) {
 					final RefSpec s = spec.expandFromDestination(refname);
 					if (result.getAdvertisedRef(s.getSource()) == null) {
-						deleteTrackingRef(result, db, walk, s, ref);
+						deleteTrackingRef(result, batch, s, ref);
 					}
 				}
 			}
@@ -449,31 +484,17 @@ class FetchProcess {
 	}
 
 	private void deleteTrackingRef(final FetchResult result,
-			final Repository db, final RevWalk walk, final RefSpec spec,
+			final BatchRefUpdate batch, final RefSpec spec, final Ref localRef) {
-			final Ref localRef) throws TransportException {
+		if (localRef.getObjectId() == null)
-		final String name = localRef.getName();
+			return;
-		try {
+		TrackingRefUpdate update = new TrackingRefUpdate(
-			final TrackingRefUpdate u = new TrackingRefUpdate(db, name, spec
+				true,
-					.getSource(), true, ObjectId.zeroId(), "deleted");
+				spec.getSource(),
-			result.add(u);
+				localRef.getName(),
-			if (transport.isDryRun()){
+				localRef.getObjectId(),
-				return;
+				ObjectId.zeroId());
-			}
+		result.add(update);
-			u.delete(walk);
+		batch.addCommand(update.asReceiveCommand());
-			switch (u.getResult()) {
-			case NEW:
-			case NO_CHANGE:
-			case FAST_FORWARD:
-			case FORCED:
-				break;
-			default:
-				throw new TransportException(transport.getURI(), MessageFormat.format(
-						JGitText.get().cannotDeleteStaleTrackingRef2, name, u.getResult().name()));
-			}
-		} catch (IOException e) {
-			throw new TransportException(transport.getURI(), MessageFormat.format(
-					JGitText.get().cannotDeleteStaleTrackingRef, name), e);
-		}
 	}
 
 	private static boolean isTag(final Ref r) {
@@ -483,4 +504,12 @@ class FetchProcess {
 	private static boolean isTag(final String name) {
 		return name.startsWith(Constants.R_TAGS);
 	}
+
+	private static String getFirstFailedRefName(BatchRefUpdate batch) {
+		for (ReceiveCommand cmd : batch.getCommands()) {
+			if (cmd.getResult() != ReceiveCommand.Result.OK)
+				return cmd.getRefName();
+		}
+		return "";
+	}
 }

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

@@ -49,6 +49,7 @@ import java.text.MessageFormat;
 import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.RefUpdate;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevWalk;
 
@@ -144,6 +145,8 @@ public class RemoteRefUpdate {
 
 	private final Repository localDb;
 
+	private RefUpdate localUpdate;
+
 	/**
 	 * Construct remote ref update request by providing an update specification.
 	 * Object is created with default {@link Status#NOT_ATTEMPTED} status and no
@@ -299,10 +302,20 @@ public class RemoteRefUpdate {
 
 		this.remoteName = remoteName;
 		this.forceUpdate = forceUpdate;
-		if (localName != null && localDb != null)
+		if (localName != null && localDb != null) {
-			trackingRefUpdate = new TrackingRefUpdate(localDb, localName,
+			localUpdate = localDb.updateRef(localName);
-					remoteName, true, newObjectId, "push");
+			localUpdate.setForceUpdate(true);
-		else
+			localUpdate.setRefLogMessage("push", true);
+			localUpdate.setNewObjectId(newObjectId);
+			trackingRefUpdate = new TrackingRefUpdate(
+					true,
+					remoteName,
+					localName,
+					localUpdate.getOldObjectId() != null
+						? localUpdate.getOldObjectId()
+						: ObjectId.zeroId(),
+					newObjectId);
+		} else
 			trackingRefUpdate = null;
 		this.localDb = localDb;
 		this.expectedOldObjectId = expectedOldObjectId;
@@ -449,9 +462,9 @@ public class RemoteRefUpdate {
 	 */
 	protected void updateTrackingRef(final RevWalk walk) throws IOException {
 		if (isDelete())
-			trackingRefUpdate.delete(walk);
+			trackingRefUpdate.setResult(localUpdate.delete(walk));
 		else
-			trackingRefUpdate.update(walk);
+			trackingRefUpdate.setResult(localUpdate.update(walk));
 	}
 
 	@Override

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

@@ -45,35 +45,31 @@
 
 package org.eclipse.jgit.transport;
 
-import java.io.IOException;
-
 import org.eclipse.jgit.lib.AnyObjectId;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.RefUpdate;
-import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.lib.RefUpdate.Result;
-import org.eclipse.jgit.revwalk.RevWalk;
 
 /** Update of a locally stored tracking branch. */
 public class TrackingRefUpdate {
 	private final String remoteName;
+	private final String localName;
+	private boolean forceUpdate;
+	private ObjectId oldObjectId;
+	private ObjectId newObjectId;
 
-	private final RefUpdate update;
+	private RefUpdate.Result result;
-
-	TrackingRefUpdate(final Repository db, final RefSpec spec,
-			final AnyObjectId nv, final String msg) throws IOException {
-		this(db, spec.getDestination(), spec.getSource(), spec.isForceUpdate(),
-				nv, msg);
-	}
 
-	TrackingRefUpdate(final Repository db, final String localName,
+	TrackingRefUpdate(
-			final String remoteName, final boolean forceUpdate,
+			boolean canForceUpdate,
-			final AnyObjectId nv, final String msg) throws IOException {
+			String remoteName,
+			String localName,
+			AnyObjectId oldValue,
+			AnyObjectId newValue) {
 		this.remoteName = remoteName;
-		update = db.updateRef(localName);
+		this.localName = localName;
-		update.setForceUpdate(forceUpdate);
+		this.forceUpdate = canForceUpdate;
-		update.setNewObjectId(nv);
+		this.oldObjectId = oldValue.copy();
-		update.setRefLogMessage(msg, true);
+		this.newObjectId = newValue.copy();
 	}
 
 	/**
@@ -95,7 +91,7 @@ public class TrackingRefUpdate {
 	 * @return the name used within this local repository.
 	 */
 	public String getLocalName() {
-		return update.getName();
+		return localName;
 	}
 
 	/**
@@ -104,7 +100,7 @@ public class TrackingRefUpdate {
 	 * @return new value. Null if the caller has not configured it.
 	 */
 	public ObjectId getNewObjectId() {
-		return update.getNewObjectId();
+		return newObjectId;
 	}
 
 	/**
@@ -115,11 +111,10 @@ public class TrackingRefUpdate {
 	 * value may change if someone else modified the ref between the time we
 	 * last read it and when the ref was locked for update.
 	 *
-	 * @return the value of the ref prior to the update being attempted; null if
+	 * @return the value of the ref prior to the update being attempted.
-	 *         the updated has not been attempted yet.
 	 */
 	public ObjectId getOldObjectId() {
-		return update.getOldObjectId();
+		return oldObjectId;
 	}
 
 	/**
@@ -127,15 +122,75 @@ public class TrackingRefUpdate {
 	 *
 	 * @return the status of the update.
 	 */
-	public Result getResult() {
+	public RefUpdate.Result getResult() {
-		return update.getResult();
+		return result;
 	}
 
-	void update(final RevWalk walk) throws IOException {
+	void setResult(RefUpdate.Result result) {
-		update.update(walk);
+		this.result = result;
 	}
 
-	void delete(final RevWalk walk) throws IOException {
+	ReceiveCommand asReceiveCommand() {
-		update.delete(walk);
+		return new Command();
+	}
+
+	final class Command extends ReceiveCommand {
+		private Command() {
+			super(oldObjectId, newObjectId, localName);
+		}
+
+		boolean canForceUpdate() {
+			return forceUpdate;
+		}
+
+		@Override
+		public void setResult(RefUpdate.Result status) {
+			result = status;
+			super.setResult(status);
+		}
+
+		@Override
+		public void setResult(ReceiveCommand.Result status) {
+			result = decode(status);
+			super.setResult(status);
+		}
+
+		@Override
+		public void setResult(ReceiveCommand.Result status, String msg) {
+			result = decode(status);
+			super.setResult(status, msg);
+		}
+
+		private RefUpdate.Result decode(ReceiveCommand.Result status) {
+			switch (status) {
+			case OK:
+				if (AnyObjectId.equals(oldObjectId, newObjectId))
+					return RefUpdate.Result.NO_CHANGE;
+				switch (getType()) {
+				case CREATE:
+					return RefUpdate.Result.NEW;
+				case UPDATE:
+					return RefUpdate.Result.FAST_FORWARD;
+				case DELETE:
+				case UPDATE_NONFASTFORWARD:
+				default:
+					return RefUpdate.Result.FORCED;
+				}
+
+			case REJECTED_NOCREATE:
+			case REJECTED_NODELETE:
+			case REJECTED_NONFASTFORWARD:
+				return RefUpdate.Result.REJECTED;
+			case REJECTED_CURRENT_BRANCH:
+				return RefUpdate.Result.REJECTED_CURRENT_BRANCH;
+			case REJECTED_MISSING_OBJECT:
+				return RefUpdate.Result.IO_FAILURE;
+			case LOCK_FAILURE:
+			case NOT_ATTEMPTED:
+			case REJECTED_OTHER_REASON:
+			default:
+				return RefUpdate.Result.LOCK_FAILURE;
+			}
+		}
 	}
 }