Re: [PATCH v2] cache.h: eliminate SHA-1 deprecation warnings on OS X 10.8

From: David Aguilar <davvid@gmail.com>
Date: May 11, 2013, 09:17 UTC
Message-ID: <CAJDDKr4DoEUw=4O_m-91Hsg4SRZzkn6yJPf_nBsPsEv0Weibrw@mail.gmail.com>
In-Reply-To: <20130511084543.GA1905@sigill.intra.peff.net>

On Sat, May 11, 2013 at 1:45 AM, Jeff King <peff@peff.net> wrote:

Show 30 quoted lines

> On Sat, May 11, 2013 at 01:38:32AM -0700, David Aguilar wrote:
>
>> > Adding "--verify-objects" would sha1 the blobs, too, which might be more
>> > reasonable (or running "git fsck"). Something like "git add" on a large
>> > blob would also be a good test.
>>
>> Thanks.  Here are the numbers with --verify-objects:
>>
>> $ time git rev-list --all --objects --verify-objects >/dev/null
>>
>> # CommonCrypto 32.24s user 4.65s system 99% cpu 37.098 total
>> # master       33.00s user 4.68s system 99% cpu 37.852 total
>> # BLK_SHA1     54.17s user 4.67s system 99% cpu 58.928 total
>>
>> Doing BLK_SHA1 seems like less of a good idea now, so I think my
>> latest re-roll might be the way to go...
>
> Wow, that's really terrible. What level of optimization do you compile
> with? With the other implementations, you are calling into
> (presumably) optimized library code, but with BLK_SHA1 you are getting
> whatever you just compiled.
>
> Here are three timings that show how big a difference that can make:
>
>   openssl,  -O0    0m21.152s
>   BLK_SHA1, -O0    0m31.920s
>   BLK_SHA1, -O3    0m19.652s
>
> So it is a win over openssl with optimizations on, but a pretty big loss
> otherwise.

Good catch. I had a config.mak without any -O flags in CFLAGS. Here are the timings with -O3. We're back to parity.

$ time git rev-list --all --objects --verify-objects >/dev/null

# CommonCrypto 28.95s user 4.62s system 99% cpu 33.630 total # master 29.81s user 4.70s system 99% cpu 34.760 total # BLK_SHA1 29.80s user 4.62s system 99% cpu 34.505 total

If BLK_SHA1 were the default on all platforms then I wouldn't have bothered with the SHA-1 patch. With this patch it makes it like Linux in that Git can choose between the built-in functions and the external library.

That's why I moved this patch to 3/3.. it could go either way. -- David