On Wed, May 08 2019, Jeff King wrote: > On Tue, May 07, 2019 at 10:12:06AM +0200, Ævar Arnfjörð Bjarmason wrote: > >> > I think we'd want a way to tell the bitmap code to update our progress >> > meter as it traverses (both single objects, but also taking into account >> > when it finds a bitmap and then suddenly bumps the value by a large >> > amount). >> >> Not splitting it will fix the progress bar stalling, so it fixes the >> problem that the user is wondering if the command is entirely hanging. >> >> But I was hoping to give the user an idea of roughly where we're >> spending our time, e.g. so you can see how much the pack.useSparse >> setting is helping (or not). > > Yeah, I think that's a bigger and more complicated problem. I admit that > my main annoyance is just the stall while we fill in the bitmaps (and > it's easy because the bitmap traversal is the same unit of work as a > regular traversal). > >> So something where we report sub-progress as we go along, and perhaps >> print some brief summary at the end if it took long enough, e.g.: >> >> Enumerating Objects (X^1%) => Marking trees (Y^1%) >> Enumerating Objects (X^2%) => Calculating bitmaps (Y^2%) >> >> And at the end: >> >> Enumerating Objects (100%) in ~2m30s -- (~10s marking trees, ~2m10s bitmaps, ~10s other) >> >> I.e. bringing the whole "nested" trace2 regions full circle with the >> progress bar where we could elect to trace/show some of that info, and >> then you could turn on some trace2 mode/verbose progress to see more. > > I do wonder if this really needs to be part of the progress bar. The > goal of the progress bar is to give the user a sense that work is > happening, and (if possible, but not for "enumerating") an idea of when > it might finish. If the trace code can already do detailed timings, then > shouldn't we just be encouraging people to use that? To just show work happening we could save ourselves some horizontal space and the debates over counting v.s. enumerating with: diff --git a/progress.c b/progress.c index 0318bdd41b..83336ca391 100644 --- a/progress.c +++ b/progress.c @@ -226,3 +226,3 @@ static struct progress *start_progress_delay(const char *title, uint64_t total, struct progress *progress = xmalloc(sizeof(*progress)); - progress->title = title; + progress->title = "Reticulating splines"; progress->total = total; :) Obviously that's silly, but the point is we do show some user messaging with these now, and e.g. the other day here on-list (can't be bothered to find the msgid) someone was lamenting that the N progressbars we show on "push" were too verbose. So by coalescing some of the existing bars that do one logical operation (push) in N steps we could be less verbose without losing the "stuff's happening" part of it, and would see if something odd was going on, e.g. the "I/O write" part being proportionally slower on this box than the other, or when they upgrade bitmaps suddenly showing up as >95% of the time. The bit I find interesting about tying it into trace2 is that once you do that the trace logs can contain e.g. min/max/avg/median/percentile time for doing some operation we can break into N steps same/similar steps, which might be interesting for performance analysis.