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

!MAILaRCHIVE_VOTE_RePLACE
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
To: Nicolas Pitre <nico@...>
Cc: Junio C Hamano <gitster@...>, <git@...>
Date: Wednesday, October 31, 2007 - 10:58 pm

Nicolas Pitre <nico@cam.org> wrote:

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.

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.

The display_progress() call already does a reasonably cheap
comparsion to see if the timer has tripped or if the percent complete
has changed.  So I figured it was more useful to get feedback to
the user that we were working, but were going to take a while,
than it was to optimize a few machine instructions out of that
inner-most per-object loop.

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.
 
-- 
Shawn.
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
[PATCH 0/5] more progress display stuff, Nicolas Pitre, (Tue Oct 30, 2:57 pm)
Re: [PATCH 1/5] prune-packed: don't call display_progress() ..., Shawn O. Pearce, (Wed Oct 31, 10:58 pm)
[PATCH 2/5] make struct progress an opaque type, Nicolas Pitre, (Tue Oct 30, 2:57 pm)
[PATCH 3/5] relax usage of the progress API, Nicolas Pitre, (Tue Oct 30, 2:57 pm)