Re: [PATCH 1/5] prune-packed: don't call display_progress() for every file

From: Nicolas Pitre <nico@cam.org>
Date: Nov 1, 2007, 12:06 UTC
Message-ID: <alpine.LFD.0.9999.0711010736430.21255@xanadu.home>
In-Reply-To: <20071101025830.GX14735@spearce.org>

On Wed, 31 Oct 2007, Shawn O. Pearce wrote:

Show 18 quoted lines

> Nicolas Pitre <nico@cam.org> wrote:
> > The progress count is per fanout directory, so it is useless to call
> > it for every file as the count doesn't change that often.
> 
> If you go back into the history and look at the commit message for
> when I introduced this per-object display_progress() call we find
> the following:
> 
>  commit b5d72f0a4cd3cce945ca0d37e4fa0ebbfcdcdb52
>  Author: Shawn O. Pearce <spearce@spearce.org>
>  Date:   Fri Oct 19 00:08:37 2007 -0400
> 
> [...snip...]
>     We perform the display_progress() call from within the very innermost
>     loop in case we spend more than 1 second within any single object
>     directory.  This ensures that a progress_update event from the
>     timer will still trigger in a timely fashion and allow the user to
>     see the progress meter.

Hmmmm OK.  I overlooked that.

Show 5 quoted lines

> During my testing with a 40,000 loose object case (yea, I fully
> unpacked a git.git clone I had laying around) my system stalled
> hard in the first object directory.  A *lot* longer than 1 second.
> So I got no progress meter for a long time, and then a progress
> meter appeared on the second directory.

But then don't you get a "0% (1/256)" on the screen for a while, like if it was stalled? Might be better than nothing at all I suppose...

> So I'm a little against this patch.  But I think I understand why
> you think its worth doing.  I just consider the progress feedback
> more important than the few machine cycles avoiding it saves.

OK, I don't mind reverting the patch, or actually providing another one to move the call back inside the loop since other changes will prevent a clean revert.

Nicolas