Re: [PATCH 1/2] sha1_file: drop experimental GIT_USE_LOOKUP search
- Date: Wed, 09 Aug 2017 11:12:19 -0700
- From: Junio C Hamano <gitster@xxxxxxxxx>
- Subject: Re: [PATCH 1/2] sha1_file: drop experimental GIT_USE_LOOKUP search
Jeff King <peff@xxxxxxxx> 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.