On Sat, Aug 25, 2018 at 04:00:31AM -0400, Jeff King wrote: > This is a follow-up to the discussion in: > > https://public-inbox.org/git/20180822030344.GA14684@xxxxxxxxxxxxxxxxxxxxx/ > > The general idea is that the majority of callers don't care about actual > plus/minus ordering from oidcmp() and hashcmp(); they just want to know > if two oids are equal. The stricter equality check can be optimized > better by the compiler. > > Due to the simplicity of the current code and our inlining, the compiler > can usually figure this out for now. So I wouldn't expect this patch to > actually improve performance right away. But as that discussion shows, > we are likely to take a performance hit as we move to more runtime > determination of the_hash_algo parameters. Having these callers use the > more strict form will potentially help us recover that. > > So in that sense we _could_ simply punt on this series until then (and > it's certainly post-v2.19 material). But I think it's worth doing now, > simply from a readability/annotation standpoint. IMHO the resulting code > is more clear (though I've long since taught myself to read !foocmp() as > equality). I would quite like to see this series picked up for v2.20. If we want to minimize performance regressions with the SHA-256 work, I think it's important. Applying the following patch on top of this series causes gcc to inline both branches, which is pretty much the best we can expect. I haven't benchmarked it, though, so I can't say what the actual performance consequence is. As for this series itself, other than the typos people have pointed out, it looks good to me. diff --git a/cache.h b/cache.h index 3bb88ac6d0..1c182c6ef6 100644 --- a/cache.h +++ b/cache.h @@ -1033,7 +1033,10 @@ static inline int oidcmp(const struct object_id *oid1, const struct object_id *o static inline int hasheq(const unsigned char *sha1, const unsigned char *sha2) { - return !hashcmp(sha1, sha2); + if (the_hash_algo->rawsz == 32) { + return !memcmp(sha1, sha2, 32); + } + return !memcmp(sha1, sha2, 20); } static inline int oideq(const struct object_id *oid1, const struct object_id *oid2) -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204
Attachment:
signature.asc
Description: PGP signature
