All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Nicolas Pitre <nico@cam.org>
Cc: Junio C Hamano <gitster@pobox.com>,
	Sebastian Schuberth <sschuberth@gmail.com>,
	Artur Skawina <art.08.09@gmail.com>,
	Johannes Sixt <j.sixt@viscovery.net>,
	msysGit <msysgit@googlegroups.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH] block-sha1: Windows declares ntohl() in winsock2.h
Date: Tue, 18 Aug 2009 13:17:57 -0700	[thread overview]
Message-ID: <7vprasc1vu.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: alpine.LFD.2.00.0908181411320.6044@xanadu.home

Nicolas Pitre <nico@cam.org> writes:

> On Tue, 18 Aug 2009, Junio C Hamano wrote:
>
>> To reduce confusion, you may want to rename compat/bswap.h to something
>> like compat/ntohl-htonl-fix.h ;-)
>
> Bah.  If you wish, you can edit the patch directly for this, unless you 
> really prefer me to repost.  Maybe we might want to add a 8-byte 
> versions of those as well eventually, which is why I chose a more 
> generic name.

Ok, here is what I came up with after many squashing...

-- >8 --
From: Nicolas Pitre <nico@cam.org>
Date: Tue, 18 Aug 2009 12:43:08 -0400
Subject: [PATCH] bswap: avoid potentially inefficient ntohl/htonl on i386/x86-64

Johannes Sixt reports that on Windows ntohl()/htonl() are not found in
<arpa/inet.h>, and as a minimal fix we need to include <winsock2.h>
instead.  Sebastian Schuberth points out that they are implemented as
out-of-line functions on Windows, which defeats these byteorder "macros"
used for performance.

Use bswap instruction through gcc inline asm instead on i386/x86-64
as a generic solution to this.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Nicolas Pitre <nico@cam.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 block-sha1/sha1.c |    4 +---
 compat/bswap.h    |   19 +++++++++++++++++++
 git-compat-util.h |    2 ++
 3 files changed, 22 insertions(+), 3 deletions(-)
 create mode 100644 compat/bswap.h

diff --git a/block-sha1/sha1.c b/block-sha1/sha1.c
index 464cb25..51a27c1 100644
--- a/block-sha1/sha1.c
+++ b/block-sha1/sha1.c
@@ -4,9 +4,7 @@
  * and to avoid unnecessary copies into the context array.
  */
 
-#include <string.h>
-#include <arpa/inet.h>
-
+#include "../git-compat-util.h"
 #include "sha1.h"
 
 #if defined(__i386__) || defined(__x86_64__)
diff --git a/compat/bswap.h b/compat/bswap.h
new file mode 100644
index 0000000..78fd2df
--- /dev/null
+++ b/compat/bswap.h
@@ -0,0 +1,19 @@
+/*
+ * Let's make sure we always have a sane definition for ntohl()/htonl().
+ * Some libraries define those as a function call, just to perform byte
+ * swapping, bringing significant overhead to what should be a simple
+ * operation.
+ */
+
+#if defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))
+
+#define bswap32(x) ({ \
+	unsigned int __res; \
+	__asm__("bswap %0" : "=r" (__res) : "0" (x)); \
+	__res; })
+#undef ntohl
+#undef htonl
+#define ntohl(x) bswap32(x)
+#define htonl(x) bswap32(x)
+
+#endif
diff --git a/git-compat-util.h b/git-compat-util.h
index 9f941e4..000859e 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -176,6 +176,8 @@ extern char *gitbasename(char *);
 #endif
 #endif
 
+#include "compat/bswap.h"
+
 /* General helper functions */
 extern void usage(const char *err) NORETURN;
 extern void die(const char *err, ...) NORETURN __attribute__((format (printf, 1, 2)));
-- 
1.6.4.245.g50659

  reply	other threads:[~2009-08-18 20:18 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-08-18  7:15 [PATCH] block-sha1: Windows declares ntohl() in winsock2.h Johannes Sixt
2009-08-18 10:45 ` Sebastian Schuberth
2009-08-18 11:07   ` Junio C Hamano
2009-08-18 11:28     ` Sebastian Schuberth
2009-08-18 12:56   ` Artur Skawina
2009-08-18 13:17     ` Sebastian Schuberth
2009-08-18 13:39       ` Junio C Hamano
2009-08-18 15:40         ` Linus Torvalds
2009-08-18 16:08           ` Linus Torvalds
2009-08-18 16:23             ` Sebastian Schuberth
2009-08-18 16:43               ` Junio C Hamano
2009-08-18 16:30         ` Nicolas Pitre
2009-08-18 16:43           ` Nicolas Pitre
2009-08-18 16:50             ` Junio C Hamano
2009-08-18 18:01               ` Nicolas Pitre
2009-08-18 19:00                 ` Junio C Hamano
2009-08-18 19:22                   ` Nicolas Pitre
2009-08-18 19:26                     ` [PATCH] make sure byte swapping is optimal for git Nicolas Pitre
2009-08-18 19:37                     ` [PATCH] block-sha1: guard gcc extensions with __GNUC__ Nicolas Pitre
2009-08-18 19:40                   ` [PATCH] block-sha1: Windows declares ntohl() in winsock2.h Junio C Hamano
2009-08-18 19:56                   ` Brandon Casey
2009-08-18 20:10                     ` Nicolas Pitre
2009-08-18 20:25                       ` Linus Torvalds
2009-08-18 20:29                       ` Brandon Casey
2009-08-20  2:26                     ` Nicolas Pitre
2009-08-20  2:30                       ` Linus Torvalds
2009-08-20  2:45                       ` Brandon Casey
2009-08-18 16:59             ` Sebastian Schuberth
2009-08-18 17:05               ` Junio C Hamano
2009-08-18 18:16                 ` Nicolas Pitre
2009-08-18 20:17                   ` Junio C Hamano [this message]
2009-08-18 20:30                     ` Junio C Hamano
2009-08-18 20:49                       ` Nicolas Pitre
2009-08-18 18:10               ` Nicolas Pitre

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7vprasc1vu.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=art.08.09@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=j.sixt@viscovery.net \
    --cc=msysgit@googlegroups.com \
    --cc=nico@cam.org \
    --cc=sschuberth@gmail.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.