From 28adcce8622814e6b0ecb33168eae1846829390d Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Wed, 5 Jul 2017 09:57:01 -0400 Subject: [PATCH 1/3] BatchRefUpdate: Clarify some ref prefix calls Inline the old addRefToPrefixes, since it was just a glorified addAll. Split getPrefixes into a variant, addPrefixesTo, that doesn't allocate a small Collection on every invocation. Use this in the tight loop of getTakenPrefixes. Change-Id: I25cc7feef0c8e312820d85b7ed48559da49b83d2 --- .../org/eclipse/jgit/lib/BatchRefUpdate.java | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/BatchRefUpdate.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/BatchRefUpdate.java index 3f6995de8..3043d4fcd 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/BatchRefUpdate.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/BatchRefUpdate.java @@ -462,7 +462,7 @@ public void execute(RevWalk walk, ProgressMonitor monitor, break SWITCH; } ru.setCheckConflicting(false); - addRefToPrefixes(takenPrefixes, cmd.getRefName()); + takenPrefixes.addAll(getPrefixes(cmd.getRefName())); takenNames.add(cmd.getRefName()); cmd.setResult(ru.update(walk)); } @@ -523,29 +523,26 @@ public void execute(RevWalk walk, ProgressMonitor monitor) execute(walk, monitor, null); } - private static Collection getTakenPrefixes( - final Collection names) { + private static Collection getTakenPrefixes(Collection names) { Collection ref = new HashSet<>(); - for (String name : names) - ref.addAll(getPrefixes(name)); - return ref; - } - - private static void addRefToPrefixes(Collection prefixes, - String name) { - for (String prefix : getPrefixes(name)) { - prefixes.add(prefix); + for (String name : names) { + addPrefixesTo(name, ref); } + return ref; } static Collection getPrefixes(String s) { Collection ret = new HashSet<>(); + addPrefixesTo(s, ret); + return ret; + } + + static void addPrefixesTo(String s, Collection out) { int p1 = s.indexOf('/'); while (p1 > 0) { - ret.add(s.substring(0, p1)); + out.add(s.substring(0, p1)); p1 = s.indexOf('/', p1 + 1); } - return ret; } /** From e08fa5afcdd95a1bd0d837863e6c1df77aae0cd0 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Wed, 5 Jul 2017 12:16:56 -0400 Subject: [PATCH 2/3] Short-circuit writing packed-refs if no refs were packed Change-Id: Id691905599b242e48f590138a96e0c86132308fd --- .../internal/storage/file/RefDirectory.java | 28 +++++++++++++++---- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java index 24d51a5ea..5d66a4fbd 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java @@ -647,16 +647,32 @@ public void pack(List refs) throws IOException { RefList cur = readPackedRefs(); // Iterate over all refs to be packed + boolean dirty = false; for (String refName : refs) { - Ref ref = readRef(refName, cur); - if (ref.isSymbolic()) + Ref oldRef = readRef(refName, cur); + if (oldRef.isSymbolic()) { continue; // can't pack symbolic refs + } // Add/Update it to packed-refs + Ref newRef = peeledPackedRef(oldRef); + if (newRef == oldRef) { + // No-op; peeledPackedRef returns the input ref only if it's already + // packed, and readRef returns a packed ref only if there is no loose + // ref. + continue; + } + + dirty = true; int idx = cur.find(refName); - if (idx >= 0) - cur = cur.set(idx, peeledPackedRef(ref)); - else - cur = cur.add(idx, peeledPackedRef(ref)); + if (idx >= 0) { + cur = cur.set(idx, newRef); + } else { + cur = cur.add(idx, newRef); + } + } + if (!dirty) { + // All requested refs were already packed accurately + return; } // The new content for packed-refs is collected. Persist it. From 40748e83039f2a8ac77a17fd09ea109ae6382e51 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Wed, 5 Jul 2017 13:32:35 -0400 Subject: [PATCH 3/3] RefList: Support capacity <= 0 on new builders Callers may estimate the size, and their estimate may be zero. Silently allow this, rather than throwing IndexOutOfBoundsException later during add. Change-Id: Ife236f9f4ce469c57b18e76cf4fad6feb52cb2b0 --- org.eclipse.jgit/src/org/eclipse/jgit/util/RefList.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/RefList.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/RefList.java index 159781795..ce4b7c750 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/RefList.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/RefList.java @@ -338,10 +338,11 @@ public Builder() { * Create an empty list with at least the specified capacity. * * @param capacity - * the new capacity. + * the new capacity; if zero or negative, behavior is the same as + * {@link #Builder()}. */ public Builder(int capacity) { - list = new Ref[capacity]; + list = new Ref[Math.max(capacity, 16)]; } /** @return number of items in this builder's internal collection. */