Re: [PATCH] Put sha1dc on a diet

From: Jeff King <peff@peff.net>
Date: Mar 13, 2017, 15:13 UTC
Message-ID: <20170313151322.ouryghyb5orkpk5g@sigill.intra.peff.net>
In-Reply-To: <CY1PR0301MB2107112BCC2DECD215E70549C42A0@CY1PR0301MB2107.namprd03.prod.outlook.com>

On Sat, Mar 04, 2017 at 01:07:16AM +0000, Dan Shumow wrote:

Show 6 quoted lines

> You are very welcome.  Thank you for the warm welcome.  As it turns
> out, Marc and I are working on the simplifications / removal of c99
> and performance upstream in our GitHub repo.  I am happy to help for
> any GitHub specific customizations that are needed as well.  But for
> now, lets see if we can get you everything you want upstream -- I
> think that's the most simple.

I've been watching the repo at:

  https://github.com/cr-marcstevens/sha1collisiondetection

The work on the feature/performance branch seems to be producing good results. The best timings I got show sha1dc (with checks enabled) at 1.75x block-sha1, which is pretty good. That was using your ad744c8b7a841d2afcb2d4c04f8952d9005501be.

Curiously, the performance gets worse after that. Even more curious, the bad performance bisects to a merge, and it performs worse than either side of the merge.

Try this:

  # mine is a 1.2GB linux packfile, but anything big should do
  file=/some/large/file

  # the merge with the funny behavior
  merge=55d1db0980501e582f6cd103a04f493995b1df78

  for i in $merge^ $merge^2 $merge; do
    git checkout $i &&
    rm -f bin/* &&
    make &&
    time bin/sha1dcsum $file
  done

I get:

  [$merge^, the feature/performance branch before the merge]
  real	0m3.391s
  user	0m3.304s
  sys	0m0.084s

  [$merge^2, the master branch before the merge]
  real	0m5.272s
  user	0m5.164s
  sys	0m0.096s

  [$merge, the merge of the two]
  real	0m7.038s
  user	0m6.924s
  sys	0m0.104s

So that's odd. Looking at the diff, I don't see anything that obviously jumps out as a mis-merge.

Feel free to tell me "stop looking at that branch; it's a work in progress". But I think the results from $merge^ (ad744c8b7) are getting good enough to consider moving forward with integrating it into git.

-Peff