From 4db695c1c6c47b540a2971470768ca024c0dd54b Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Wed, 10 Apr 2013 21:19:46 -0700 Subject: [PATCH 1/9] Mark DeltaWindowEntry methods final This class and all of its methods are only package visible. Clarify the methods as final for the benefit of the JIT to inline trivial code. Change-Id: I078841f9900dbf299fbe6abf2599f0208ae96856 --- .../jgit/internal/storage/pack/DeltaWindowEntry.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindowEntry.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindowEntry.java index 7d29fd876..0b8c7ab81 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindowEntry.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindowEntry.java @@ -43,7 +43,7 @@ package org.eclipse.jgit.internal.storage.pack; -class DeltaWindowEntry { +final class DeltaWindowEntry { ObjectToPack object; /** Complete contents of this object. Lazily loaded. */ @@ -52,29 +52,29 @@ class DeltaWindowEntry { /** Index of this object's content, to encode other deltas. Lazily loaded. */ DeltaIndex index; - void set(ObjectToPack object) { + final void set(ObjectToPack object) { this.object = object; this.index = null; this.buffer = null; } /** @return current delta chain depth of this object. */ - int depth() { + final int depth() { return object.getDeltaDepth(); } /** @return type of the object in this window entry. */ - int type() { + final int type() { return object.getType(); } /** @return estimated unpacked size of the object, in bytes . */ - int size() { + final int size() { return object.getWeight(); } /** @return true if there is no object stored in this entry. */ - boolean empty() { + final boolean empty() { return object == null; } } From 6903fa4a3447d74cf76f5bc661da0af46d5691ff Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Wed, 10 Apr 2013 22:21:18 -0700 Subject: [PATCH 2/9] Micro-optimize DeltaWindow maxMemory test to be != 0 Instead of using a compare-with-0 use a does not equal 0. javac bytecode has a special instruction for this, as it is very common in software. We can assume the JIT knows how to efficiently translate the opcode to machine code, and processors can do != 0 very quickly. Change-Id: Idb84c1d744d2874517fd4bfa1db390e2dbf64eac --- .../jgit/internal/storage/pack/DeltaWindow.java | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindow.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindow.java index a4ff9da9f..298be02cc 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindow.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindow.java @@ -62,11 +62,8 @@ final class DeltaWindow { private static final int NEXT_SRC = 1; private final PackConfig config; - private final DeltaCache deltaCache; - private final ObjectReader reader; - private final ProgressMonitor monitor; private final DeltaWindowEntry[] window; @@ -136,7 +133,7 @@ final class DeltaWindow { for (int i = 0; i < window.length; i++) window[i] = new DeltaWindowEntry(); - maxMemory = config.getDeltaSearchMemoryLimit(); + maxMemory = Math.max(0, config.getDeltaSearchMemoryLimit()); maxDepth = config.getMaxDeltaDepth(); } @@ -172,7 +169,7 @@ final class DeltaWindow { next = toSearch[cur++]; } res = window[resSlot]; - if (0 < maxMemory) { + if (maxMemory != 0) { clear(res); int tail = next(resSlot); final long need = estimateSize(next); @@ -480,7 +477,7 @@ final class DeltaWindow { e.setObjectId(ent.object); throw e; } - if (0 < maxMemory) + if (maxMemory != 0) loaded += idx.getIndexSize() - idx.getSourceSize(); ent.index = idx; } @@ -494,7 +491,7 @@ final class DeltaWindow { checkLoadable(ent, ent.size()); buf = PackWriter.buffer(config, reader, ent.object); - if (0 < maxMemory) + if (maxMemory != 0) loaded += buf.length; ent.buffer = buf; } @@ -502,7 +499,7 @@ final class DeltaWindow { } private void checkLoadable(DeltaWindowEntry ent, long need) { - if (maxMemory <= 0) + if (maxMemory == 0) return; int tail = next(resSlot); From 1db50c9d91adaadc019938eaaa7e91e3963fb681 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Wed, 10 Apr 2013 21:27:54 -0700 Subject: [PATCH 3/9] Micro-optimize DeltaWindow primary loop javac and the JIT are more likely to understand a boolean being used as a branch conditional than comparing int against 0 and 1. Rewrite NEXT_RES and NEXT_SRC constants to be booleans so the code is clarified for the JIT. Change-Id: I1bdd8b587a69572975a84609c779b9ebf877b85d --- .../jgit/internal/storage/pack/DeltaWindow.java | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindow.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindow.java index 298be02cc..3ff0a6c1c 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindow.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindow.java @@ -57,9 +57,8 @@ import org.eclipse.jgit.storage.pack.PackConfig; import org.eclipse.jgit.util.TemporaryBuffer; final class DeltaWindow { - private static final int NEXT_RES = 0; - - private static final int NEXT_SRC = 1; + private static final boolean NEXT_RES = false; + private static final boolean NEXT_SRC = true; private final PackConfig config; private final DeltaCache deltaCache; @@ -236,10 +235,10 @@ final class DeltaWindow { DeltaWindowEntry src = window[srcSlot]; if (src.empty()) break; - if (delta(src, srcSlot) == NEXT_RES) { - bestDelta = null; - return; - } + if (delta(src, srcSlot) /* == NEXT_SRC */) + continue; + bestDelta = null; + return; } // We couldn't find a suitable delta for this object, but it may @@ -286,7 +285,7 @@ final class DeltaWindow { keepInWindow(); } - private int delta(final DeltaWindowEntry src, final int srcSlot) + private boolean delta(final DeltaWindowEntry src, final int srcSlot) throws IOException { // Objects must use only the same type as their delta base. // If we are looking at something where that isn't true we From 0f32901ab7da9f0eaf1c716d66ce8d58bc70f101 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Thu, 11 Apr 2013 01:11:11 -0700 Subject: [PATCH 4/9] Micro-optimize copy instructions in DeltaEncoder The copy instruction formatter should not to compute the shifts and masks twice. Instead compute them once and assume there is a register available to store the temporary "b" for compare with 0. Change-Id: Ic7826f29dca67b16903d8f790bdf785eb478c10d --- .../internal/storage/pack/DeltaEncoder.java | 37 ++++++++++--------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaEncoder.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaEncoder.java index 8dfa62f0d..cccbc5903 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaEncoder.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaEncoder.java @@ -134,7 +134,7 @@ public class DeltaEncoder { } buf[p++] = (byte) (((int) sz) & 0x7f); size += p; - if (limit <= 0 || size < limit) + if (limit == 0 || size < limit) out.write(buf, 0, p); } @@ -189,7 +189,7 @@ public class DeltaEncoder { throws IOException { if (cnt <= 0) return true; - if (0 < limit) { + if (limit != 0) { int hdrs = cnt / MAX_INSERT_DATA_SIZE; if (cnt % MAX_INSERT_DATA_SIZE != 0) hdrs++; @@ -236,7 +236,7 @@ public class DeltaEncoder { cnt -= MAX_V2_COPY; if (buf.length < p + MAX_COPY_CMD_SIZE) { - if (0 < limit && limit < size + p) + if (limit != 0 && limit < size + p) return false; out.write(buf, 0, p); size += p; @@ -245,7 +245,7 @@ public class DeltaEncoder { } p = encodeCopy(p, offset, cnt); - if (0 < limit && limit < size + p) + if (limit != 0 && limit < size + p) return false; out.write(buf, 0, p); size += p; @@ -255,36 +255,37 @@ public class DeltaEncoder { private int encodeCopy(int p, long offset, int cnt) { int cmd = 0x80; final int cmdPtr = p++; // save room for the command + byte b; - if ((offset & 0xff) != 0) { + if ((b = (byte) (offset & 0xff)) != 0) { cmd |= 0x01; - buf[p++] = (byte) (offset & 0xff); + buf[p++] = b; } - if ((offset & (0xff << 8)) != 0) { + if ((b = (byte) ((offset >>> 8) & 0xff)) != 0) { cmd |= 0x02; - buf[p++] = (byte) ((offset >>> 8) & 0xff); + buf[p++] = b; } - if ((offset & (0xff << 16)) != 0) { + if ((b = (byte) ((offset >>> 16) & 0xff)) != 0) { cmd |= 0x04; - buf[p++] = (byte) ((offset >>> 16) & 0xff); + buf[p++] = b; } - if ((offset & (0xff << 24)) != 0) { + if ((b = (byte) ((offset >>> 24) & 0xff)) != 0) { cmd |= 0x08; - buf[p++] = (byte) ((offset >>> 24) & 0xff); + buf[p++] = b; } if (cnt != MAX_V2_COPY) { - if ((cnt & 0xff) != 0) { + if ((b = (byte) (cnt & 0xff)) != 0) { cmd |= 0x10; - buf[p++] = (byte) (cnt & 0xff); + buf[p++] = b; } - if ((cnt & (0xff << 8)) != 0) { + if ((b = (byte) ((cnt >>> 8) & 0xff)) != 0) { cmd |= 0x20; - buf[p++] = (byte) ((cnt >>> 8) & 0xff); + buf[p++] = b; } - if ((cnt & (0xff << 16)) != 0) { + if ((b = (byte) ((cnt >>> 16) & 0xff)) != 0) { cmd |= 0x40; - buf[p++] = (byte) ((cnt >>> 16) & 0xff); + buf[p++] = b; } } From af33a911d0e5a4ee3a5e520b36e1b565c84df6e5 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Wed, 10 Apr 2013 22:29:32 -0700 Subject: [PATCH 5/9] Replace DeltaWindow array with circularly linked list Typical window sizes are 10 and 250 (although others are accepted). In either case the pointer overhead of 1 pointer in an array or 2 pointers for a double linked list is trivial. A doubly linked list as used here for window=250 is only another 1024 bytes on a 32 bit machine, or 2048 bytes on a 64 bit machine. The critical search loops scan through the array in either the previous direction or the next direction until the cycle is finished, or some other scan abort condition is reached. Loading the next object's pointer from a field in the current object avoids the branch required to test for wrapping around the edge of the array. It also saves the array bounds check on each access. When a delta is chosen the window is shuffled to hoist the currently selected base as an earlier candidate for the next object. Moving the window entry is easier in a double-linked list than sliding a group of array entries. Change-Id: I9ccf20c3362a78678aede0f0f2cda165e509adff --- .../internal/storage/pack/DeltaWindow.java | 97 +++++-------------- .../storage/pack/DeltaWindowEntry.java | 38 ++++++++ 2 files changed, 62 insertions(+), 73 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindow.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindow.java index 3ff0a6c1c..2eb1daf74 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindow.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindow.java @@ -65,8 +65,6 @@ final class DeltaWindow { private final ObjectReader reader; private final ProgressMonitor monitor; - private final DeltaWindowEntry[] window; - /** Maximum number of bytes to admit to the window at once. */ private final long maxMemory; @@ -74,9 +72,7 @@ final class DeltaWindow { private final int maxDepth; private final ObjectToPack[] toSearch; - private int cur; - private int end; /** Amount of memory we have loaded right now. */ @@ -84,9 +80,6 @@ final class DeltaWindow { // The object we are currently considering needs a lot of state: - /** Position of {@link #res} within {@link #window} array. */ - private int resSlot; - /** * Maximum delta chain depth the current object can have. *

@@ -100,8 +93,8 @@ final class DeltaWindow { /** If we have a delta for {@link #res}, this is the shortest found yet. */ private TemporaryBuffer.Heap bestDelta; - /** If we have {@link #bestDelta}, the window position it was created by. */ - private int bestSlot; + /** If we have {@link #bestDelta}, the window entry it was created from. */ + private DeltaWindowEntry bestBase; /** Used to compress cached deltas. */ private Deflater deflater; @@ -117,23 +110,9 @@ final class DeltaWindow { cur = beginIndex; end = endIndex; - // C Git increases the window size supplied by the user by 1. - // We don't know why it does this, but if the user asks for - // window=10, it actually processes with window=11. Because - // the window size has the largest direct impact on the final - // pack file size, we match this odd behavior here to give us - // a better chance of producing a similar sized pack as C Git. - // - // We would prefer to directly honor the user's request since - // PackWriter has a minimum of 2 for the window size, but then - // users might complain that JGit is creating a bigger pack file. - // - window = new DeltaWindowEntry[config.getDeltaSearchWindowSize() + 1]; - for (int i = 0; i < window.length; i++) - window[i] = new DeltaWindowEntry(); - maxMemory = Math.max(0, config.getDeltaSearchMemoryLimit()); maxDepth = config.getMaxDeltaDepth(); + res = DeltaWindowEntry.createWindow(config.getDeltaSearchWindowSize()); } synchronized int remaining() { @@ -167,15 +146,12 @@ final class DeltaWindow { break; next = toSearch[cur++]; } - res = window[resSlot]; if (maxMemory != 0) { clear(res); - int tail = next(resSlot); final long need = estimateSize(next); - while (maxMemory < loaded + need && tail != resSlot) { - clear(window[tail]); - tail = next(tail); - } + DeltaWindowEntry n = res.next; + for (; maxMemory < loaded + need && n != res; n = n.next) + clear(n); } res.set(next); @@ -231,11 +207,10 @@ final class DeltaWindow { // Loop through the window backwards, considering every entry. // This lets us look at the bigger objects that came before. // - for (int srcSlot = prior(resSlot); srcSlot != resSlot; srcSlot = prior(srcSlot)) { - DeltaWindowEntry src = window[srcSlot]; + for (DeltaWindowEntry src = res.prev; src != res; src = src.prev) { if (src.empty()) break; - if (delta(src, srcSlot) /* == NEXT_SRC */) + if (delta(src) /* == NEXT_SRC */) continue; bestDelta = null; return; @@ -251,7 +226,7 @@ final class DeltaWindow { // Select this best matching delta as the base for the object. // - ObjectToPack srcObj = window[bestSlot].object; + ObjectToPack srcObj = bestBase.object; ObjectToPack resObj = res.object; if (srcObj.isEdge()) { // The source (the delta base) is an edge object outside of the @@ -285,7 +260,7 @@ final class DeltaWindow { keepInWindow(); } - private boolean delta(final DeltaWindowEntry src, final int srcSlot) + private boolean delta(final DeltaWindowEntry src) throws IOException { // Objects must use only the same type as their delta base. // If we are looking at something where that isn't true we @@ -319,12 +294,12 @@ final class DeltaWindow { srcIndex = index(src); } catch (LargeObjectException tooBig) { // If the source is too big to work on, skip it. - dropFromWindow(srcSlot); + dropFromWindow(src); return NEXT_SRC; } catch (IOException notAvailable) { if (src.object.isEdge()) { // This is an edge that is suddenly not available. - dropFromWindow(srcSlot); + dropFromWindow(src); return NEXT_SRC; } else { throw notAvailable; @@ -355,7 +330,7 @@ final class DeltaWindow { if (isBetterDelta(src, delta)) { bestDelta = delta; - bestSlot = srcSlot; + bestBase = src; } return NEXT_SRC; @@ -390,39 +365,17 @@ final class DeltaWindow { } private void shuffleBaseUpInPriority() { - // Shuffle the entire window so that the best match we just used - // is at our current index, and our current object is at the index - // before it. Slide any entries in between to make space. - // - window[resSlot] = window[bestSlot]; - - DeltaWindowEntry next = res; - int slot = prior(resSlot); - for (; slot != bestSlot; slot = prior(slot)) { - DeltaWindowEntry e = window[slot]; - window[slot] = next; - next = e; - } - window[slot] = next; + // Reorder the window so that the best match we just used + // is the current one, and the now current object is before. + res.makeNext(bestBase); + res = bestBase; } private void keepInWindow() { - resSlot = next(resSlot); - } - - private int next(int slot) { - if (++slot == window.length) - return 0; - return slot; - } - - private int prior(int slot) { - if (slot == 0) - return window.length - 1; - return slot - 1; + res = res.next; } - private void dropFromWindow(@SuppressWarnings("unused") int srcSlot) { + private void dropFromWindow(@SuppressWarnings("unused") DeltaWindowEntry src) { // We should drop the current source entry from the window, // it is somehow invalid for us to work with. } @@ -437,7 +390,7 @@ final class DeltaWindow { // to access during reads. // if (resDelta.length() == bestDelta.length()) - return src.depth() < window[bestSlot].depth(); + return src.depth() < bestBase.depth(); return resDelta.length() < bestDelta.length(); } @@ -501,14 +454,12 @@ final class DeltaWindow { if (maxMemory == 0) return; - int tail = next(resSlot); - while (maxMemory < loaded + need) { - DeltaWindowEntry cur = window[tail]; - clear(cur); - if (cur == ent) + DeltaWindowEntry n = res.next; + for (; maxMemory < loaded + need; n = n.next) { + clear(n); + if (n == ent) throw new LargeObjectException.ExceedsLimit( maxMemory, loaded + need); - tail = next(tail); } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindowEntry.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindowEntry.java index 0b8c7ab81..958bae187 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindowEntry.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindowEntry.java @@ -44,6 +44,8 @@ package org.eclipse.jgit.internal.storage.pack; final class DeltaWindowEntry { + DeltaWindowEntry prev; + DeltaWindowEntry next; ObjectToPack object; /** Complete contents of this object. Lazily loaded. */ @@ -77,4 +79,40 @@ final class DeltaWindowEntry { final boolean empty() { return object == null; } + + final void makeNext(DeltaWindowEntry e) { + // Disconnect e from the chain. + e.prev.next = e.next; + e.next.prev = e.prev; + + // Insert e after this. + e.next = next; + e.prev = this; + next.prev = e; + next = e; + } + + static DeltaWindowEntry createWindow(int cnt) { + // C Git increases the window size supplied by the user by 1. + // We don't know why it does this, but if the user asks for + // window=10, it actually processes with window=11. Because + // the window size has the largest direct impact on the final + // pack file size, we match this odd behavior here to give us + // a better chance of producing a similar sized pack as C Git. + // + // We would prefer to directly honor the user's request since + // PackWriter has a minimum of 2 for the window size, but then + // users might complain that JGit is creating a bigger pack file. + DeltaWindowEntry res = new DeltaWindowEntry(); + DeltaWindowEntry p = res; + for (int i = 0; i < cnt; i++) { + DeltaWindowEntry e = new DeltaWindowEntry(); + e.prev = p; + p.next = e; + p = e; + } + p.next = res; + res.prev = p; + return res; + } } From 3b7924f403afea45b2c7464d88b0b10f8f9c656a Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Thu, 11 Apr 2013 15:32:07 -0700 Subject: [PATCH 6/9] Split remaining delta work on path boundaries When an idle thread tries to steal work from a sibling's remaining toSearch queue, always try to split along a path boundary. This avoids missing delta opportunities in the current window of the thread whose work is being taken. The search order is reversed to walk further down the chain from current position, avoiding the risk of splitting the list within the path the thread is currently processing. When selecting which thread to split from use an accurate estimate of the size to be taken. This avoids selecting a thread that has only one path remaining but may contain more pending entries than another thread with several paths remaining. As there is now a race condition where the straggling thread can start the next path before the split can finish, the stealWork() loop spins until it is able to acquire a split or there is only one path remaining in the siblings. Change-Id: Ib11ff99f90a4d9efab24bf4a85342cc63203dba5 --- .../jgit/internal/storage/pack/DeltaTask.java | 29 +++++++------ .../internal/storage/pack/DeltaWindow.java | 43 ++++++++++++------- 2 files changed, 44 insertions(+), 28 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaTask.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaTask.java index 218696eef..cc212fb81 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaTask.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaTask.java @@ -76,23 +76,24 @@ final class DeltaTask implements Callable { } synchronized Slice stealWork() { - for (int attempts = 0; attempts < 2; attempts++) { + for (;;) { DeltaTask maxTask = null; + Slice maxSlice = null; int maxWork = 0; + for (DeltaTask task : tasks) { - int r = task.remaining(); - if (maxWork < r) { + Slice s = task.remaining(); + if (s != null && maxWork < s.size()) { maxTask = task; - maxWork = r; + maxSlice = s; + maxWork = s.size(); } } if (maxTask == null) return null; - Slice s = maxTask.stealWork(); - if (s != null) - return s; + if (maxTask.tryStealWork(maxSlice)) + return maxSlice; } - return null; } } @@ -104,6 +105,10 @@ final class DeltaTask implements Callable { beginIndex = b; endIndex = e; } + + final int size() { + return endIndex - beginIndex; + } } private final Block block; @@ -131,13 +136,13 @@ final class DeltaTask implements Callable { return null; } - int remaining() { + Slice remaining() { DeltaWindow d = dw; - return d != null ? d.remaining() : 0; + return d != null ? d.remaining() : null; } - Slice stealWork() { + boolean tryStealWork(Slice s) { DeltaWindow d = dw; - return d != null ? d.stealWork() : null; + return d != null ? d.tryStealWork(s) : false; } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindow.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindow.java index 2eb1daf74..7688da395 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindow.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindow.java @@ -115,26 +115,37 @@ final class DeltaWindow { res = DeltaWindowEntry.createWindow(config.getDeltaSearchWindowSize()); } - synchronized int remaining() { - return end - cur; - } - - synchronized DeltaTask.Slice stealWork() { + synchronized DeltaTask.Slice remaining() { int e = end; - int n = (e - cur) >>> 1; - if (0 == n) + int halfRemaining = (e - cur) >>> 1; + if (0 == halfRemaining) return null; - int t = e - n; - int h = toSearch[t].getPathHash(); - while (cur < t) { - if (h == toSearch[t - 1].getPathHash()) - t--; - else - break; + int split = e - halfRemaining; + int h = toSearch[split].getPathHash(); + + // Attempt to split on the next path after the 50% split point. + for (int n = split + 1; n < e; n++) { + if (h != toSearch[n].getPathHash()) + return new DeltaTask.Slice(n, e); } - end = t; - return new DeltaTask.Slice(t, e); + + if (h != toSearch[cur].getPathHash()) { + // Try to split on the path before the 50% split point. + // Do not split the path currently being processed. + for (int p = split - 1; cur < p; p--) { + if (h != toSearch[p].getPathHash()) + return new DeltaTask.Slice(p + 1, e); + } + } + return null; + } + + synchronized boolean tryStealWork(DeltaTask.Slice s) { + if (s.beginIndex <= cur) + return false; + end = s.beginIndex; + return true; } void search() throws IOException { From 8a7c2f97d06df698bd8086ece4a9ade83273dd20 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Thu, 11 Apr 2013 00:30:15 -0700 Subject: [PATCH 7/9] Correct distribution of allowed delta size along chain length Nicolas Pitre discovered a very simple rule for selecting between two different delta base candidates: - if based whole object, must be <= 50% of target - if at end of a chain, must be <= 1/depth * 50% of target The rule penalizes deltas near the end of the chain, requiring them to be very small in order to be kept by the packer. This favors deltas that are based on a shorter chain, where the read-time unpack cost is much lower. Fewer bytes need to be consulted from the source pack file, and less copying is required in memory to rebuild the object. Junio Hamano explained Nico's rule to me today, and this commit fixes DeltaWindow to implement it as described. When no base has been chosen the computation is simply the statements denoted above. However once a base with depth of 9 has been chosen (e.g. when pack.depth is limited to 10), a non-delta source may create a new delta that is up to 10x larger than the already selected base. This reflects the intent of Nico's size distribution rule no matter what order objects are visited in the DeltaWindow. With this patch and my other patches applied, repacking JGit with: [pack] reuseObjects = false reuseDeltas = false depth = 50 window = 250 threads = 4 compression = 9 CGit (all) 5,711,735 bytes; real 0m13.942s user 0m47.722s [1] JGit heads 5,718,295 bytes; real 0m11.880s user 0m38.177s [2] rest 9,809 bytes The improved JGit result for the head pack is only 6.4 KiB larger than CGit's resulting pack. This patch allowed JGit to find an additional 39.7 KiB worth of space savings. JGit now also often runs 2s faster than CGit, despite also creating bitmaps and pruning objects after the head pack creation. [1] time git repack -a -d -F --window=250 --depth=50 [2] time java -Xmx128m -jar jgit debug-gc Change-Id: I5caec31359bf7248cabdd2a3254c84d4ee3cd96b --- .../internal/storage/pack/DeltaWindow.java | 162 ++++++------------ 1 file changed, 49 insertions(+), 113 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindow.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindow.java index 7688da395..144b9bdb4 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindow.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindow.java @@ -80,13 +80,6 @@ final class DeltaWindow { // The object we are currently considering needs a lot of state: - /** - * Maximum delta chain depth the current object can have. - *

- * This can be smaller than {@link #maxDepth}. - */ - private int resMaxDepth; - /** Window entry of the object we are currently considering. */ private DeltaWindowEntry res; @@ -206,31 +199,21 @@ final class DeltaWindow { } private void searchInWindow() throws IOException { - // TODO(spearce) If the object is used as a base for other - // objects in this pack we should limit the depth we create - // for ourselves to be the remainder of our longest dependent - // chain and the configured maximum depth. This can happen - // when the dependents are being reused out a pack, but we - // cannot be because we are near the edge of a thin pack. - // - resMaxDepth = maxDepth; - // Loop through the window backwards, considering every entry. // This lets us look at the bigger objects that came before. - // for (DeltaWindowEntry src = res.prev; src != res; src = src.prev) { if (src.empty()) break; if (delta(src) /* == NEXT_SRC */) continue; + bestBase = null; bestDelta = null; return; } // We couldn't find a suitable delta for this object, but it may // still be able to act as a base for another one. - // - if (bestDelta == null) { + if (bestBase == null) { keepInWindow(); return; } @@ -245,76 +228,60 @@ final class DeltaWindow { // has on hand, so we don't want to send it. We have to store // an ObjectId and *NOT* an ObjectToPack for the base to ensure // the base isn't included in the outgoing pack file. - // resObj.setDeltaBase(srcObj.copy()); } else { // The base is part of the pack we are sending, so it should be // a direct pointer to the base. - // resObj.setDeltaBase(srcObj); } - resObj.setDeltaDepth(srcObj.getDeltaDepth() + 1); + + int depth = srcObj.getDeltaDepth() + 1; + resObj.setDeltaDepth(depth); resObj.clearReuseAsIs(); cacheDelta(srcObj, resObj); - // Discard the cached best result, otherwise it leaks. - // - bestDelta = null; - - // If this should be the end of a chain, don't keep - // it in the window. Just move on to the next object. - // - if (resObj.getDeltaDepth() == maxDepth) - return; + if (depth < maxDepth) { + // Reorder the window so that the best base will be tested + // first for the next object, and the current object will + // be the second candidate to consider before any others. + res.makeNext(bestBase); + res = bestBase.next; + } - shuffleBaseUpInPriority(); - keepInWindow(); + bestBase = null; + bestDelta = null; } private boolean delta(final DeltaWindowEntry src) throws IOException { // Objects must use only the same type as their delta base. - // If we are looking at something where that isn't true we - // have exhausted everything of the correct type and should - // move on to the next thing to examine. - // if (src.type() != res.type()) { keepInWindow(); return NEXT_RES; } - // Only consider a source with a short enough delta chain. - if (src.depth() > resMaxDepth) + // If the sizes are radically different, this is a bad pairing. + if (res.size() < src.size() >>> 4) return NEXT_SRC; - // Estimate a reasonable upper limit on delta size. - int msz = deltaSizeLimit(res, resMaxDepth, src); - if (msz <= 8) + int msz = deltaSizeLimit(src); + if (msz <= 8) // Nearly impossible to fit useful delta. return NEXT_SRC; // If we have to insert a lot to make this work, find another. if (res.size() - src.size() > msz) return NEXT_SRC; - // If the sizes are radically different, this is a bad pairing. - if (res.size() < src.size() / 16) - return NEXT_SRC; - DeltaIndex srcIndex; try { srcIndex = index(src); } catch (LargeObjectException tooBig) { // If the source is too big to work on, skip it. - dropFromWindow(src); return NEXT_SRC; } catch (IOException notAvailable) { - if (src.object.isEdge()) { - // This is an edge that is suddenly not available. - dropFromWindow(src); + if (src.object.isEdge()) // Missing edges are OK. return NEXT_SRC; - } else { - throw notAvailable; - } + throw notAvailable; } byte[] resBuf; @@ -325,26 +292,41 @@ final class DeltaWindow { return NEXT_RES; } - // If we already have a delta for the current object, abort - // encoding early if this new pairing produces a larger delta. - if (bestDelta != null && bestDelta.length() < msz) - msz = (int) bestDelta.length(); - - TemporaryBuffer.Heap delta = new TemporaryBuffer.Heap(msz); try { - if (!srcIndex.encode(delta, resBuf, msz)) - return NEXT_SRC; + TemporaryBuffer.Heap delta = new TemporaryBuffer.Heap(msz); + if (srcIndex.encode(delta, resBuf, msz)) { + bestBase = src; + bestDelta = delta; + } } catch (IOException deltaTooBig) { - // This only happens when the heap overflows our limit. - return NEXT_SRC; + // Unlikely, encoder should see limit and return false. } + return NEXT_SRC; + } - if (isBetterDelta(src, delta)) { - bestDelta = delta; - bestBase = src; + private int deltaSizeLimit(DeltaWindowEntry src) { + if (bestBase == null) { + // Any delta should be no more than 50% of the original size + // (for text files deflate of whole form should shrink 50%). + int n = res.size() >>> 1; + + // Evenly distribute delta size limits over allowed depth. + // If src is non-delta (depth = 0), delta <= 50% of original. + // If src is almost at limit (9/10), delta <= 10% of original. + return n * (maxDepth - src.depth()) / maxDepth; } - return NEXT_SRC; + // With a delta base chosen any new delta must be "better". + // Retain the distribution described above. + int d = bestBase.depth(); + int n = (int) bestDelta.length(); + + // If src is whole (depth=0) and base is near limit (depth=9/10) + // any delta using src can be 10x larger and still be better. + // + // If src is near limit (depth=9/10) and base is whole (depth=0) + // a new delta dependent on src must be 1/10th the size. + return n * (maxDepth - src.depth()) / (maxDepth - d); } private void cacheDelta(ObjectToPack srcObj, ObjectToPack resObj) { @@ -375,56 +357,10 @@ final class DeltaWindow { return insz + ((insz + 7) >> 3) + ((insz + 63) >> 6) + 11; } - private void shuffleBaseUpInPriority() { - // Reorder the window so that the best match we just used - // is the current one, and the now current object is before. - res.makeNext(bestBase); - res = bestBase; - } - private void keepInWindow() { res = res.next; } - private void dropFromWindow(@SuppressWarnings("unused") DeltaWindowEntry src) { - // We should drop the current source entry from the window, - // it is somehow invalid for us to work with. - } - - private boolean isBetterDelta(DeltaWindowEntry src, - TemporaryBuffer.Heap resDelta) { - if (bestDelta == null) - return true; - - // If both delta sequences are the same length, use the one - // that has a shorter delta chain since it would be faster - // to access during reads. - // - if (resDelta.length() == bestDelta.length()) - return src.depth() < bestBase.depth(); - - return resDelta.length() < bestDelta.length(); - } - - private static int deltaSizeLimit(DeltaWindowEntry res, int maxDepth, - DeltaWindowEntry src) { - // Ideally the delta is at least 50% of the original size, - // but we also want to account for delta header overhead in - // the pack file (to point to the delta base) so subtract off - // some of those header bytes from the limit. - // - final int limit = res.size() / 2 - 20; - - // Distribute the delta limit over the entire chain length. - // This is weighted such that deeper items in the chain must - // be even smaller than if they were earlier in the chain, as - // they cost significantly more to unpack due to the increased - // number of recursive unpack calls. - // - final int remainingDepth = maxDepth - src.depth(); - return (limit * remainingDepth) / maxDepth; - } - private DeltaIndex index(DeltaWindowEntry ent) throws MissingObjectException, IncorrectObjectTypeException, IOException, LargeObjectException { From a5c6aac76c6f5fa19e4582bef60417647faeccf8 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Thu, 11 Apr 2013 00:49:42 -0700 Subject: [PATCH 8/9] Avoid TemporaryBuffer.Heap on very small deltas TemporaryBuffer is great when the output size is not known, but must be bound by a relatively large upper limit that fits in memory, e.g. 64 KiB or 20 MiB. The buffer gracefully supports growing storage by allocating 8 KiB blocks and storing them in an ArrayList. In a Git repository many deltas are less than 8 KiB. Typical tree objects are well below this threshold, and their deltas must be encoded even smaller. For these much smaller cases avoid the 8 KiB minimum allocation used by TemporaryBuffer. Instead allocate a very small OutputStream writing to an array that is sized at the limit. Change-Id: Ie25c6d3a8cf4604e0f8cd9a3b5b701a592d6ffca --- .../internal/storage/pack/DeltaWindow.java | 85 +++++++++++++------ 1 file changed, 60 insertions(+), 25 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindow.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindow.java index 144b9bdb4..66871bb14 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindow.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/DeltaWindow.java @@ -83,11 +83,10 @@ final class DeltaWindow { /** Window entry of the object we are currently considering. */ private DeltaWindowEntry res; - /** If we have a delta for {@link #res}, this is the shortest found yet. */ - private TemporaryBuffer.Heap bestDelta; - - /** If we have {@link #bestDelta}, the window entry it was created from. */ + /** If we have chosen a base, the window entry it was created from. */ private DeltaWindowEntry bestBase; + private int deltaLen; + private Object deltaBuf; /** Used to compress cached deltas. */ private Deflater deflater; @@ -207,7 +206,7 @@ final class DeltaWindow { if (delta(src) /* == NEXT_SRC */) continue; bestBase = null; - bestDelta = null; + deltaBuf = null; return; } @@ -249,7 +248,7 @@ final class DeltaWindow { } bestBase = null; - bestDelta = null; + deltaBuf = null; } private boolean delta(final DeltaWindowEntry src) @@ -293,17 +292,31 @@ final class DeltaWindow { } try { - TemporaryBuffer.Heap delta = new TemporaryBuffer.Heap(msz); - if (srcIndex.encode(delta, resBuf, msz)) { - bestBase = src; - bestDelta = delta; - } + OutputStream delta = msz <= (8 << 10) + ? new ArrayStream(msz) + : new TemporaryBuffer.Heap(msz); + if (srcIndex.encode(delta, resBuf, msz)) + selectDeltaBase(src, delta); } catch (IOException deltaTooBig) { // Unlikely, encoder should see limit and return false. } return NEXT_SRC; } + private void selectDeltaBase(DeltaWindowEntry src, OutputStream delta) { + bestBase = src; + + if (delta instanceof ArrayStream) { + ArrayStream a = (ArrayStream) delta; + deltaBuf = a.buf; + deltaLen = a.cnt; + } else { + TemporaryBuffer.Heap b = (TemporaryBuffer.Heap) delta; + deltaBuf = b; + deltaLen = (int) b.length(); + } + } + private int deltaSizeLimit(DeltaWindowEntry src) { if (bestBase == null) { // Any delta should be no more than 50% of the original size @@ -319,7 +332,7 @@ final class DeltaWindow { // With a delta base chosen any new delta must be "better". // Retain the distribution described above. int d = bestBase.depth(); - int n = (int) bestDelta.length(); + int n = deltaLen; // If src is whole (depth=0) and base is near limit (depth=9/10) // any delta using src can be 10x larger and still be better. @@ -330,25 +343,23 @@ final class DeltaWindow { } private void cacheDelta(ObjectToPack srcObj, ObjectToPack resObj) { - if (Integer.MAX_VALUE < bestDelta.length()) - return; - - int rawsz = (int) bestDelta.length(); - if (deltaCache.canCache(rawsz, srcObj, resObj)) { + if (deltaCache.canCache(deltaLen, srcObj, resObj)) { try { - byte[] zbuf = new byte[deflateBound(rawsz)]; - + byte[] zbuf = new byte[deflateBound(deltaLen)]; ZipStream zs = new ZipStream(deflater(), zbuf); - bestDelta.writeTo(zs, null); - bestDelta = null; + if (deltaBuf instanceof byte[]) + zs.write((byte[]) deltaBuf, 0, deltaLen); + else + ((TemporaryBuffer.Heap) deltaBuf).writeTo(zs, null); + deltaBuf = null; int len = zs.finish(); - resObj.setCachedDelta(deltaCache.cache(zbuf, len, rawsz)); - resObj.setCachedSize(rawsz); + resObj.setCachedDelta(deltaCache.cache(zbuf, len, deltaLen)); + resObj.setCachedSize(deltaLen); } catch (IOException err) { - deltaCache.credit(rawsz); + deltaCache.credit(deltaLen); } catch (OutOfMemoryError err) { - deltaCache.credit(rawsz); + deltaCache.credit(deltaLen); } } } @@ -468,4 +479,28 @@ final class DeltaWindow { throw new UnsupportedOperationException(); } } + + static final class ArrayStream extends OutputStream { + final byte[] buf; + int cnt; + + ArrayStream(int max) { + buf = new byte[max]; + } + + @Override + public void write(int b) throws IOException { + if (cnt == buf.length) + throw new IOException(); + buf[cnt++] = (byte) b; + } + + @Override + public void write(byte[] b, int off, int len) throws IOException { + if (len > buf.length - cnt) + throw new IOException(); + System.arraycopy(b, off, buf, cnt, len); + cnt += len; + } + } } From c9707e6353617f86cc75a7692797e1ed45e47cd3 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Fri, 12 Apr 2013 12:59:02 -0700 Subject: [PATCH 9/9] Always attempt delta compression when reuseDeltas is false If reuseObjects=true but reuseDeltas=false the caller wants attempt a delta for every object in the input list. Test for reuseDeltas to ensure every object passes through the searchInWindow() method. If no delta is possible for an object and it will be stored whole (non-delta format), PackWriter may still reuse its content from any source pack. This avoids an inflate()-deflate() cycle to recompress the object contents. Change-Id: I845caeded419ef4551ef1c85787dd5ffd73235d9 --- .../src/org/eclipse/jgit/internal/storage/pack/PackWriter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java index 2e6812c1c..54a5826c0 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java @@ -1996,7 +1996,7 @@ public class PackWriter { otp.clearReuseAsIs(); } - otp.setDeltaAttempted(next.wasDeltaAttempted()); + otp.setDeltaAttempted(reuseDeltas & next.wasDeltaAttempted()); otp.select(next); }