{"thread":{"id":"20725","subject":"[PATCH/RFC] make the new block-sha1 the default","startedAt":"2009-08-25T03:04:37Z","lastAt":"2009-08-25T17:33:22Z","messageCount":4,"participants":["Nicolas Pitre","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"121699","messageId":"alpine.LFD.2.00.0908242249420.6044@xanadu.home","threadId":"20725","inReplyTo":null,"subject":"[PATCH/RFC] make the new block-sha1 the default","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2009-08-25T03:04:37Z","receivedAt":"2009-08-25T03:04:37Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"... and remove support for linking against the openssl SHA1 code.\n\nThe block-sha1 implementation is not significantly worse and sometimes \neven faster than the openssl SHA1 implementation.  This allows for\ngetting rid of the dependency and runtime linking to openssl which is\na relatively important source of latency when executing git commands.\n\nSigned-off-by: Nicolas Pitre <nico@cam.org>\n---\n\nOK... So here it is.  After all, wanting to get rid of openssl is what \nstarted it all in the first place.\n\ndiff --git a/INSTALL b/INSTALL\nindex ae7f750..55eb962 100644\n--- a/INSTALL\n+++ b/INSTALL\n@@ -52,13 +52,6 @@ Issues of note:\n \n \t- \"zlib\", the compression library. Git won't build without it.\n \n-\t- \"openssl\".  Unless you specify otherwise, you'll get the SHA1\n-\t  library from here.\n-\n-\t  If you don't have openssl, you can use one of the SHA1 libraries\n-\t  that come with git (git includes the one from Mozilla, and has\n-\t  its own PowerPC and ARM optimized ones too - see the Makefile).\n-\n \t- libcurl library; git-http-fetch and git-fetch use them.  You\n \t  might also want the \"curl\" executable for debugging purposes.\n \t  If you do not use http transfer, you are probably OK if you\ndiff --git a/Makefile b/Makefile\nindex 4190a5d..8f28b09 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -16,7 +16,6 @@ all::\n # when attempting to read from an fopen'ed directory.\n #\n # Define NO_OPENSSL environment variable if you do not have OpenSSL.\n-# This also implies BLK_SHA1.\n #\n # Define NO_CURL if you do not have libcurl installed.  git-http-pull and\n # git-http-push are not built, and you cannot use http:// and https://\n@@ -84,10 +83,6 @@ all::\n # specify your own (or DarwinPort's) include directories and\n # library directories by defining CFLAGS and LDFLAGS appropriately.\n #\n-# Define BLK_SHA1 environment variable if you want the C version\n-# of the SHA1 that assumes you can do unaligned 32-bit loads and\n-# have a fast htonl() function.\n-#\n # Define PPC_SHA1 environment variable when running make to make use of\n # a bundled SHA1 routine optimized for PowerPC.\n #\n@@ -1013,7 +1008,6 @@ ifndef NO_OPENSSL\n \tendif\n else\n \tBASIC_CFLAGS += -DNO_OPENSSL\n-\tBLK_SHA1 = 1\n \tOPENSSL_LIBSSL =\n endif\n ifdef NEEDS_SSL_WITH_CRYPTO\n@@ -1162,18 +1156,14 @@ ifdef NO_DEFLATE_BOUND\n \tBASIC_CFLAGS += -DNO_DEFLATE_BOUND\n endif\n \n-ifdef BLK_SHA1\n-\tSHA1_HEADER = \"block-sha1/sha1.h\"\n-\tLIB_OBJS += block-sha1/sha1.o\n-else\n ifdef PPC_SHA1\n \tSHA1_HEADER = \"ppc/sha1.h\"\n \tLIB_OBJS += ppc/sha1.o ppc/sha1ppc.o\n else\n-\tSHA1_HEADER = <openssl/sha.h>\n-\tEXTLIBS += $(LIB_4_CRYPTO)\n-endif\n+\tSHA1_HEADER = \"block-sha1/sha1.h\"\n+\tLIB_OBJS += block-sha1/sha1.o\n endif\n+\n ifdef NO_PERL_MAKEMAKER\n \texport NO_PERL_MAKEMAKER\n endif\ndiff --git a/cache.h b/cache.h\nindex dd7f71e..39a4edd 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -6,12 +6,6 @@\n #include \"hash.h\"\n \n #include SHA1_HEADER\n-#ifndef git_SHA_CTX\n-#define git_SHA_CTX\tSHA_CTX\n-#define git_SHA1_Init\tSHA1_Init\n-#define git_SHA1_Update\tSHA1_Update\n-#define git_SHA1_Final\tSHA1_Final\n-#endif\n \n #include <zlib.h>\n #if defined(NO_DEFLATE_BOUND) || ZLIB_VERNUM < 0x1200\ndiff --git a/configure.ac b/configure.ac\nindex b09b8e4..634186c 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -160,7 +160,6 @@ AC_MSG_NOTICE([CHECKS for site configuration])\n # a bundled SHA1 routine optimized for PowerPC.\n #\n # Define NO_OPENSSL environment variable if you do not have OpenSSL.\n-# This also implies BLK_SHA1.\n #\n # Define OPENSSLDIR=/foo/bar if your openssl header and library files are in\n # /foo/bar/include and /foo/bar/lib directories.\n"},{"id":"121700","messageId":"20090825041859.GA10033@coredump.intra.peff.net","threadId":"20725","inReplyTo":"alpine.LFD.2.00.0908242249420.6044@xanadu.home","subject":"Re: [PATCH/RFC] make the new block-sha1 the default","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-25T04:18:59Z","receivedAt":"2009-08-25T04:18:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 24, 2009 at 11:04:37PM -0400, Nicolas Pitre wrote:\n\n> ... and remove support for linking against the openssl SHA1 code.\n> \n> The block-sha1 implementation is not significantly worse and sometimes \n> even faster than the openssl SHA1 implementation.  This allows for\n\nIs there a reason not to leave the option of linking against openssl?\n\nI'm still getting better numbers for OpenSSL over block-sha1 when doing\n\"git fsck --full\" in some repos. Particularly those with large files and\nfew deltas, where the time is heavily influenced by sha-1 performance.\nI'm seeing up to 20% speed improvement using OpenSSL on those repos, and\nabout 8% on linux-2.6 (the processor is a Conroe Core 2, git compiled\nwith -O2).\n\nBut what really kills me is that I usually compile git with '-O0'\nbecause I am often investigating bugs and I like the debugger to act\nsanely. The performance hit is usually not noticeable, but in this case\nit is: my \"git fsck --full\" times jump from ~8.2s (OpenSSL) and ~10.3s\n(block-sha1, -O2) to ~18.2s (block-sha1, -O0).\n\nCertainly you can argue that it is idiotic to benchmark anything at -O0.\nBut right now, it is perfectly reasonable to compile git with -O0 and\nassume OpenSSL is compiled with sane optimizations. I'd rather not take\nthat away without a good reason.\n\n-Peff\n"},{"id":"121716","messageId":"7vprakpett.fsf@alter.siamese.dyndns.org","threadId":"20725","inReplyTo":"20090825041859.GA10033@coredump.intra.peff.net","subject":"Re: [PATCH/RFC] make the new block-sha1 the default","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-25T06:50:22Z","receivedAt":"2009-08-25T06:50:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Mon, Aug 24, 2009 at 11:04:37PM -0400, Nicolas Pitre wrote:\n>\n>> ... and remove support for linking against the openssl SHA1 code.\n>> \n>> The block-sha1 implementation is not significantly worse and sometimes \n>> even faster than the openssl SHA1 implementation.  This allows for\n>\n> Is there a reason not to leave the option of linking against openssl?\n\nI think it is a valid question.  Why remove the _option_?\n\nI would certainly understand it if you made BLK_SHA1 the _default_, though.\n"},{"id":"121736","messageId":"alpine.LFD.2.00.0908251323280.6044@xanadu.home","threadId":"20725","inReplyTo":"7vprakpett.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] make the new block-sha1 the default","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2009-08-25T17:33:22Z","receivedAt":"2009-08-25T17:33:22Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Mon, 24 Aug 2009, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > On Mon, Aug 24, 2009 at 11:04:37PM -0400, Nicolas Pitre wrote:\n> >\n> >> ... and remove support for linking against the openssl SHA1 code.\n> >> \n> >> The block-sha1 implementation is not significantly worse and sometimes \n> >> even faster than the openssl SHA1 implementation.  This allows for\n> >\n> > Is there a reason not to leave the option of linking against openssl?\n> \n> I think it is a valid question.  Why remove the _option_?\n\nIndeed, there is no value in limiting the choice.\n\n> I would certainly understand it if you made BLK_SHA1 the _default_, though.\n\nSince this is a RFC, and because this is not a clear choice, I'll simply \nlet others play with it and see for themselves.  Suffice to compile git \nwith or without NO_OPENSSL defined.  Some people (such as Jeff) are \nfinding the openssl SHA1 faster (irrespective of the -O0 issue), whereas \nLinus simply hammered on the block-sha1 version until it was faster than \nopenssl for him (this is faster for me as well, on X86 and ARM).  Also \nthose who initially found openssl to put a significant overhead on the \ndynamic linking should probably perform more measurements with and \nwithout NO_OPENSSL again.  If more positive results are presented then \nchanging the default might make sense.\n\n\nNicolas\n"}]}