Re: [PATCH 09/10] name-rev: generate name strings only if they are better

From: René Scharfe <l.s.r@web.de>
Date: Feb 5, 2020, 15:50 UTC
Message-ID: <1e9251b2-4bbf-48b8-b7f7-e0835fefeeb0@web.de>
In-Reply-To: <3a59d8b4-3c3d-3812-9b7e-ac7e331ccd1c@gmail.com>

Am 05.02.20 um 16:11 schrieb Derrick Stolee:

Show 13 quoted lines

> On 2/4/2020 4:25 PM, René Scharfe wrote:
>> -			if (create_or_update_name(parent, new_name, taggerdate,
>> -						  generation, distance,
>> -						  from_tag)) {
>> +			parent_name = create_or_update_name(parent, taggerdate,
>> +							    generation,
>> +							    distance, from_tag);
>> +			if (parent_name) {
>
> As someone unfamiliar with the name-rev code, it took me a while to see why
> the algorithm isn't exponential in complexity. It technically _is_, but it
> is of the form 2^{N / MERGE_TRAVERSAL_WEIGHT} = 2^{N / 65535} and only if
> we create a particularly nasty commit history that would never appear in the wild.

As I understand it, name_rev() attaches a name (a tag or other reference) to a commit, and then goes through all its ancestors, depth first, and attaches a name based on that to all of them as well. It stops traversal if the name is not better than a previously attached name. Each commit has at most one name.

If we have N commits and M tags then we could traverse N * M commits and set their names in the worst case, if the M tags are all for the very latest commit and if the tags are ordered from worst to best. Right?

Which implies that ordering names before attaching them one by one should yield another speedup. Older tags are preferred and should be tried first, followed by younger tags and non-tags. Hmm.. :)

> But, the critical section is the block above. The confusing part was that
> create_or_update_name() returns NULL if the name is updated, but non-NULL if
> a better name is created. That makes it clear that this change is saving
> allocations.

create_or_update_name() returns NULL if the new name is not better than an existing one, and the old name is left untouched. It returns a pointer to the name record if there either was no old name or if that name was worse. In that case it updates the name.

After this patch it doesn't set the name string (tip_name) anymore, which is left for the caller. And the benefit is that the caller only needs to prepare said name string if the new name is better.

René