git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Dan Shumow <danshu@microsoft.com>
To: Linus Torvalds <torvalds@linux-foundation.org>,
	Jeff King <peff@peff.net>
Cc: Junio C Hamano <gitster@pobox.com>,
	Git Mailing List <git@vger.kernel.org>,
	Marc Stevens <marc.stevens@cwi.nl>
Subject: RE: [PATCH] Put sha1dc on a diet
Date: Thu, 2 Mar 2017 01:31:17 +0000	[thread overview]
Message-ID: <CY1PR0301MB21073D82F4A6AB0DAD8BF1FCC4280@CY1PR0301MB2107.namprd03.prod.outlook.com> (raw)
In-Reply-To: <CA+55aFz4ixVKVURki8FeXjL5H51A_cQXsZpzKJ-N9n574Yy1rg@mail.gmail.com>

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 we are doing collision detection out, I can reach performance parity with the block-sha1 implementation in the Git codebase, which basically tells me that is about as good as I can do for optimizing the C code.

SHA1 is more amenable to assembler implementation because its use of rotations, which are notoriously difficult to access through C code.  And as this happens in the inner loop of the function, the inline asm tends to not cut it.  This is one of the reasons that the OpenSSL SHA-1 runs like a scalded monkey, compared to the C implemenations.  Marc and I have also discussed using SIMD operations to speed up the UBC checks, which could definitely help achieve better performance, but is highly dependent on processor support.  It will take some time to do either a SIMD implementation of the UBC checks or an assembler implementation.

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 there is interest Marc and I can investigate other processor specific optimizations like ASM or SIMD and circle back with those performance optimizations at a later date.

Also, to Johannes Schindelin's point:
> My concern is about that unexpected turn "oh, let's just switch to C99 because, well, because my compiler canehandle it, and everybody else should just switch tn a modern compiler". That really sounded careless.

While it will probably be a pain, if it is a requirement, we can modify the code to move away from any c99 specific stuff we have in here, if it makes adopting the code more palatable for Git.

Thanks,
Dan



  reply	other threads:[~2017-03-02  3:09 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-01  0:30 [PATCH] Put sha1dc on a diet Linus Torvalds
2017-03-01 18:42 ` Junio C Hamano
2017-03-01 18:49   ` Linus Torvalds
2017-03-01 19:41     ` Junio C Hamano
2017-03-01 21:56       ` Johannes Schindelin
2017-03-01 22:05         ` Junio C Hamano
2017-03-01 22:16         ` Linus Torvalds
2017-03-01 22:51           ` Johannes Schindelin
2017-03-01 23:05             ` Linus Torvalds
2017-03-01 23:19               ` Jeff King
2017-03-02  6:10                 ` Duy Nguyen
2017-03-02 14:45                   ` Johannes Schindelin
2017-03-02 16:35                     ` Linus Torvalds
2017-03-02 18:37                       ` Jeff Hostetler
2017-03-02 19:04                         ` Linus Torvalds
2017-03-02 14:39                 ` Johannes Schindelin
2017-03-02 14:37               ` Johannes Schindelin
2017-03-01 19:53     ` Jeff King
     [not found]       ` <CA+55aFwf3sxKW+dGTMjNAeHMOf=rvctEQohm+rbhEb=e3KLpHw@mail.gmail.com>
2017-03-01 20:34         ` Jeff King
     [not found]           ` <CA+55aFwr1jncrk-cekn0Y8rs_S+zs7RrgQ-Jb-ZbgCvmVrHT_A@mail.gmail.com>
2017-03-01 23:13             ` Jeff King
2017-03-01 23:38           ` Linus Torvalds
2017-03-02  1:31             ` Dan Shumow [this message]
2017-03-02  4:38               ` Junio C Hamano
2017-03-04  1:07                 ` Dan Shumow
2017-03-13 15:13                   ` Jeff King
     [not found]                     ` <CY1PR0301MB2107B3C5131D5DC7F91A0147C4250@CY1PR0301MB2107.namprd03.prod.outlook.com>
     [not found]                       ` <CY1PR0301MB2107876B6E47FBCF03AB1EA1C4250@CY1PR0301MB2107.namprd03.prod.outlook.com>
2017-03-13 19:48                         ` Jeff King
2017-03-13 20:12                           ` Marc Stevens
2017-03-13 20:20                             ` Linus Torvalds
2017-03-13 20:47                               ` Marc Stevens
2017-03-13 21:00                                 ` Jeff King
2017-03-13 21:15                                   ` Marc Stevens
2017-03-16 18:22                                     ` Marc Stevens
2017-03-16 22:06                                       ` Jeff King
2017-03-16 22:07                                         ` Dan Shumow
2017-03-01 19:07 ` Jeff King
2017-03-01 19:10   ` Linus Torvalds

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=CY1PR0301MB21073D82F4A6AB0DAD8BF1FCC4280@CY1PR0301MB2107.namprd03.prod.outlook.com \
    --to=danshu@microsoft.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=marc.stevens@cwi.nl \
    --cc=peff@peff.net \
    --cc=torvalds@linux-foundation.org \
    /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).