From: Junio C Hamano <gitster@pobox•com>
To: "René Scharfe" <rene.scharfe@lsrfire•ath.cx>
Cc: "Øyvind A. Holm" <sunny@sunbase•org>,
git@vger•kernel.org, "Thomas Rast" <trast@student•ethz.ch>,
"Junio C Hamano" <gitster@pobox•com>
Subject: Re: tr/xdiff-fast-hash generates warnings and breaks tests
Date: Thu, 17 May 2012 09:23:40 -0700 [thread overview]
Message-ID: <xmqqmx56rd2r.fsf@junio.mtv.corp.google.com> (raw)
In-Reply-To: <4FB4A4B9.3080009@lsrfire.ath.cx> ("René Scharfe"'s message of "Thu, 17 May 2012 09:11:53 +0200")
René Scharfe <rene.scharfe@lsrfire•ath.cx> writes:
> Am 17.05.2012 01:31, schrieb Øyvind A. Holm:
>> On Debian GNU/Linux 6.0.5 (squeeze), the two commits on the
>> tr/xdiff-fast-hash branch introduces compiler warnings and breaks
>> t/t0020-crlf.sh and maybe later tests:
>
> What does the following short C program report when run (e.g. put it
> in a file named s.c, then run "gcc -o s s.c" and "./s")?
>
> #include <stdio.h>
> int main(int argc, const char **argv) {
> printf("%u %u %u\n", sizeof(int), sizeof(long), sizeof(void *));
> return 0;
> }
>
> I suspect you run a 32-bit userland on a 64-bit kernel.
>
> On Ubuntu 12.04 x86, t0020 fails for me as well when I compile with
> XDL_FAST_HASH explicitly set (it's off by default).
OK. So does that indicate at least breakage in the Makefile that
attempts to set XDL_FAST_HASH only on x86_64, mistakenly triggering
on Øyvind's x86 32-bit userland, or did Øyvind manually flipped the
feature on?
It is a separate issue that XDL_FAST_HASH code does not work on 32-bit
systems, even though it is advertised that it only needs to be on
little-endian.
> It succeeds after reverting 6f1af02, though, strangely enough.
It is strange. I do not see anything glaringly wrong in the conversion
in that commit. The only difference I see is that count_masked_bytes in
the original used to take unsigned long on 64-bit archs but the updated
one takes signed long, but that on 32-bit archs the function takes
signed long in both versions so it cannot be it. Stumped...
> Also, here are the measurements for master (v1.7.10.2-520-g6a4a482)
> without XDL_FAST_HASH, and with master minus 6f1af02 plus explicitly
> set XDL_FAST_HASH:
>
> Test master reverted+FAST
> ---------------------------------------------------------------------
> 4000.1: log -3000 (baseline) 0.08(0.05+0.02) 0.08(0.05+0.02)
> 4000.2: log --raw -3000 (tree-only) 0.39(0.34+0.04) 0.39(0.32+0.06)
> 4000.3: log -p -3000 (Myers) 1.55(1.43+0.11) 1.43(1.29+0.12)
> 4000.4: log -p -3000 --histogram 1.63(1.51+0.10) 1.50(1.35+0.14)
> 4000.5: log -p -3000 --patience 1.85(1.71+0.13) 1.73(1.62+0.10)
So the patch does give us slight performance edge, when it works ;-)
next prev parent reply other threads:[~2012-05-17 16:23 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-05-16 23:31 tr/xdiff-fast-hash generates warnings and breaks tests Øyvind A. Holm
2012-05-17 7:11 ` René Scharfe
2012-05-17 9:33 ` Øyvind A. Holm
2012-05-17 16:23 ` Junio C Hamano [this message]
2012-05-17 18:40 ` René Scharfe
2012-05-19 14:17 ` Øyvind A. Holm
2012-05-22 20:36 ` [PATCH 1/3] xdiff: avoid compiler warnings with XDL_FAST_HASH on 32-bit machines René Scharfe
2012-05-22 20:36 ` [PATCH 2/3] xdiff: avoid more " René Scharfe
2012-05-23 8:30 ` Thomas Rast
2012-05-22 20:36 ` [PATCH 3/3] xdiff: import new 32-bit version of count_masked_bytes() René Scharfe
2012-05-25 15:18 ` Øyvind A. Holm
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=xmqqmx56rd2r.fsf@junio.mtv.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=rene.scharfe@lsrfire$(echo .)ath.cx \
--cc=sunny@sunbase$(echo .)org \
--cc=trast@student$(echo .)ethz.ch \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox