Re: [PATCH] progress: simplify "delayed" progress API

[Jeff King]

On Sat, Aug 19, 2017 at 10:39:41AM -0700, Junio C Hamano wrote:

> We used to expose the full power of the delayed progress API to the
> callers, so that they can specify, not just the message to show and
> expected total amount of work that is used to compute the percentage
> of work performed so far, the percent-threshold parameter P and the
> delay-seconds parameter N.  The progress meter starts to show at N
> seconds into the operation only if the amount of work completed
> exceeds P.
> 
> Most callers used either (0%, 2s) or (50%, 1s) as (P, N), but there
> are oddballs that chose more random-looking values like 95%.
> 
> For a smoother workload, (50%, 1s) would allow us to start showing
> the progress meter earlier than (0%, 2s), while keeping the chance
> of not showing progress meter for long running operation the same as
> the latter.  For a task that would take 2s or more to complete, it
> is likely that less than half of it would complete within the first
> second, if the workload is smooth.  But for a spiky workload whose
> earlier part is easier, such a setting is likely to fail to show the
> progress meter entirely and (0%, 2s) is more appropriate.
> 
> But that is merely a theory.  Realistically, it is of dubious value
> to ask each codepath to carefully consider smoothness of their
> workload and specify their own setting by passing two extra
> parameters.  Let's simplify the API by dropping both parameters and
> have everybody use (0%, 2s).

Nicely explained (modulo the reversal you noticed in the first
paragraph). The patch looks good to me.

> Oh, by the way, the percent-threshold parameter and the structure
> member were consistently misspelled, which also is now fixed ;-)

Heh, that was bugging me, too. :)

>  * So before I forget all about this topic, here is a patch to
>actually do this.
> 
>> So I'd vote to drop that parameter entirely. And if 1 second seems
>> noticeably snappier, then we should probably just move everything to a 1
>> second delay (I don't have a strong feeling either way).
> 
>I was also tempted to do this, but decided to keep it closer to
>the original for majority of callers by leaving the delay at 2s.
>With this patch, such a change as a follow-up is made a lot
>easier (somebody may want to even make a configuration out of it,
>but that would not be me).

Yeah, I agree that it can come later on top (if at all).

Thanks for finishing this off.

-Peff

Re: [PATCH] progress: simplify "delayed" progress API

[Junio C Hamano]

Junio C Hamano  writes:

> We used to expose the full power of the delayed progress API to the
> callers, so that they can specify, not just the message to show and
> expected total amount of work that is used to compute the percentage
> of work performed so far, the percent-threshold parameter P and the
> delay-seconds parameter N.  The progress meter starts to show at N
> seconds into the operation only if the amount of work completed
> exceeds P.

Oops, we inspect the doneness at N seconds and show only if we have
not yet completed P percent of the total work.

Perhaps

... at N seconds into the operation only if we have not
completed P per-cent of the total work.