]> source.dussan.org Git - jgit.git/commitdiff
Speed up clone/fetch with large number of refs 48/10448/4
authorRobin Rosenberg <robin.rosenberg@dewire.com>
Mon, 18 Feb 2013 19:25:00 +0000 (20:25 +0100)
committerRobin Rosenberg <robin.rosenberg@dewire.com>
Sat, 30 Mar 2013 12:36:44 +0000 (13:36 +0100)
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
org.eclipse.jgit/src/org/eclipse/jgit/lib/BatchRefUpdate.java
org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java

index 7d39d0900a772f5839f13e434566d222e6084f01..9bf3b94c431fa7e45e77e9bbcac5b1130e9af032 100644 (file)
@@ -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");
        }
index 98c264719038781c712536d931766f3cfd4991c1..b86d6fad8431dbfb0bdf2fbd0aae30b168974407 100644 (file)
@@ -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,41 +244,120 @@ 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 DELETE:
-                                               cmd.setResult(ru.delete(walk));
-                                               continue;
-
                                        case CREATE:
+                                               namesToCheck.add(cmd.getRefName());
+                                               commands2.add(cmd);
+                                               break;
                                        case UPDATE:
                                        case UPDATE_NONFASTFORWARD:
-                                               cmd.setResult(ru.update(walk));
-                                               continue;
+                                               commands2.add(cmd);
+                                               break;
+                                       case DELETE:
+                                               RefUpdate rud = newUpdate(cmd);
+                                               monitor.update(1);
+                                               cmd.setResult(rud.delete(walk));
                                        }
                                }
                        } catch (IOException err) {
-                               cmd.setResult(REJECTED_OTHER_REASON, MessageFormat.format(
-                                               JGitText.get().lockError, err.getMessage()));
+                               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);
+
+                       // 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));
+                                               }
+                                       }
+                               } catch (IOException err) {
+                                       cmd.setResult(REJECTED_OTHER_REASON, MessageFormat.format(
+                                                       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;
        }
 
        /**
index fcf38e6950b48abfcb7c528eaa9fa70b784bd0dd..56ec7af89bb4c80c370e579e713338956682dabd 100644 (file)
@@ -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 {