RE: [PATCH] Put sha1dc on a diet

.nl> Cc: Linus Torvalds <torva...@linux-foundation.org>; Dan Shumow <dan...@microsoft.com>; Junio C Hamano <gits...@pobox.com>; Git Mailing List <git@vger.kernel.org> Subject: Re: [PATCH] Put sha1dc on a diet On Thu, Mar 16, 2017 at 07:22:14PM +0100, Marc Stevens wro

Re: [PATCH] Put sha1dc on a diet

On Thu, Mar 16, 2017 at 07:22:14PM +0100, Marc Stevens wrote: > Today I merged the perf-branch into master after code review and correctness > testing. > So master is now more performant and safe to use. Great, thank you (and Dan) so much for all your work. We're looking at integrating this

Re: [PATCH] Put sha1dc on a diet

Hi all, Today I merged the perf-branch into master after code review and correctness testing. So master is now more performant and safe to use. -- Marc

Re: [PATCH] Put sha1dc on a diet

> Cc: "Linus Torvalds" <torva...@linux-foundation.org>, "Dan Shumow" <dan...@microsoft.com>, "Junio C Hamano" <gits...@pobox.com>, "Git Mailing List" <git@vger.kernel.org> Sent: Monday, March 13, 2017 10:00:23 PM Subject: Re: [PAT

Re: [PATCH] Put sha1dc on a diet

On Mon, Mar 13, 2017 at 09:47:54PM +0100, Marc Stevens wrote: > Linus: > I would be surprised, the dependencies should be automatically determined. > > BTW Did you make local changes to this perf branch? I can reproduce it with: cd sha1collisiondetection git clean -dqfx ;# make sure we are

Re: [PATCH] Put sha1dc on a diet

o" <gits...@pobox.com>, "Git Mailing List" <git@vger.kernel.org> Sent: Monday, March 13, 2017 9:20:02 PM Subject: Re: [PATCH] Put sha1dc on a diet On Mon, Mar 13, 2017 at 1:12 PM, Marc Stevens <marc.stev...@cwi.nl> wrote: > Indeed, I've committed a fix, and a small bug

Re: [PATCH] Put sha1dc on a diet

On Mon, Mar 13, 2017 at 1:12 PM, Marc Stevens wrote: > Indeed, I've committed a fix, and a small bug fix for the new code just now. Unrelated side note: there may be some missing dependencies in the build infrastructure or something, because when I tried Jeff's script that

Re: [PATCH] Put sha1dc on a diet

Indeed, I've committed a fix, and a small bug fix for the new code just now. The merge incorrectly removed some control logic, which caused more unnecessary checks to happen. I already marked this in the PR, but committed a fix only today. BTW as noted in the Readme, the theoretic false positive

Re: [PATCH] Put sha1dc on a diet

On Mon, Mar 13, 2017 at 07:42:17PM +, Dan Shumow wrote: > Marc just made a commit this morning fixing problems with the merge. > Please give the latest in feature/performance a try, as that seems to > eliminate the problem. Yeah, b17728507 makes the problem go away for me. Thanks. FWIW, I

Re: [PATCH] Put sha1dc on a diet

On Sat, Mar 04, 2017 at 01:07:16AM +, Dan Shumow wrote: > 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

RE: [PATCH] Put sha1dc on a diet

From: Junio C Hamano [mailto:gits...@pobox.com] > As you and Marc seemed to be still working on speeding up, such a > customization work to fully adjust your code to our codebase was premature, > so I tentatively queued what we saw on the list as-is on our 'pu' branch so > that people can

Re: [PATCH] Put sha1dc on a diet

On Thu, Mar 2, 2017 at 10:37 AM, Jeff Hostetler wrote: >> >> Now, if your _file_ index is 300-400MB (and I do think we check the >> SHA fingerprint on that even on just reading it - verify_hdr() in >> do_read_index()), then that's going to be a somewhat noticeable hit on

Re: [PATCH] Put sha1dc on a diet

On 3/2/2017 11:35 AM, Linus Torvalds wrote: On Thu, Mar 2, 2017 at 6:45 AM, Johannes Schindelin wrote: It would probably make sense to switch the index integrity check away from SHA-1 because we really only care about detecting bit flips there, and we have no need

Re: [PATCH] Put sha1dc on a diet

On Thu, Mar 2, 2017 at 6:45 AM, Johannes Schindelin wrote: > > It would probably make sense to switch the index integrity check away from > SHA-1 because we really only care about detecting bit flips there, and we > have no need for the computational overhead of using

Re: [PATCH] Put sha1dc on a diet

Hi Duy, On Thu, 2 Mar 2017, Duy Nguyen wrote: > On Thu, Mar 2, 2017 at 6:19 AM, Jeff King wrote: > > You have to remember that some of the Git for Windows users are doing > > horrific things like using repositories with 450MB .git/index files, > > and the speed to compute the

Re: [PATCH] Put sha1dc on a diet

Hi Linus, On Wed, 1 Mar 2017, Linus Torvalds wrote: > On Wed, Mar 1, 2017 at 2:51 PM, Johannes Schindelin > wrote: > > > > But I think bigger than just developers on Windows OS. There are many > > developers out there working on large repositories (yes, much larger >

Re: [PATCH] Put sha1dc on a diet

Hi Peff, On Wed, 1 Mar 2017, Jeff King wrote: > I do think that could argue for turning on the collision detection only > during object-write operations, which is where it matters. It would be > really trivial to flip the "check collisions" bit on sha1dc. But I > suspect you could go faster

Re: [PATCH] Put sha1dc on a diet

On Wed, Mar 1, 2017 at 11:07 AM, Jeff King wrote: > > So obviously the smaller object size is nice, and the diffstat is > certainly satisfying. My only qualm would be whether this conflicts with > the optimizations that Dan is working on (probably not conceptually, but >

Re: [PATCH] Put sha1dc on a diet

On Thu, Mar 2, 2017 at 6:19 AM, Jeff King wrote: > You have to remember that some of the Git for Windows users are doing > horrific things like using repositories with 450MB .git/index files, and > the speed to compute the sha1 during an update is noticeable there. We probably

Re: [PATCH] Put sha1dc on a diet

Dan Shumow writes: > At this point, I would suggest that I take the C optimizations, > clean them up and fold them in with the diet changes Linus has > suggested. The slowdown is still 2x over block-sha1 and more over > OpenSSL. But it is better than nothing. And then if

RE: [PATCH] Put sha1dc on a diet

I played around tweaking the code a bit more and I got our performance down to a 2.077182x slowdown with check and a 1.055961x slowdown without checking. However, that slowdown is basically with the check turned off through our API. If I rip extraneous code for storing states and checking if

Re: [PATCH] Put sha1dc on a diet

On Wed, Mar 1, 2017 at 12:34 PM, Jeff King wrote: > > I don't think that helps. The sha1 over the pack-file takes about 1.3s > with openssl, and 5s with sha1dc. So we already know the increase there > is only a few seconds, not a few minutes. Yeah, I did a few statistics by adding

Re: [PATCH] Put sha1dc on a diet

On Wed, Mar 01, 2017 at 03:05:25PM -0800, Linus Torvalds wrote: > On Wed, Mar 1, 2017 at 2:51 PM, Johannes Schindelin > wrote: > > > > But I think bigger than just developers on Windows OS. There are many > > developers out there working on large repositories (yes,

Re: [PATCH] Put sha1dc on a diet

On Wed, Mar 01, 2017 at 12:58:39PM -0800, Linus Torvalds wrote: >> I don't think that helps. The sha1 over the pack-file takes about 1.3s >> with openssl, and 5s with sha1dc. So we already know the increase there >> is only a few seconds, not a few minutes. > > OK. I guess what w could easily do

Re: [PATCH] Put sha1dc on a diet

On Wed, Mar 1, 2017 at 2:51 PM, Johannes Schindelin wrote: > > But I think bigger than just developers on Windows OS. There are many > developers out there working on large repositories (yes, much larger than > Linux). Also using Macs and Linux. I am not at all sure

Re: [PATCH] Put sha1dc on a diet

Hi, On Wed, 1 Mar 2017, Linus Torvalds wrote: > On Wed, Mar 1, 2017 at 1:56 PM, Johannes Schindelin > wrote: > > Footnote *1*: I know, it is easy to forget that some developers cannot > > choose their tools, or even their hardware. In the past, we seemed to take > >

Re: [PATCH] Put sha1dc on a diet

On Wed, Mar 1, 2017 at 1:56 PM, Johannes Schindelin wrote: > Footnote *1*: I know, it is easy to forget that some developers cannot > choose their tools, or even their hardware. In the past, we seemed to take > appropriate care, though. I don't think you need to worry

Re: [PATCH] Put sha1dc on a diet

Hi, On Wed, 1 Mar 2017, Junio C Hamano wrote: > Linus Torvalds writes: > > > That said, I think that it would be lovely to just default to > > USE_SHA1DC and just put the whole attack behind us. Yes, it's slower. > > No, it doesn't really seem to matter that much

Re: [PATCH] Put sha1dc on a diet

On Wed, Mar 1, 2017 at 1:56 PM, Johannes Schindelin wrote: > Hi, > > On Wed, 1 Mar 2017, Junio C Hamano wrote: > >> Linus Torvalds writes: >> >> > That said, I think that it would be lovely to just default to >> > USE_SHA1DC and just put

Re: [PATCH] Put sha1dc on a diet

On Wed, Mar 01, 2017 at 12:14:34PM -0800, Linus Torvalds wrote: > > My biggest concern is the index-pack operation. Try this: > > I'm mobile right now, so I can't test, but I'd this perhaps at least partly > due to the full checksum over the pack-file? > > We have two very different uses of

Re: [PATCH] Put sha1dc on a diet

Linus Torvalds writes: > That said, I think that it would be lovely to just default to > USE_SHA1DC and just put the whole attack behind us. Yes, it's slower. > No, it doesn't really seem to matter that much in practice. Yes. It would be a very good goal.

Re: [PATCH] Put sha1dc on a diet

On Wed, Mar 01, 2017 at 10:49:55AM -0800, Linus Torvalds wrote: > That said, I think that it would be lovely to just default to > USE_SHA1DC and just put the whole attack behind us. Yes, it's slower. > No, it doesn't really seem to matter that much in practice. My biggest concern is the

Re: [PATCH] Put sha1dc on a diet

On Tue, Feb 28, 2017 at 04:30:26PM -0800, Linus Torvalds wrote: > From: Linus Torvalds <torva...@linux-foundation.org> > Date: Tue, 28 Feb 2017 16:12:32 -0800 > Subject: [PATCH] Put sha1dc on a diet > > This removes the unnecessary parts of the sha1dc code, shrinking th

Re: [PATCH] Put sha1dc on a diet

On Wed, Mar 1, 2017 at 10:42 AM, Junio C Hamano wrote: > I see //c99 comments sha1dc is already full of // style comments. I just followed the existing practice. > and also T array[] = { [58] = val } both of > which I think we stay away from (and the former is from the

Re: [PATCH] Put sha1dc on a diet

Linus Torvalds writes: > I notice that sha1dc is in the 'pu' branch now, so let's put my money > where my mouth is, and send in the sha1dc diet patch. I see //c99 comments and also T array[] = { [58] = val } both of which I think we stay away from (and the former

[PATCH] Put sha1dc on a diet

From: Linus Torvalds <torva...@linux-foundation.org> Date: Tue, 28 Feb 2017 16:12:32 -0800 Subject: [PATCH] Put sha1dc on a diet This removes the unnecessary parts of the sha1dc code, shrinking things from [torvalds@i7 git]$ size sha1dc/*.o textdata bss