git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] sha1_file: avoid comparison if no packed hash matches the first byte

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 8, 2017, 22:43 UTC
Message-ID
<xmqq3791adfi.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<0c1f898c-46c4-033d-001b-114b17d7d36f@web.de>
René Scharfe <l.s.r@web.de> writes:
Show 39 quoted lines
> find_pack_entry_one() uses the fan-out table of pack indexes to find out
> which entries match the first byte of the searched hash and does a
> binary search on this subset of the main index table.
>
> If there are no matching entries then lo and hi will have the same
> value.  The binary search still starts and compares the hash of the
> following entry (which has a non-matching first byte, so won't cause any
> trouble), or whatever comes after the sorted list of entries.
>
> The probability of that stray comparison matching by mistake is low, but
> let's not take any chances and check when entering the binary search
> loop if we're actually done already.
>
> Signed-off-by: Rene Scharfe <l.s.r@web.de>
> ---
>  sha1_file.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/sha1_file.c b/sha1_file.c
> index b60ae15f70..11ee69a99d 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -2799,7 +2799,7 @@ off_t find_pack_entry_one(const unsigned char *sha1,
>  		return nth_packed_object_offset(p, pos);
>  	}
>  
> -	do {
> +	while (lo < hi) {
>  		unsigned mi = (lo + hi) / 2;
>  		int cmp = hashcmp(index + mi * stride, sha1);
>  
> @@ -2812,7 +2812,7 @@ off_t find_pack_entry_one(const unsigned char *sha1,
>  			hi = mi;
>  		else
>  			lo = mi+1;
> -	} while (lo < hi);
> +	}
>  	return 0;
>  }

Interesting. I see that we still have the conditional code to call out to sha1-lookup.c::sha1_entry_pos(). Do we need a similar change over there, I wonder? Alternatively, as we have had the experimental sha1-lookup.c::sha1_entry_pos() long enough without anybody using it, perhaps we should write it off as a failed experiment and retire it?

Previous: Jonathan NiederNext: Jeff King
Message 3 of 14 in “sha1_file: avoid comparison if no packed hash matches the first byte”
  1. sha1_file: avoid comparison if no packed hash matches the first byteRené Scharfe, Aug 8, 2017
  2. Jonathan NiederAug 8, 2017
  3. Junio C HamanoAug 8, 2017
  4. Jeff KingAug 8, 2017
  5. Jeff KingAug 8, 2017
  6. Junio C HamanoAug 9, 2017
  7. Jeff KingAug 9, 2017
  8. Jeff KingAug 9, 2017
  9. 1/2 sha1_file: drop experimental GIT_USE_LOOKUP searchJeff King, Aug 9, 2017
  10. Junio C HamanoAug 9, 2017
  11. Jeff KingAug 9, 2017
  12. 2/2 hashcmp: use memcmp instead of open-coded loopJeff King, Aug 9, 2017
  13. René ScharfeAug 9, 2017
  14. Jeff KingAug 9, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.