Re: [PATCH] revision: trace topo-walk statistics

From: Derrick Stolee <stolee@gmail.com>
Date: Jan 7, 2021, 02:04 UTC
Message-ID: <cbc17f1b-57fc-497f-f1ab-baa8cc84620d@gmail.com>
In-Reply-To: <xmqqh7nto7ck.fsf@gitster.c.googlers.com>

On 1/6/2021 8:37 PM, Junio C Hamano wrote:

Show 19 quoted lines

> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> diff --git a/revision.c b/revision.c
>> index 9dff845bed6..1bb590ece78 100644
>> --- a/revision.c
>> +++ b/revision.c
>> @@ -3308,6 +3308,26 @@ struct topo_walk_info {
>>  	struct author_date_slab author_date;
>>  };
>>  
>> +static int topo_walk_atexit_registered;
>> +static unsigned int count_explore_walked;
>> +static unsigned int count_indegree_walked;
>> +static unsigned int count_topo_walked;
> 
> The revision walk machinery is designed to be callable more than
> once during the lifetime of a process.  These make readers wonder
> if they should be defined in "struct rev_info" to allow stats
> collected per traversal.

That's possible, but the use of an at-exit method means we only report one set of statistics and the 'struct rev_info' might be defunct.

It does limit how useful the statistics can be when there are multiple 'struct rev_info's in use, but we also cannot trust that the rev_infos are being cleaned up properly at the end of the process to trigger the stats logging.

Of course, maybe that _is_ something we could guarantee, or rather _should_ guarantee by patching any leaks. Seems like a lot of work when these aggregate statistics will be effective enough. But maybe I'm judging the potential work too harshly?

Thanks, -Stolee