On Tue, May 01, 2018 at 11:36:03AM +0200, Duy Nguyen wrote:
> On Mon, Apr 23, 2018 at 11:39:11PM +0000, brian m. carlson wrote:
> > diff --git a/cache.h b/cache.h
> > index bbaf5c349a..4bca177cf3 100644
> > --- a/cache.h
> > +++ b/cache.h
> > @@ -1008,6 +1008,11 @@ static inline void oidclr(struct object_id *oid)
> > memset(oid->hash, 0, GIT_MAX_RAWSZ);
> > }
> >
> > +static inline void oidread(struct object_id *oid, const unsigned char *hash)
> > +{
> > + memcpy(oid->hash, hash, the_hash_algo->rawsz);
>
> If performance is a concern, should we go with GIT_MAX_RAWSZ instead
> of the_hash_algo->rawsz which gives the compiler some more to bypass
> actual memcpy function and generate copy code directly?
I don't think we can do that. If we have both NewHash and SHA-1
compiled in and are using SHA-1, GIT_MAX_RAWSZ will be 32, but we may
only have 20 bytes that are valid to read.
> If it is not a performance problem, should we avoid inline and move
> the implementation somewhere?
I would like to make it as fast as possible if we can, especially since
hashcpy is inline. If you have concerns about performance, I can add a
patch in a future series that does some sort of macro if we're using gcc
that does something like the following:
({
int rawsz = the_hash_algo->rawsz;
if (rawsz != GIT_SHA1_RAWSZ && rawsz != GIT_MAX_RAWSZ)
__builtin_trap(); /* never reached */
rawsz;
})
And then use that instead of the_hash_algo->rawsz in performance
sensitive paths. That would mean the compiler would know that it was
only one of those two values and it could optimize better.
--
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204
Attachment:
signature.asc
Description: PGP signature
