Re: [PATCH 3/3] packfile: use oidset for bad objects

> On Sat, Sep 11, 2021 at 06:08:38PM +0200, René Scharfe wrote:
>
>>>> +	nth_midxed_object_oid(&oid, m, pos);
>>>> +	if (oidset_contains(&p->bad_objects, &oid))
>>>> +		return 0;
>>>
>>> Calling nth_midxed_object_oid() implies a memcpy() under the hood. In
>>> the old code, we'd skip that in the common case that we had no corrupt
>>> objects, but now we'll pay the cost regardless. memcpy() isn't _that_
>>> expensive, but I'd expect this to be a relatively hot code path.
>>>
>>> Is it worth sticking all of this inside:
>>>
>>>   if (oidset_size(&p->bad_objects))
>>>
>>> ?
>>
>> Hard to say.  It would certainly match the old code more closely.  Is a
>> function call cheaper than copying 32 bytes?  Depends on the CPU and
>> whether the hash is cached, I guess.  And cached it probably is, because
>> the caller did a binary search for it..
>
> You already have a function call for nth_midxed_object_oid(), so
> checking oidset_size() would be a strict improvement.

>> We can pass on the original oid to avoid the nth_midxed_object_oid()
>> call, but inlining the whole thing might even be nicer.
>
> Yeah, it occurs to me that oidset_size() would be a good candidate for
> inlining, if that's what you mean.