Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(2256)

Issue 8088044: code review 8088044: go.crypto/sha3: change keccakF to stateless function

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 years, 7 months ago by Eric Roshan Eisner
Modified:
6 years, 4 months ago
Reviewers:
nigeltao, jcb, agl1
CC:
jcb, agl1, golang-dev, nigeltao
Visibility:
Public.

Description

go.crypto/sha3: change keccakF to stateless function Taken from my implementation: https://bitbucket.org/ede/sha3 Performance gain from using less memory and more registers. benchmark old ns/op new ns/op delta BenchmarkPermutationFunction 1484 1118 -24.66% BenchmarkBulkKeccak512 374993 295178 -21.28% BenchmarkBulkKeccak256 215496 172335 -20.03% benchmark old MB/s new MB/s speedup BenchmarkPermutationFunction 134.76 178.80 1.33x BenchmarkBulkKeccak512 43.69 55.51 1.27x BenchmarkBulkKeccak256 76.03 95.07 1.25x

Patch Set 1 #

Patch Set 2 : diff -r 4b83cf024724 https://code.google.com/p/go.crypto #

Patch Set 3 : diff -r 4b83cf024724 https://code.google.com/p/go.crypto #

Patch Set 4 : diff -r 4b83cf024724 https://code.google.com/p/go.crypto #

Total comments: 2

Patch Set 5 : diff -r 4b83cf024724 https://code.google.com/p/go.crypto #

Unified diffs Side-by-side diffs Delta from patch set Stats (+123 lines, -137 lines) Patch
M sha3/keccakf.go View 1 2 3 4 1 chunk +113 lines, -119 lines 0 comments Download
M sha3/sha3.go View 1 5 chunks +8 lines, -11 lines 0 comments Download
M sha3/sha3_test.go View 1 1 chunk +2 lines, -7 lines 0 comments Download

Messages

Total messages: 11
Eric Roshan Eisner
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go.crypto
6 years, 7 months ago (2013-03-31 06:23:13 UTC) #1
nigeltao
Go code LGTM but give jcb or agl a chance to review this for crypto ...
6 years, 7 months ago (2013-04-02 01:15:00 UTC) #2
Eric Roshan Eisner
On 2013/04/02 01:15:00, nigeltao wrote: > Go code LGTM but give jcb or agl a ...
6 years, 7 months ago (2013-04-02 06:33:59 UTC) #3
jcb
LGTM- this change shouldn't have any bearing on security. Personally I think it's incrementally cleaner ...
6 years, 7 months ago (2013-04-02 12:58:37 UTC) #4
agl1
LGTM. I agree that the function is, conceptually, a member of the digest. But this ...
6 years, 7 months ago (2013-04-02 14:30:59 UTC) #5
agl1
*** Submitted as https://code.google.com/p/go/source/detail?r=2e6f4675f294&repo=crypto *** go.crypto/sha3: change keccakF to stateless function Taken from my implementation: ...
6 years, 7 months ago (2013-04-02 14:42:58 UTC) #6
nigeltao
jcb: between this CL and the compiler change (https://codereview.appspot.com/7944043/) I recently submitted, I'm curious how ...
6 years, 7 months ago (2013-04-02 23:47:36 UTC) #7
jcb
Is it possible to cycle-count level benchmarking in Go? All of the benchmarks published for ...
6 years, 7 months ago (2013-04-03 01:26:46 UTC) #8
nigeltao
On Wed, Apr 3, 2013 at 12:26 PM, Joseph Bonneau <jcb@google.com> wrote: > Is it ...
6 years, 7 months ago (2013-04-03 02:45:04 UTC) #9
jcb
Yes, I made a script to compare all the hashes. With the optimizations, FWIW: SHA3 ...
6 years, 7 months ago (2013-04-03 03:20:59 UTC) #10
remyoudompheng
6 years, 4 months ago (2013-07-21 10:22:07 UTC) #11
R=close
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b