Re: [PATCH 4/3] midx: inline nth_midxed_pack_entry()

From: Jeff King <peff@peff.net>
Date: Sep 11, 2021, 17:07 UTC
Message-ID: <YTziaNywmCMn07IS@coredump.intra.peff.net>
In-Reply-To: <7d9e67bf-e057-694c-c976-ba19e9521882@web.de>

On Sat, Sep 11, 2021 at 06:08:42PM +0200, René Scharfe wrote:

> fill_midx_entry() finds the position of an object ID and passes it to
> nth_midxed_pack_entry(), which uses the position to look up the object
> ID for its own purposes.  Inline the latter into the former to avoid
> that lookup.

Ah, I see what you mean now by "inline" in the other part of the thread.

Yes, I think this makes sense since there is no other reasonable caller of the nth_midxed_pack_entry() helper (and its one caller is itself trivial).

Show 8 quoted lines

> @@ -304,8 +307,7 @@ static int nth_midxed_pack_entry(struct repository *r,
>  	if (!is_pack_valid(p))
>  		return 0;
> 
> -	nth_midxed_object_oid(&oid, m, pos);
> -	if (oidset_contains(&p->bad_objects, &oid))
> +	if (oidset_contains(&p->bad_objects, oid))
>  		return 0;

So we get to avoid the nth_midxed_object_oid() copy entirely. Very nice.

Compared to the code before your series, we still have an extra function call to oidset_contains(), which will (in the common case) notice we have no entries and immediately return. But I think that's getting into pointless micro-optimization.

-Peff