Browse Source

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
stable-3.0
Robin Rosenberg 12 years ago
parent
commit
5cf53fdacf
  1. 115
      org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefDirectoryTest.java
  2. 103
      org.eclipse.jgit/src/org/eclipse/jgit/lib/BatchRefUpdate.java
  3. 16
      org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java

115
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");
}

103
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));
continue;
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()));
}
}
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;
}
/**

16
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 {

Loading…
Cancel
Save