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

From: Jeff Hostetler <git@jeffhostetler.com>
Date: Mar 19, 2019, 14:00 UTC
Message-ID: <d3f119b1-22f6-e34a-a68f-883aca3d8013@jeffhostetler.com>
In-Reply-To: <87lg1bet9d.fsf@evledraar.gmail.com>

On 3/18/2019 6:02 PM, Ævar Arnfjörð Bjarmason wrote:

Show 7 quoted lines

> 
> 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:
>>>

[...]

Show 14 quoted lines

>>>> +	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.
>>

[...]

Show 21 quoted lines

>>
>> 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.

Right? Jeff