Re: [PATCH 2/2] read-cache: fix incorrect count and progress bar stalling

> Am 07.06.21 um 17:58 schrieb Ævar Arnfjörð Bjarmason:
>>
>> On Mon, Jun 07 2021, Derrick Stolee wrote:
>>
>>> On 6/7/2021 10:43 AM, Ævar Arnfjörð Bjarmason wrote:
>>>> Fix a potential incorrect display of the number of items (off by one)
>>>> and stalling of the progress bar in refresh_index().
>>>>
>>>> The off-by-one error is minor, we should say we're processing the 1st
>>>> item, not the 0th. This along with the next change also allows us to
>>>> remove the last display_progress() call outside the loop, as we'll
>>>> always have reached 100% now.
>>>
>>> This "pre-announce the progress" seems correct and is unlikely
>>> to have a user sitting at "100%" while the loop is actually doing
>>> work on that last cache entry.
>>
>> I guess pre-announce v.s. post-announce is a matter of some philosophy,
>> for O(n) when can we be said to be doing work on n[0]? We entered the
>> for-loop and are doing work on that istate->cache[i] item, so I'd like
>> to think of it more as post-announce :)
>
> Say you have a single item to process and it takes a minute.  The
> original code shows 0% for a minute, then 100% at the end.  With your
> change you'd get 100% for a minute.  Both would be annoying, but the
> latter would have me raging.  "If you're done", I'd yell at the uncaring
> machine, "what are you still doing!?".

> Showing only the completed items makes sense.  That the next one is
> being processed is self-understood.  Once all of them are done, 100% is
> shown and the progress line is finished.
>
> So I think this pattern works:
>
> 	for (i = 0; i < nr; i++) {
> 		display_progress(p, i);
> 		/* work work work */
> 	}
> 	display_progress(p, nr);
>
> Alternatively, if the work part doesn't contain continue statements:
>
> 	for (i = 0; i < nr; i++) {
> 		/* work work work */
> 		display_progress(p, i + 1);
> 	}