Re: [PATCH 1/2] sha1_file: drop experimental GIT_USE_LOOKUP search

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> This lets us remove sha1_entry_pos() entirely, as the .idx
> lookup code was the only caller.  Note that sha1-lookup.c
> still contains sha1_pos(), which differs from
> sha1_entry_pos() in two ways:
>
>   - it has a different interface; it uses a function pointer
>     to access sha1 entries rather than a size/offset pair
>     describing the table's memory layout
>
>   - it only scales the initial selection of "mi", rather
>     than each iteration of the search
>
> We can't get rid of this function, as it's called from
> several places. It may be that we could replace it with a
> simple binary search, but that's out of scope for this patch
> (and would need benchmarking).

Thanks for reducing the count of binary search functions by one.

I think the "just one round of newton-raphson" in sha1_pos() comes
from [*1*]; I agree that it needs benchmarking before tweaking it.

We may want to tell libgit2 folks about this change, though [*2*].
I think they too are carrying dead code that is only used under CPP
macro GIT_USE_LOOKUP, which they do not seem to define.


[Reference]

*1* https://public-inbox.org/git/7v38vso8kz.fsf@alter.siamese.dyndns.org/
*2* https://github.com/libgit2/libgit2/commit/dd453c4dbf9a1fa38530b1f51e079852736b8f66