From e0111b18c8386d86eac2671b8cf639b63ec53560 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Wed, 27 Jul 2011 11:54:11 -0700 Subject: [PATCH 1/3] IndexPack: Fix "Resolving deltas" progress meter This progress meter never reached 100% as it did not update while resolving the external bases in thin packs. Instead of updating in batches at the top level, update once per delta that is resolved. The batching progress meter type should smooth out the frequent updates to an update rate that is more reasonable to send to the UI, while also ensuring a successful pack parse always reaches 100% deltas resolved. Change-Id: Ic77dcac542cfa97213a6b0194708f9d3c256d223 Signed-off-by: Shawn O. Pearce --- .../eclipse/jgit/transport/PackParser.java | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackParser.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackParser.java index 03370034b..653fd4c40 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackParser.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackParser.java @@ -478,6 +478,7 @@ public abstract class PackParser { if (!deferredCheckBlobs.isEmpty()) doDeferredCheckBlobs(); if (deltaCount > 0) { + resolving.beginTask(JGitText.get().resolvingDeltas, deltaCount); resolveDeltas(resolving); if (entryCount < objectCount) { if (!isAllowThin()) { @@ -494,6 +495,7 @@ public abstract class PackParser { (objectCount - entryCount))); } } + resolving.endTask(); } packDigest = null; @@ -518,20 +520,17 @@ public abstract class PackParser { private void resolveDeltas(final ProgressMonitor progress) throws IOException { - progress.beginTask(JGitText.get().resolvingDeltas, deltaCount); final int last = entryCount; for (int i = 0; i < last; i++) { - final int before = entryCount; - resolveDeltas(entries[i]); - progress.update(entryCount - before); + resolveDeltas(entries[i], progress); if (progress.isCancelled()) throw new IOException( JGitText.get().downloadCancelledDuringIndexing); } - progress.endTask(); } - private void resolveDeltas(final PackedObjectInfo oe) throws IOException { + private void resolveDeltas(final PackedObjectInfo oe, + ProgressMonitor progress) throws IOException { UnresolvedDelta children = firstChildOf(oe); if (children == null) return; @@ -559,12 +558,14 @@ public abstract class PackParser { .getOffset())); } - resolveDeltas(visit.next(), info.type, info); + resolveDeltas(visit.next(), info.type, info, progress); } private void resolveDeltas(DeltaVisit visit, final int type, - ObjectTypeAndSize info) throws IOException { + ObjectTypeAndSize info, ProgressMonitor progress) + throws IOException { do { + progress.update(1); info = openDatabase(visit.delta, info); switch (info.type) { case Constants.OBJ_OFS_DELTA: @@ -749,7 +750,8 @@ public abstract class PackParser { entries[entryCount++] = oe; visit.nextChild = firstChildOf(oe); - resolveDeltas(visit.next(), typeCode, new ObjectTypeAndSize()); + resolveDeltas(visit.next(), typeCode, + new ObjectTypeAndSize(), progress); if (progress.isCancelled()) throw new IOException( From c81f6ab3abf9fecc298294b44385538ac9e80e92 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Wed, 27 Jul 2011 11:59:31 -0700 Subject: [PATCH 2/3] IndexPack: Defer the "Resolving deltas" progress meter If delta resolution completes in < 1000 milliseconds, don't bother showing the progress meter. This is actually very common for a Gerrit Code Review server, where the client is probably sending 1 commit and only a few trees/blobs modified... and the base objects are hot in the process buffer cache. The 1000 millisecond delay is just a guess at a reasonable time to wait. Change-Id: I440baa64ab0dfa21be61deae8dcd3ca061bed8ce Signed-off-by: Shawn O. Pearce --- .../src/org/eclipse/jgit/transport/PackParser.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackParser.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackParser.java index 653fd4c40..1b30e859e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackParser.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackParser.java @@ -54,6 +54,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Comparator; import java.util.List; +import java.util.concurrent.TimeUnit; import java.util.zip.DataFormatException; import java.util.zip.Inflater; @@ -61,6 +62,7 @@ import org.eclipse.jgit.JGitText; import org.eclipse.jgit.errors.CorruptObjectException; import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.lib.AnyObjectId; +import org.eclipse.jgit.lib.BatchingProgressMonitor; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.InflaterCache; import org.eclipse.jgit.lib.MutableObjectId; @@ -478,6 +480,11 @@ public abstract class PackParser { if (!deferredCheckBlobs.isEmpty()) doDeferredCheckBlobs(); if (deltaCount > 0) { + if (resolving instanceof BatchingProgressMonitor) { + ((BatchingProgressMonitor) resolving).setDelayStart( + 1000, + TimeUnit.MILLISECONDS); + } resolving.beginTask(JGitText.get().resolvingDeltas, deltaCount); resolveDeltas(resolving); if (entryCount < objectCount) { From 68cc21b60d83b5c4fb1de6c34a79836c51dd9b3b Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Wed, 27 Jul 2011 12:10:19 -0700 Subject: [PATCH 3/3] PackWriter: Skip progress messages on fast operations If the "Finding sources" phase will complete in <1 second with no delta compression enabled, don't bother showing the progress meter for this phase. Small repositories on the local filesystem tend to rip through this phase always subsecond and the ProgressMonitor display can actually slow the operation down. If delta compression is enabled, there are two phases that may run very quickly. Set the timer to 500 milliseconds instead, reducing the risk that the user has to wait longer than 1 second before any sort of output from the packer occurs. Change-Id: I58110f17e2a5ffa0134f9768b94804d16bbb8399 Signed-off-by: Shawn O. Pearce --- .../eclipse/jgit/storage/pack/PackWriter.java | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) 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 8a912aeb6..3007b809c 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 @@ -80,6 +80,7 @@ import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.errors.StoredObjectRepresentationNotAvailableException; import org.eclipse.jgit.lib.AnyObjectId; import org.eclipse.jgit.lib.AsyncObjectSizeQueue; +import org.eclipse.jgit.lib.BatchingProgressMonitor; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.NullProgressMonitor; import org.eclipse.jgit.lib.ObjectId; @@ -640,10 +641,21 @@ public class PackWriter { if (writeMonitor == null) writeMonitor = NullProgressMonitor.INSTANCE; - if (reuseSupport != null && ( + boolean needSearchForReuse = reuseSupport != null && ( reuseDeltas || config.isReuseObjects() - || !cachedPacks.isEmpty())) + || !cachedPacks.isEmpty()); + + if (compressMonitor instanceof BatchingProgressMonitor) { + long delay = 1000; + if (needSearchForReuse && config.isDeltaCompress()) + delay = 500; + ((BatchingProgressMonitor) compressMonitor).setDelayStart( + delay, + TimeUnit.MILLISECONDS); + } + + if (needSearchForReuse) searchForReuse(compressMonitor); if (config.isDeltaCompress()) searchForDeltas(compressMonitor);