Subject: Re: [PATCH 2/3] midx: verify: group objects by packfile to speed up object verification

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Tue, 19 Mar 2019 15:06:45 +0100

On Tue, Mar 19 2019, Jeff Hostetler wrote:

> On 3/18/2019 6:02 PM, Ævar Arnfjörð Bjarmason wrote:
>>
>> On Mon, Mar 18 2019, Jeff Hostetler wrote:
>>
>>> On 3/18/2019 11:53 AM, Ævar Arnfjörð Bjarmason wrote:
>>>>
>>>> On Mon, Mar 18 2019, Jeff Hostetler via GitGitGadget wrote:
>>>>
> [...]
>>>>> +	QSORT(pairs, m->num_objects, compare_pair_pos_vs_id);
>>>>> +
>>>>> +	for (k = 0; k < m->num_objects; k++) {
>>>>> [...]
>>>>
>>>> I have not tested this (or midx in general), but isn't this new QSORT()
>>>> introducing the same sort of progress stalling that I fixed for
>>>> commit-graph in 890226ccb57 ("commit-graph write: add itermediate
>>>> progress", 2019-01-19)? I.e. something you can work around with a
>>>> "display_progress(progress, 0)" before the QSORT().
>>>>
>>>
>>> I wasn't tracking your commit-graph changes, but yes, I think it is.
>>>
> [...]
>>>
>>> There is the dead time while the sort() itself is running, but at least
>>> there is isn't a 5+ second frozen at 0% message on screen.
>>
>> Yeah, the same with the commit-graph with my hack. I.e. it'll sit there,
>> but at least it sits like this:
>>
>>      What I was doing before 100% (X/Y)
>>      What I'm about to start doing 0% (0/Z) [hanging]
>>
>> Instead of:
>>
>>      What I was doing before 100% (X/Y)
>>      [hanging]
>>
>> So that's an improvement, i.e. you know it's started that next phase at
>> least instead of just having a non-descriptive hang.
>>
>> Ideally there would be some way to reach into the QSORT() and display
>> progress there, but that's all sorts of nasty, so as the TODO comment in
>> commit-graph.c notes I punted it.
>
> Perhaps I'm confused or this is a Windows issue, but when I do:
>
> 	progress = start_delayed_progress("sorting", n);
> 	display_progress(progress, 0);
> 	QSORT(...);
> 	stop_progress(&progress);
>
> I never see the 0% message.  It always does the hang with the cursor in
> column 0 on a blank line.  If I make this a regular start_progress(),
> I do see the 0% message for the duration of the qsort hang.
>
> I did some similar testing around your QSORT() in commit-graph.c
> and got the same result.  It looks like start_delayed_processing()
> wants to wait at least 2 seconds before displaying anything and has
> an interval timer to notify it that another message should be printed,
> but the display_progress(0) prior to the QSORT() arrives before the 2
> seconds are up and so nothing is printed.  It's not until we get into
> the loop below the QSORT that one of the display_progress(i+1) calls
> could cause a message to appear.

Correct, it'll still hang the N seconds that start_delayed_progress()
imposes.

In the commit-graph code this would sometimes take longer than that, so
I'd see the 0% earlier.

But maybe in this case even on humongous repositories it's faster than
that.