[jgit-dev] space regression in PackWriter

[Shawn Pearce <spearce@xxxxxxxxxxx>]

Yesterday I found a major space regression in PackWriter... for
example C Git was packing 7 MB larger (36M instead of 29M). Today I
tracked it down with git bisect to this commit. Unfortunately it was
added as a performance improvement when pack.deltaCompress = true. The
"tiny optimization" might be misleading, if there are 30k some objects
not delta compressed, this patch saves a lot of time inflating them
and checking them in the window search algorithm. I'm fairly certain
this came about as a "performance improvement" to improve the DHT
environment, where loading objects is very slow. Unfortunately the
space increase isn't worthwhile.


commit 67b064fc9fa7418fab83957b4f4e4baf9c6e08be
Author: Shawn O. Pearce <spearce@xxxxxxxxxxx>
Date:   Tue Mar 1 09:28:11 2011 -0800

    PackWriter: Do not delta compress already packed objects

    This is a tiny optimization to how delta search works.  Checking for
    isReuseAsIs() avoids doing delta compression search on non-delta
    objects already stored in packs within the repository.  Such objects
    are not likely to be delta compressable, as they were already delta
    searched when their containing pack was generated and they were
    not delta compressed at that time.  Doing delta compression now is
    unlikely to produce a different result, but would waste a lot of CPU.

    The isReuseAsIs() flag is checked before isDoNotDelta() because it
    is very common to reuse objects in the output pack.  Most objects
    get reused, and only a handful have the isDoNotDelta() bit set.
    Moving the check earlier allows the loop to more quickly skip
    through objects that will never need to be considered.

    Change-Id: Ied757363f775058177fc1befb8ace20fe9759bac
    Signed-off-by: Shawn O. Pearce <spearce@xxxxxxxxxxx>

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackWriter.java
b/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackWriter.java
index ee7ffae..eee1215 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackWriter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackWriter.java
@@ -792,9 +792,9 @@ public int compare(ObjectToPack a, ObjectToPack b) {

 	private int findObjectsNeedingDelta(ObjectToPack[] list, int cnt, int type) {
 		for (ObjectToPack otp : objectsLists[type]) {
-			if (otp.isDoNotDelta()) // delta is disabled for this path
+			if (otp.isReuseAsIs()) // already reusing a representation
 				continue;
-			if (otp.isDeltaRepresentation()) // already reusing a delta
+			if (otp.isDoNotDelta()) // delta is disabled for this path
 				continue;
 			otp.setWeight(0);
 			list[cnt++] = otp;


-- 
Shawn.