Skip to content

Commit 183a638

Browse files
peffgitster
authored andcommitted
hashcmp: assert constant hash size
Prior to 509f6f6 (cache: update object ID functions for the_hash_algo, 2018-07-16), hashcmp() called memcmp() with a constant size of 20 bytes. Some compilers were able to turn that into a few quad-word comparisons, which is faster than actually calling memcmp(). In 509f6f6, we started using the_hash_algo->rawsz instead. Even though this will always be 20, the compiler doesn't know that while inlining hashcmp() and ends up just generating a call to memcmp(). Eventually we'll have to deal with multiple hash sizes, but for the upcoming v2.19, we can restore some of the original performance by asserting on the size. That gives the compiler enough information to know that the memcmp will always be called with a length of 20, and it performs the same optimization. Here are numbers for p0001.2 run against linux.git on a few versions. This is using -O2 with gcc 8.2.0. Test v2.18.0 v2.19.0-rc0 HEAD ------------------------------------------------------------------------------ 0001.2: 34.24(33.81+0.43) 34.83(34.42+0.40) +1.7% 33.90(33.47+0.42) -1.0% You can see that v2.19 is a little slower than v2.18. This commit ended up slightly faster than v2.18, but there's a fair bit of run-to-run noise (the generated code in the two cases is basically the same). This patch does seem to be consistently 1-2% faster than v2.19. I tried changing hashcpy(), which was also touched by 509f6f6, in the same way, but couldn't measure any speedup. Which makes sense, at least for this workload. A traversal of the whole commit graph requires looking up every entry of every tree via lookup_object(). That's many multiples of the numbers of objects in the repository (most of the lookups just return "yes, we already saw that object"). [jn: verified using "make object.s" that the memcmp call goes away.] Reported-by: Derrick Stolee <[email protected]> Signed-off-by: Jeff King <[email protected] Reviewed-by: Jonathan Nieder <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 29d9e3e commit 183a638

File tree

1 file changed

+10
-0
lines changed

1 file changed

+10
-0
lines changed

cache.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1023,6 +1023,16 @@ extern const struct object_id null_oid;
10231023

10241024
static inline int hashcmp(const unsigned char *sha1, const unsigned char *sha2)
10251025
{
1026+
/*
1027+
* This is a temporary optimization hack. By asserting the size here,
1028+
* we let the compiler know that it's always going to be 20, which lets
1029+
* it turn this fixed-size memcmp into a few inline instructions.
1030+
*
1031+
* This will need to be extended or ripped out when we learn about
1032+
* hashes of different sizes.
1033+
*/
1034+
if (the_hash_algo->rawsz != 20)
1035+
BUG("hash size not yet supported by hashcmp");
10261036
return memcmp(sha1, sha2, the_hash_algo->rawsz);
10271037
}
10281038

0 commit comments

Comments
 (0)