git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Thomas Rast <trast@student.ethz.ch>
To: Junio C Hamano <gitster@pobox.com>
Cc: Brian Gernhardt <brian@gernhardtsoftware.com>,
	<git@vger.kernel.org>, Thomas Rast <trast@student.ethz.ch>
Subject: [PATCH] Choose XDL_FAST_HASH code on sizeof(long) instead of __WORDSIZE
Date: Tue, 1 May 2012 10:36:51 +0200	[thread overview]
Message-ID: <399c9a1f0dd548da3144eff3c50d04a8f343ab24.1335860033.git.trast@student.ethz.ch> (raw)
In-Reply-To: <7v4ns0cy3l.fsf@alter.siamese.dyndns.org>

Darwin does not define __WORDSIZE, and compiles the 32-bit code path
on 64-bit systems, resulting in a totally broken git.

I could not find an alternative -- other than the platform symbols
(__x86_64__ etc.) -- that does the test in the preprocessor.  However,
we can also just test for the size of a 'long', which is what really
matters here.  Any compiler worth its salt will leave only the branch
relevant for its platform, and indeed on Linux/GCC the numbers don't
change:

 Test                                  tr/darwin-xdl-fast-hash   origin/next              origin/master
 ------------------------------------------------------------------------------------------------------------------
 4000.1: log -3000 (baseline)          0.09(0.07+0.01)           0.09(0.07+0.01) -5.5%*   0.09(0.07+0.01) -4.1%
 4000.2: log --raw -3000 (tree-only)   0.47(0.41+0.05)           0.47(0.40+0.05) -0.5%    0.45(0.38+0.06) -3.5%.
 4000.3: log -p -3000 (Myers)          1.81(1.67+0.12)           1.81(1.67+0.13) +0.3%    1.99(1.84+0.12) +10.2%***
 4000.4: log -p -3000 --histogram      1.79(1.66+0.11)           1.80(1.67+0.11) +0.4%    1.96(1.82+0.10) +9.2%***
 4000.5: log -p -3000 --patience       2.17(2.02+0.13)           2.20(2.04+0.13) +1.3%.   2.33(2.18+0.13) +7.4%***
 ------------------------------------------------------------------------------------------------------------------
 Significance hints:  '.' 0.1  '*' 0.05  '**' 0.01  '***' 0.001

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

Incidentally, on the OS X machine I have access to, the difference is
even bigger, something like 14% for 'log -p -3000'.  I'm too lazy to
work out the exact numbers since the perf code doesn't work there.

 xdiff/xutils.c |   52 +++++++++++++++++++++++-----------------------------
 1 file changed, 23 insertions(+), 29 deletions(-)

diff --git a/xdiff/xutils.c b/xdiff/xutils.c
index e05b5c9..1b3b471 100644
--- a/xdiff/xutils.c
+++ b/xdiff/xutils.c
@@ -290,39 +290,33 @@ static inline unsigned long has_zero(unsigned long a)
 	return ((a - ONEBYTES) & ~a) & HIGHBITS;
 }
 
-#if __WORDSIZE == 64
-
-/*
- * Jan Achrenius on G+: microoptimized version of
- * the simpler "(mask & ONEBYTES) * ONEBYTES >> 56"
- * that works for the bytemasks without having to
- * mask them first.
- */
 static inline long count_masked_bytes(unsigned long mask)
 {
-	return mask * 0x0001020304050608 >> 56;
-}
-
-#else	/* 32-bit case */
-
-/* Modified Carl Chatfield G+ version for 32-bit */
-static inline long count_masked_bytes(long mask)
-{
-	/*
-	 * (a) gives us
-	 *   -1 (0, ff), 0 (ffff) or 1 (ffffff)
-	 * (b) gives us
-	 *   0 for 0, 1 for (ff ffff ffffff)
-	 * (a+b+1) gives us
-	 *   correct 0-3 bytemask count result
-	 */
-	long a = (mask - 256) >> 23;
-	long b = mask & 1;
-	return a + b + 1;
+	if (sizeof(long) == 8) {
+		/*
+		 * Jan Achrenius on G+: microoptimized version of
+		 * the simpler "(mask & ONEBYTES) * ONEBYTES >> 56"
+		 * that works for the bytemasks without having to
+		 * mask them first.
+		 */
+		return mask * 0x0001020304050608 >> 56;
+	} else {
+		/*
+		 * Modified Carl Chatfield G+ version for 32-bit *
+		 *
+		 * (a) gives us
+		 *   -1 (0, ff), 0 (ffff) or 1 (ffffff)
+		 * (b) gives us
+		 *   0 for 0, 1 for (ff ffff ffffff)
+		 * (a+b+1) gives us
+		 *   correct 0-3 bytemask count result
+		 */
+		long a = (mask - 256) >> 23;
+		long b = mask & 1;
+		return a + b + 1;
+	}
 }
 
-#endif
-
 unsigned long xdl_hash_record(char const **data, char const *top, long flags)
 {
 	unsigned long hash = 5381;
-- 
1.7.10.512.g5bfc6

  reply	other threads:[~2012-05-01  8:37 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-29 20:52 XDL_FAST_HASH breaks git on OS X 10.7.3 Brian Gernhardt
2012-04-30 16:38 ` Thomas Rast
2012-05-01  2:13   ` Brian Gernhardt
2012-05-01  4:39     ` Junio C Hamano
2012-05-01  4:41       ` Junio C Hamano
2012-05-01  8:36         ` Thomas Rast [this message]
2012-05-01 17:33           ` [PATCH] Choose XDL_FAST_HASH code on sizeof(long) instead of __WORDSIZE Junio C Hamano
2012-05-01  5:32   ` XDL_FAST_HASH breaks git on OS X 10.7.3 David Aguilar
2012-05-01  6:17     ` Junio C Hamano

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=399c9a1f0dd548da3144eff3c50d04a8f343ab24.1335860033.git.trast@student.ethz.ch \
    --to=trast@student.ethz.ch \
    --cc=brian@gernhardtsoftware.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).