-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
sha1: use openssl sha1 routines on mingw #915
sha1: use openssl sha1 routines on mingw #915
Conversation
Interesting. I seem to remember that we could not use OpenSSL's SHA-1 function originally because block-sha1 turned out to be faster or something. But the speed improvements are intriguing (would you mind amending the commit message with some measurements?) and I would like to have this change. The problem is that I ran out of time before I could find out how to test how bad it would be to fall back to OpenSSL's SHA-1 implementation that does not use Intel CPU features. And actually, I think that these improvements are contingent on the OpenSSL version, see https://software.intel.com/en-us/articles/improving-openssl-performance for details... Having said that, it appears that OpenSSL 1.0.2 (which Git for Windows uses) has those improvements. Maybe worth mentioning in the commit message? Finally, I would love it if we could not comment-out the line, but rather move it into the MINGW <2.0 section as well as the Git for Windows 1.x section... With those changes, and hopefully some testing I could somehow perform that hopefully verify that falling back to slower SHA-1 code in OpenSSL is not all that bad, I would be very happy to take those changes (and I'll take them upstream, too). |
Actually, you know what? I think that the trade-off is just fine to always use OpenSSL's SHA-1 routines in Git for Windows. Nobody works on performance in Git's fall-back code, but tons of people are interested in making OpenSSL's code top notch. |
Use OpenSSL SHA1 routines rather than builtin block-sha1 routines. This improves performance on SHA1 operations on Intel processors. OpenSSL 1.0.2 has made considerable performance improvements and support the Intel hardware acceleration features. See: https://software.intel.com/en-us/articles/improving-openssl-performance https://software.intel.com/en-us/articles/intel-sha-extensions To test this I added/staged a single file in a gigantic repository having a 450MB index file. The code in read-cache.c verifies the header SHA as it reads the index and computes a new header SHA as it writes out the new index. Therefore, in this test the SHA code must process 900MB of data. Testing was done on an Intel I7-4770 CPU @ 3.40GHz (Intel64, Family 6, Model 60) CPU. The block-sha1 version averaged 5.27 seconds. The OpenSSL version averaged 4.50 seconds. ================================================================ $ echo xxx >> project.mk $ time /e/blk_sha/bin/git.exe add project.mk real 0m5.207s user 0m0.000s sys 0m0.250s $ echo xxx >> project.mk $ time /e/blk_sha/bin/git.exe add project.mk real 0m5.362s user 0m0.015s sys 0m0.234s $ echo xxx >> project.mk $ time /e/blk_sha/bin/git.exe add project.mk real 0m5.300s user 0m0.016s sys 0m0.250s $ echo xxx >> project.mk $ time /e/blk_sha/bin/git.exe add project.mk real 0m5.216s user 0m0.000s sys 0m0.250s ================================================================ $ echo xxx >> project.mk $ time /e/openssl/bin/git.exe add project.mk real 0m4.431s user 0m0.000s sys 0m0.250s $ echo xxx >> project.mk $ time /e/openssl/bin/git.exe add project.mk real 0m4.478s user 0m0.000s sys 0m0.265s $ echo xxx >> project.mk $ time /e/openssl/bin/git.exe add project.mk real 0m4.690s user 0m0.000s sys 0m0.250s $ echo xxx >> project.mk $ time /e/openssl/bin/git.exe add project.mk real 0m4.420s user 0m0.000s sys 0m0.234s ================================================================ Signed-off-by: Jeff Hostetler <[email protected]>
80132eb
to
8d16184
Compare
Second version just deleted the config line. Added perf data for simple test. Let me know if the commit message has too much data. Thanks for the quick review on this. |
Perfect! Thank you so much! |
The speed of the SHA-1 calculation was improved by [using OpenSSL's routines](git-for-windows/git#915) which leverages features of current Intel hardware. Signed-off-by: Johannes Schindelin <[email protected]>
…ssl_sha1 sha1: use openssl sha1 routines on mingw
…ssl_sha1 sha1: use openssl sha1 routines on mingw
…ssl_sha1 sha1: use openssl sha1 routines on mingw
…ssl_sha1 sha1: use openssl sha1 routines on mingw
…ssl_sha1 sha1: use openssl sha1 routines on mingw
…ssl_sha1 sha1: use openssl sha1 routines on mingw
…ssl_sha1 sha1: use openssl sha1 routines on mingw
…ssl_sha1 sha1: use openssl sha1 routines on mingw
Note the Intel SHA extensions are very recent, first CPUs implementing them have been introduced in 2016. More relevant, the above benchmark has been done on a pretty good CPU, an i7-4770 (year 2013) which has AVX2 instructions. Thus, benchmarks on older CPUs would be interesting. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thus, benchmarks on older CPUs would be interesting.
Sure. If you have access to older CPUs, it would be interesting to see what your benchmarks say.
@@ -515,7 +515,8 @@ ifneq (,$(findstring MINGW,$(uname_S))) | |||
OBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo | |||
NO_REGEX = YesPlease | |||
NO_PYTHON = YesPlease | |||
BLK_SHA1 = YesPlease | |||
# Omit BLK_SHA1 and use OpenSSL's SHA1 routines for better performance. |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Use OpenSSL SHA1 routines rather than builtin block-sha routines.
This improves performance on SHA1 operations on Intel processors.
Signed-off-by: Jeff Hostetler [email protected]