Skip to content
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

Merged

Conversation

jeffhostetler
Copy link

Use OpenSSL SHA1 routines rather than builtin block-sha routines.
This improves performance on SHA1 operations on Intel processors.

Signed-off-by: Jeff Hostetler jeffhost@microsoft.com

@dscho
Copy link
Member

dscho commented Oct 12, 2016

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).

@dscho
Copy link
Member

dscho commented Oct 12, 2016

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 <jeffhost@microsoft.com>
@jeffhostetler jeffhostetler force-pushed the jeffhostetler/g4w_use_openssl_sha1 branch from 80132eb to 8d16184 Compare October 12, 2016 18:10
@jeffhostetler
Copy link
Author

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.

@dscho
Copy link
Member

dscho commented Oct 13, 2016

Perfect! Thank you so much!

@dscho dscho merged commit 8dbd49b into git-for-windows:master Oct 13, 2016
dscho added a commit to git-for-windows/build-extra that referenced this pull request Oct 13, 2016
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 <johannes.schindelin@gmx.de>
dscho added a commit that referenced this pull request Oct 13, 2016
…ssl_sha1

sha1: use openssl sha1 routines on mingw
@jeffhostetler jeffhostetler deleted the jeffhostetler/g4w_use_openssl_sha1 branch October 13, 2016 13:27
dscho pushed a commit that referenced this pull request Oct 13, 2016
…ssl_sha1

sha1: use openssl sha1 routines on mingw
dscho pushed a commit that referenced this pull request Oct 13, 2016
…ssl_sha1

sha1: use openssl sha1 routines on mingw
dscho added a commit that referenced this pull request Oct 15, 2016
…ssl_sha1

sha1: use openssl sha1 routines on mingw
dscho pushed a commit that referenced this pull request Oct 18, 2016
…ssl_sha1

sha1: use openssl sha1 routines on mingw
dscho pushed a commit that referenced this pull request Oct 18, 2016
…ssl_sha1

sha1: use openssl sha1 routines on mingw
dscho added a commit that referenced this pull request Oct 18, 2016
…ssl_sha1

sha1: use openssl sha1 routines on mingw
dscho pushed a commit that referenced this pull request Oct 20, 2016
…ssl_sha1

sha1: use openssl sha1 routines on mingw
@vlakoff
Copy link

vlakoff commented Nov 24, 2016

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.

Copy link
Member

@dscho dscho left a 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants