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

[Derrick Stolee <stolee@gmail.com>]

On 1/6/2021 8:37 PM, Junio C Hamano wrote:
> "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