[PATCH/RFC] make the new block-sha1 the default

[PATCH/RFC] make the new block-sha1 the default

[Nicolas Pitre]

... and remove support for linking against the openssl SHA1 code.

The block-sha1 implementation is not significantly worse and sometimes 
even faster than the openssl SHA1 implementation.  This allows for
getting rid of the dependency and runtime linking to openssl which is
a relatively important source of latency when executing git commands.

Signed-off-by: Nicolas Pitre <nico@cam.org>
---

OK... So here it is.  After all, wanting to get rid of openssl is what 
started it all in the first place.

diff --git a/INSTALL b/INSTALL
index ae7f750..55eb962 100644
--- a/INSTALL
+++ b/INSTALL
@@ -52,13 +52,6 @@ Issues of note:
 
 	- "zlib", the compression library. Git won't build without it.
 
-	- "openssl".  Unless you specify otherwise, you'll get the SHA1
-	  library from here.
-
-	  If you don't have openssl, you can use one of the SHA1 libraries
-	  that come with git (git includes the one from Mozilla, and has
-	  its own PowerPC and ARM optimized ones too - see the Makefile).
-
 	- libcurl library; git-http-fetch and git-fetch use them.  You
 	  might also want the "curl" executable for debugging purposes.
 	  If you do not use http transfer, you are probably OK if you
diff --git a/Makefile b/Makefile
index 4190a5d..8f28b09 100644
--- a/Makefile
+++ b/Makefile
@@ -16,7 +16,6 @@ all::
 # when attempting to read from an fopen'ed directory.
 #
 # Define NO_OPENSSL environment variable if you do not have OpenSSL.
-# This also implies BLK_SHA1.
 #
 # Define NO_CURL if you do not have libcurl installed.  git-http-pull and
 # git-http-push are not built, and you cannot use http:// and https://
@@ -84,10 +83,6 @@ all::
 # specify your own (or DarwinPort's) include directories and
 # library directories by defining CFLAGS and LDFLAGS appropriately.
 #
-# Define BLK_SHA1 environment variable if you want the C version
-# of the SHA1 that assumes you can do unaligned 32-bit loads and
-# have a fast htonl() function.
-#
 # Define PPC_SHA1 environment variable when running make to make use of
 # a bundled SHA1 routine optimized for PowerPC.
 #
@@ -1013,7 +1008,6 @@ ifndef NO_OPENSSL
 	endif
 else
 	BASIC_CFLAGS += -DNO_OPENSSL
-	BLK_SHA1 = 1
 	OPENSSL_LIBSSL =
 endif
 ifdef NEEDS_SSL_WITH_CRYPTO
@@ -1162,18 +1156,14 @@ ifdef NO_DEFLATE_BOUND
 	BASIC_CFLAGS += -DNO_DEFLATE_BOUND
 endif
 
-ifdef BLK_SHA1
-	SHA1_HEADER = "block-sha1/sha1.h"
-	LIB_OBJS += block-sha1/sha1.o
-else
 ifdef PPC_SHA1
 	SHA1_HEADER = "ppc/sha1.h"
 	LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o
 else
-	SHA1_HEADER = <openssl/sha.h>
-	EXTLIBS += $(LIB_4_CRYPTO)
-endif
+	SHA1_HEADER = "block-sha1/sha1.h"
+	LIB_OBJS += block-sha1/sha1.o
 endif
+
 ifdef NO_PERL_MAKEMAKER
 	export NO_PERL_MAKEMAKER
 endif
diff --git a/cache.h b/cache.h
index dd7f71e..39a4edd 100644
--- a/cache.h
+++ b/cache.h
@@ -6,12 +6,6 @@
 #include "hash.h"
 
 #include SHA1_HEADER
-#ifndef git_SHA_CTX
-#define git_SHA_CTX	SHA_CTX
-#define git_SHA1_Init	SHA1_Init
-#define git_SHA1_Update	SHA1_Update
-#define git_SHA1_Final	SHA1_Final
-#endif
 
 #include <zlib.h>
 #if defined(NO_DEFLATE_BOUND) || ZLIB_VERNUM < 0x1200
