Fix RefUpdate performance for existing Refs

No longer invoke the expensive RefDatabase.isNameConflicting() check on
updating existing refs, reducing batch ref update time by ~97%.

The RefDirectory implementation of isNameConflicting() is quite
slow (it has to do an expensive loose-ref scan) but it's only necessary
to perform this check on ref update if the ref is being *created* - if
the ref already exists, we can already guarantee that it does not
conflict with any other refs.

C-Git seems to use a similar condition before making the
is_refname_available() check:

https://github.com/git/git/blob/v1.8.1.4/refs.c#L1660-L1670

As an example of the effects on performance, here's a simple timing
experiment using The BFG to remove one file from the JGit repo:

---
$ wget http://repo1.maven.org/maven2/com/madgag/bfg-repo-cleaner/1.0.1/bfg-1.0.1.jar
$ git clone --mirror https://git.eclipse.org/r/p/jgit/jgit.git
$ java -jar bfg-1.0.1.jar -D make_jgit.sh jgit.git
....
Updating references:    100% (5760/5760)
...Ref update completed in 148,949 ms.

BFG run is complete!
---

The execution time for the run is completely dominated by the batch ref
update at the end. Repeating the experiment with BFG v1.0.2 (using JGit
patched with this change), the refs update is dramatically reduced:

---
Updating references:    100% (5760/5760)
...Ref update completed in 4,327 ms.
---

Change-Id: I9057bc4ee22f9cc269b1cc00c493841c71527cd6

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 394af295340fdb2a432ab87f186849c68b53565c..fcf38e6950b48abfcb7c528eaa9fa70b784bd0dd 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java
@@ -598,7 +598,8 @@ public abstract class RefUpdate {
                RevObject newObj;
                RevObject oldObj;
 
-               if (getRefDatabase().isNameConflicting(getName()))
+               // don't make expensive conflict check if this is an existing Ref
+               if (oldValue == null && getRefDatabase().isNameConflicting(getName()))
                        return Result.LOCK_FAILURE;
                try {
                        if (!tryLock(true))