Re: [PATCH v3 09/16] refs/packed-backend.c: implement jump lists to avoid excluded pattern(s)

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 20, 2023, 18:49 UTC
Message-ID: <xmqqedm6m0ju.fsf@gitster.g>
In-Reply-To: <ZJGV/bYDN9Dn4V8S@nand.local>

Taylor Blau <me@ttaylorr.com> writes:

Show 24 quoted lines

>> > +static const char *ptr_max(const char *x, const char *y)
>> > +{
>> > +	if (x > y)
>> > +		return x;
>> > +	return y;
>> > +}
>>
>> Hopefully the compiler would inline the function without being told.
>>
>> These pointers point into the same mmapped region of contiguous
>> memory that holds the contents of the packed-refs file, so
>> comparison between them is always defined.  Good.
>>
>> I wondered if
>>
>> 	return (x > y) ? x : y;
>>
>> is easier to read, simply because it treats both cases more equally
>> (in other words, as written, (x>y) appears more "special"), but that
>> is minor.
>
> Yeah, I think that any reasonable compiler would almost certainly inline
> this, especially at higher optimization levels. But I agree with your
> suggestion nonetheless, thanks.

Having seen how this is used (only at a single callsite), I actually think that special casing (x>y) is the right thing to do, especially if you inline it in the caller. That is,

	if (last_disjoint->end < ours->end)
		last_disjoint->end = ours->end;

reads much more naturally than

	last_disjoint->end = (last_disjoint->end > ours->end)
		? last_disjoint->end : ours_end;

as a way to say "if ours is larger, record it as the largest position we have seen so far".