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