From 2b722815c9b6e3f8e50a081404265fb5b56b60e8 Mon Sep 17 00:00:00 2001 From: Roberto Tyley Date: Wed, 29 Jan 2014 00:32:55 +0000 Subject: [PATCH] Non-Fast-Forward Ref-Updates: Omit isMergedInto() calls When the caller specifies to JGit in advance that a ref-update is a non-fast-forward update, and that those are permitted, we should never need to call the potentially expensive isMergedInto() check. Re-checking that the older commit is /not/ reachable from the newer is superfluous. http://dev.eclipse.org/mhonarc/lists/jgit-dev/msg02258.html Change-Id: I4bbf593de4dcea6b6f082881c1a33cb3a6a7fb89 Signed-off-by: Roberto Tyley --- .../storage/file/RefDirectoryTest.java | 21 +++++++++++++++++++ .../internal/storage/file/RefUpdateTest.java | 1 - .../src/org/eclipse/jgit/lib/RefUpdate.java | 5 +++-- 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefDirectoryTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefDirectoryTest.java index 87b917636..8dbe64478 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefDirectoryTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefDirectoryTest.java @@ -1225,6 +1225,27 @@ public class RefDirectoryTest extends LocalDiskRepositoryTestCase { assertEquals(A.getId(), refs.get("refs/heads/masters").getObjectId()); } + @Test + public void testBatchRefUpdateNonFastForwardDoesNotDoExpensiveMergeCheck() + throws IOException { + writeLooseRef("refs/heads/master", B); + List commands = Arrays.asList( + newCommand(B, A, "refs/heads/master", + ReceiveCommand.Type.UPDATE_NONFASTFORWARD)); + BatchRefUpdate batchUpdate = refdir.newBatchUpdate(); + batchUpdate.setAllowNonFastForwards(true); + batchUpdate.addCommand(commands); + batchUpdate.execute(new RevWalk(diskRepo) { + @Override + public boolean isMergedInto(RevCommit base, RevCommit tip) { + throw new AssertionError("isMergedInto() should not be called"); + } + }, new StrictWorkMonitor()); + Map refs = refdir.getRefs(RefDatabase.ALL); + assertEquals(ReceiveCommand.Result.OK, commands.get(0).getResult()); + assertEquals(A.getId(), refs.get("refs/heads/master").getObjectId()); + } + @Test public void testBatchRefUpdateConflict() throws IOException { writeLooseRef("refs/heads/master", A); diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefUpdateTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefUpdateTest.java index 1731a3362..098b31fcc 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefUpdateTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefUpdateTest.java @@ -455,7 +455,6 @@ public class RefUpdateTest extends SampleDataRepositoryTestCase { // now update that ref updateRef = db.updateRef(Constants.HEAD); - updateRef.setForceUpdate(true); updateRef.setNewObjectId(oldValue); update = updateRef.update(); assertEquals(Result.FAST_FORWARD, update); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java index 8d59cb47b..f47dff75a 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java @@ -620,13 +620,14 @@ public abstract class RefUpdate { if (newObj == oldObj && !detachingSymbolicRef) return store.execute(Result.NO_CHANGE); + if (isForceUpdate()) + return store.execute(Result.FORCED); + if (newObj instanceof RevCommit && oldObj instanceof RevCommit) { if (walk.isMergedInto((RevCommit) oldObj, (RevCommit) newObj)) return store.execute(Result.FAST_FORWARD); } - if (isForceUpdate()) - return store.execute(Result.FORCED); return Result.REJECTED; } finally { unlock();