Re: tr/xdiff-fast-hash generates warnings and breaks tests

[Junio C Hamano <gitster@pobox.com>]

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 ;-)