FAQ
hmmm, i can't replicate your benchmark results.

linux/amd64: Intel(R) Core(TM) i5-2520M CPU @ 2.50GHz

benchmark old ns/op new ns/op delta
BenchmarkHash8Bytes 866 824 -4.85%
BenchmarkHash1K 10058 9520 -5.35%
BenchmarkHash8K 74567 70953 -4.85%

benchmark old MB/s new MB/s speedup
BenchmarkHash8Bytes 9.24 9.71 1.05x
BenchmarkHash1K 101.81 107.56 1.06x
BenchmarkHash8K 109.86 115.46 1.05x

https://codereview.appspot.com/6820096/

Search Discussions

  • Notcarl at Nov 6, 2012 at 9:50 pm
    $ cat /proc/cpuinfo
    processor : 0
    vendor_id : GenuineIntel
    cpu family : 6
    model : 44
    model name : Intel(R) Xeon(R) CPU W3690 @ 3.47GHz
    ...



    https://codereview.appspot.com/6820096/
  • Dave at Nov 6, 2012 at 10:04 pm
    Looks pretty good to me as well. I'll run some benchmarks on 32 bit
    platforms once mine get through building the overnight changes.


    https://codereview.appspot.com/6820096/diff/4002/src/pkg/crypto/sha1/sha1block.go
    File src/pkg/crypto/sha1/sha1block.go (right):

    https://codereview.appspot.com/6820096/diff/4002/src/pkg/crypto/sha1/sha1block.go#newcode26
    src/pkg/crypto/sha1/sha1block.go:26: j := i * 4
    j := i << 2 avoids the imul on intel. The compiler should be smarter
    about this.

    https://codereview.appspot.com/6820096/
  • Notcarl at Nov 6, 2012 at 10:51 pm

    On 2012/11/06 22:04:24, dfc wrote:
    Looks pretty good to me as well. I'll run some benchmarks on 32 bit platforms
    once mine get through building the overnight changes.

    https://codereview.appspot.com/6820096/diff/4002/src/pkg/crypto/sha1/sha1block.go
    File src/pkg/crypto/sha1/sha1block.go (right):

    https://codereview.appspot.com/6820096/diff/4002/src/pkg/crypto/sha1/sha1block.go#newcode26
    src/pkg/crypto/sha1/sha1block.go:26: j := i * 4
    j := i << 2 avoids the imul on intel. The compiler should be smarter
    about this.

    I tried changing the variables to unsigned to see if it would change
    anything. I also tried adding in the shift instead of the mult, but
    neither of these two changes affected the results of my benchmarks. I
    think it would be safe to leave this alone for now.

    https://codereview.appspot.com/6820096/
  • Dave at Nov 6, 2012 at 11:07 pm
    Fair enough. The compiler should be able to do this for us eventually.

    I know you're a Googler, so most of the CLA process does not apply to
    you, but have you done whatever is needed on your side ?

    https://codereview.appspot.com/6820096/
  • Ian Lance Taylor at Nov 6, 2012 at 11:18 pm

    On Tue, Nov 6, 2012 at 3:07 PM, wrote:
    Fair enough. The compiler should be able to do this for us eventually.

    I know you're a Googler, so most of the CLA process does not apply to
    you, but have you done whatever is needed on your side ?
    To be clear, are you approving this patch? Let me know.

    We just need to get him into CONTRIBUTORS.

    Ian
  • Dave Cheney at Nov 6, 2012 at 11:18 pm
    Yes, LGTM.
    On Wed, Nov 7, 2012 at 10:18 AM, Ian Lance Taylor wrote:
    On Tue, Nov 6, 2012 at 3:07 PM, wrote:
    Fair enough. The compiler should be able to do this for us eventually.

    I know you're a Googler, so most of the CLA process does not apply to
    you, but have you done whatever is needed on your side ?
    To be clear, are you approving this patch? Let me know.

    We just need to get him into CONTRIBUTORS.

    Ian
  • Dave at Nov 6, 2012 at 11:24 pm
    Some results from

    linux/arm:

    benchmark old ns/op new ns/op delta
    BenchmarkHash8Bytes 6487 6497 +0.15%
    BenchmarkHash1K 69794 70773 +1.40%
    BenchmarkHash8K 517529 525604 +1.56%

    benchmark old MB/s new MB/s speedup
    BenchmarkHash8Bytes 1.23 1.23 1.00x
    BenchmarkHash1K 14.67 14.47 0.99x
    BenchmarkHash8K 15.83 15.59 0.98x

    Wasn't expecting much, and it looks like my expectations were met. This
    is within the margin of error.

    https://codereview.appspot.com/6820096/
  • Dave at Nov 7, 2012 at 2:41 am
    *** Submitted as
    http://code.google.com/p/go/source/detail?r=1369d7bb329d ***

    crypto/sha1: Make sha-1 do block mixup in place

    Benchmarks:

    benchmark old ns/op new ns/op delta
    BenchmarkHash8Bytes 762 674 -11.55%
    BenchmarkHash1K 8791 7375 -16.11%
    BenchmarkHash8K 65094 54881 -15.69%

    benchmark old MB/s new MB/s speedup
    BenchmarkHash8Bytes 10.50 11.86 1.13x
    BenchmarkHash1K 116.48 138.84 1.19x
    BenchmarkHash8K 125.85 149.27 1.19x

    R=dave, rsc, iant
    CC=golang-dev
    http://codereview.appspot.com/6820096

    Committer: Dave Cheney <dave@cheney.net>


    http://codereview.appspot.com/6820096/

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedNov 6, '12 at 9:46p
activeNov 7, '12 at 2:41a
posts9
users3
websitegolang.org

3 users in discussion

Dave: 6 posts Notcarl: 2 posts Ian Lance Taylor: 1 post

People

Translate

site design / logo © 2022 Grokbase