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.ga14...@sigill.intra.peff.net/ > > 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
signature.asc
Description: PGP signature
