Re: [PATCH v2] cache.h: eliminate SHA-1 deprecation warnings on OS X 10.8
Hi, David Aguilar wrote: Mac OS X Mountain Lion prints warnings when building git: warning: 'SHA1_Init' is deprecated (declared at /usr/include/openssl/sha.h:121) Silence the warnings by using the Common Digest SHA-1 functions for SHA1_Init(), SHA1_Update(), and SHA1_Final(). Thanks. Does this perform better or worse than just setting BLK_SHA1=YesPlease? I'd naively think it could go either way: on one hand adding another library dependency can slow down startup, and on the other hand the implementation may or may not be optimized better than the generic block-sha1/ implementation. Curious, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] cache.h: eliminate SHA-1 deprecation warnings on OS X 10.8
On Fri, May 10, 2013 at 11:23 PM, Jonathan Nieder jrnie...@gmail.com wrote: Hi, David Aguilar wrote: Mac OS X Mountain Lion prints warnings when building git: warning: 'SHA1_Init' is deprecated (declared at /usr/include/openssl/sha.h:121) Silence the warnings by using the Common Digest SHA-1 functions for SHA1_Init(), SHA1_Update(), and SHA1_Final(). Thanks. Does this perform better or worse than just setting BLK_SHA1=YesPlease? I'd naively think it could go either way: on one hand adding another library dependency can slow down startup, and on the other hand the implementation may or may not be optimized better than the generic block-sha1/ implementation. Pretty much identical. Here are the timings (I should probably read t/perf/README and get better numbers): Best of ten $ time git rev-list --all --objects /dev/null # master git rev-list --all --objects /dev/null 5.16s user 0.11s system 99% cpu 5.277 total # BLK_SHA1 git rev-list --all --objects /dev/null 5.17s user 0.11s system 99% cpu 5.299 total # CommonDigest git rev-list --all --objects /dev/null 5.18s user 0.11s system 99% cpu 5.301 total The library startup cost is identical to master since at the end of the day it's still libcrypto. FWIW the way it's done in this patch still allows defining BLK_SHA1, which is parity with how the Makefile behaves on Linux. Re-roll? -- David -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] cache.h: eliminate SHA-1 deprecation warnings on OS X 10.8
On Sat, May 11, 2013 at 12:11:05AM -0700, David Aguilar wrote: Does this perform better or worse than just setting BLK_SHA1=YesPlease? I'd naively think it could go either way: on one hand adding another library dependency can slow down startup, and on the other hand the implementation may or may not be optimized better than the generic block-sha1/ implementation. Pretty much identical. Here are the timings (I should probably read t/perf/README and get better numbers): Best of ten $ time git rev-list --all --objects /dev/null [...] I'm not sure that's a great test of sha1 performance. It will hash the commit and tree objects it loads during the traversal, but that time is almost certainly dwarfed by zlib inflation and by lookup_object. 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. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] cache.h: eliminate SHA-1 deprecation warnings on OS X 10.8
On Sat, May 11, 2013 at 1:22 AM, Jeff King p...@peff.net wrote: On Sat, May 11, 2013 at 12:11:05AM -0700, David Aguilar wrote: Does this perform better or worse than just setting BLK_SHA1=YesPlease? I'd naively think it could go either way: on one hand adding another library dependency can slow down startup, and on the other hand the implementation may or may not be optimized better than the generic block-sha1/ implementation. Pretty much identical. Here are the timings (I should probably read t/perf/README and get better numbers): Best of ten $ time git rev-list --all --objects /dev/null [...] I'm not sure that's a great test of sha1 performance. It will hash the commit and tree objects it loads during the traversal, but that time is almost certainly dwarfed by zlib inflation and by lookup_object. 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... -- David -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] cache.h: eliminate SHA-1 deprecation warnings on OS X 10.8
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, -O00m21.152s BLK_SHA1, -O00m31.920s BLK_SHA1, -O30m19.652s So it is a win over openssl with optimizations on, but a pretty big loss otherwise. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] cache.h: eliminate SHA-1 deprecation warnings on OS X 10.8
On Sat, May 11, 2013 at 1:45 AM, Jeff King p...@peff.net wrote: 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, -O00m21.152s BLK_SHA1, -O00m31.920s BLK_SHA1, -O30m19.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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] cache.h: eliminate SHA-1 deprecation warnings on OS X 10.8
On Sat, May 11, 2013 at 02:17:10AM -0700, David Aguilar wrote: 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. With those numbers above, it seems like CommonCrypto is still a reasonable choice. I don't know anything about availability or other OS X specific issues, but assuming there are no issues there, your patch makes sense to me. Thanks for double-checking the timings. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