diff --git a/configure.ac b/configure.ac
index b09b8e4..634186c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -160,7 +160,6 @@ AC_MSG_NOTICE([CHECKS for site configuration])
 # a bundled SHA1 routine optimized for PowerPC.
 #
 # Define NO_OPENSSL environment variable if you do not have OpenSSL.
-# This also implies BLK_SHA1.
 #
 # Define OPENSSLDIR=/foo/bar if your openssl header and library files are in
 # /foo/bar/include and /foo/bar/lib directories.

Re: [PATCH/RFC] make the new block-sha1 the default

[Jeff King]

On Mon, Aug 24, 2009 at 11:04:37PM -0400, Nicolas Pitre wrote:

> ... and remove support for linking against the openssl SHA1 code.
> 
> The block-sha1 implementation is not significantly worse and sometimes 
> even faster than the openssl SHA1 implementation.  This allows for

Is there a reason not to leave the option of linking against openssl?

I'm still getting better numbers for OpenSSL over block-sha1 when doing
"git fsck --full" in some repos. Particularly those with large files and
few deltas, where the time is heavily influenced by sha-1 performance.
I'm seeing up to 20% speed improvement using OpenSSL on those repos, and
about 8% on linux-2.6 (the processor is a Conroe Core 2, git compiled
with -O2).

But what really kills me is that I usually compile git with '-O0'
because I am often investigating bugs and I like the debugger to act
sanely. The performance hit is usually not noticeable, but in this case
it is: my "git fsck --full" times jump from ~8.2s (OpenSSL) and ~10.3s
(block-sha1, -O2) to ~18.2s (block-sha1, -O0).

Certainly you can argue that it is idiotic to benchmark anything at -O0.
But right now, it is perfectly reasonable to compile git with -O0 and
assume OpenSSL is compiled with sane optimizations. I'd rather not take
that away without a good reason.

-Peff

Re: [PATCH/RFC] make the new block-sha1 the default

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Mon, Aug 24, 2009 at 11:04:37PM -0400, Nicolas Pitre wrote:
>
>> ... and remove support for linking against the openssl SHA1 code.
>> 
>> The block-sha1 implementation is not significantly worse and sometimes 
>> even faster than the openssl SHA1 implementation.  This allows for
>
> Is there a reason not to leave the option of linking against openssl?

I think it is a valid question.  Why remove the _option_?

I would certainly understand it if you made BLK_SHA1 the _default_, though.

Re: [PATCH/RFC] make the new block-sha1 the default

[Nicolas Pitre]

On Mon, 24 Aug 2009, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > On Mon, Aug 24, 2009 at 11:04:37PM -0400, Nicolas Pitre wrote:
> >
> >> ... and remove support for linking against the openssl SHA1 code.
> >> 
> >> The block-sha1 implementation is not significantly worse and sometimes 
> >> even faster than the openssl SHA1 implementation.  This allows for
> >
> > Is there a reason not to leave the option of linking against openssl?
> 
> I think it is a valid question.  Why remove the _option_?

Indeed, there is no value in limiting the choice.

> I would certainly understand it if you made BLK_SHA1 the _default_, though.

Since this is a RFC, and because this is not a clear choice, I'll simply 
let others play with it and see for themselves.  Suffice to compile git 
with or without NO_OPENSSL defined.  Some people (such as Jeff) are 
finding the openssl SHA1 faster (irrespective of the -O0 issue), whereas 
Linus simply hammered on the block-sha1 version until it was faster than 
openssl for him (this is faster for me as well, on X86 and ARM).  Also 
those who initially found openssl to put a significant overhead on the 
dynamic linking should probably perform more measurements with and 
without NO_OPENSSL again.  If more positive results are presented then 
changing the default might make sense.


Nicolas