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

Issue 24250044: crypto/cipher: speed up xor operations in CBC, OBF, CTR... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 9 months ago by hanwen-google
Modified:
8 years, 7 months ago
Reviewers:
agl, dfc
CC:
agl1, dfc, agl, golang-dev
Visibility:
Public.

Description

crypto/cipher: speed up xor operations in CBC, CFB, OBF, CTR and GCM on 386 and amd64 Intel(R) Core(TM) i5-2540M CPU @ 2.60GHz: benchmark old MB/s new MB/s speedup BenchmarkAESGCMSeal1K 82.39 92.05 1.12x BenchmarkAESGCMOpen1K 82.28 91.88 1.12x BenchmarkAESCFBEncrypt1K 141.54 277.59 1.96x BenchmarkAESCFBDecrypt1K 133.06 278.07 2.09x BenchmarkAESOFB1K 160.51 380.24 2.37x BenchmarkAESCTR1K 164.07 429.25 2.62x BenchmarkAESCBCEncrypt1K 170.99 263.74 1.54x BenchmarkAESCBCDecrypt1K 124.96 249.14 1.99x Fixes issue 6741.

Patch Set 1 #

Patch Set 2 : diff -r 9fe43a708b89 https://code.google.com/p/go #

Total comments: 1

Patch Set 3 : diff -r 9fe43a708b89 https://code.google.com/p/go #

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

Patch Set 5 : diff -r 9fe43a708b89 https://code.google.com/p/go #

Total comments: 42

Patch Set 6 : diff -r c8d3de543c1b https://code.google.com/p/go #

Patch Set 7 : diff -r c8d3de543c1b https://code.google.com/p/go #

Patch Set 8 : diff -r c8d3de543c1b https://code.google.com/p/go #

Patch Set 9 : diff -r c8d3de543c1b https://code.google.com/p/go #

Unified diffs Side-by-side diffs Delta from patch set Stats (+358 lines, -93 lines) Patch
A src/pkg/crypto/cipher/benchmark_test.go View 1 2 3 4 5 1 chunk +139 lines, -0 lines 0 comments Download
M src/pkg/crypto/cipher/cbc.go View 1 2 chunks +5 lines, -12 lines 0 comments Download
M src/pkg/crypto/cipher/cfb.go View 1 2 3 4 5 6 7 8 2 chunks +30 lines, -29 lines 0 comments Download
M src/pkg/crypto/cipher/ctr.go View 1 2 3 4 5 6 7 8 1 chunk +37 lines, -16 lines 0 comments Download
M src/pkg/crypto/cipher/gcm.go View 1 2 3 4 5 6 7 3 chunks +3 lines, -10 lines 0 comments Download
M src/pkg/crypto/cipher/gcm_test.go View 1 1 chunk +0 lines, -16 lines 0 comments Download
M src/pkg/crypto/cipher/ofb.go View 1 2 3 4 5 6 7 8 2 chunks +32 lines, -10 lines 0 comments Download
A src/pkg/crypto/cipher/xor.go View 1 2 3 4 5 6 7 1 chunk +84 lines, -0 lines 0 comments Download
A src/pkg/crypto/cipher/xor_test.go View 1 2 3 4 5 1 chunk +28 lines, -0 lines 0 comments Download

Messages

Total messages: 20
minux1
https://codereview.appspot.com/24250044/diff/20001/src/pkg/crypto/cipher/xor.go File src/pkg/crypto/cipher/xor.go (right): https://codereview.appspot.com/24250044/diff/20001/src/pkg/crypto/cipher/xor.go#newcode19 src/pkg/crypto/cipher/xor.go:19: dw := *(*[]uintptr)(unsafe.Pointer(&dst)) this assumes the dst, a, b ...
8 years, 9 months ago (2013-11-11 12:10:03 UTC) #1
hanwen-google
yes, the process should support unaligned writes, which I guess is OK for x86 and ...
8 years, 9 months ago (2013-11-11 12:17:54 UTC) #2
hanwen-google
On 2013/11/11 12:17:54, hanwen-google wrote: > yes, the process should support unaligned writes, which I ...
8 years, 9 months ago (2013-11-11 12:18:14 UTC) #3
minux1
On Mon, Nov 11, 2013 at 7:17 AM, <hanwen@google.com> wrote: > yes, the process should ...
8 years, 9 months ago (2013-11-11 13:49:07 UTC) #4
bradfitz
use $GOROOT/misc/benchcmp in CL description
8 years, 9 months ago (2013-11-11 20:17:25 UTC) #5
hanwen-google
thanks, updated description, added CFB and restricted the unsafe code to x86-only
8 years, 9 months ago (2013-11-12 11:58:08 UTC) #6
minux1
On 2013/11/12 11:58:08, hanwen-google wrote: > thanks, updated description, added CFB and restricted the unsafe ...
8 years, 9 months ago (2013-11-12 20:04:06 UTC) #7
hanwen-google
Hi Adam, This should not be landed yet, per the freeze instructions, but it might ...
8 years, 8 months ago (2013-12-02 16:06:27 UTC) #8
dfc
On 2013/12/02 16:06:27, hanwen-google wrote: > Hi Adam, > > This should not be landed ...
8 years, 8 months ago (2013-12-02 22:39:45 UTC) #9
dfc
On 2013/12/02 22:39:45, dfc wrote: > On 2013/12/02 16:06:27, hanwen-google wrote: > > Hi Adam, ...
8 years, 8 months ago (2013-12-02 23:06:55 UTC) #10
agl1
https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/benchmark_test.go File src/pkg/crypto/cipher/benchmark_test.go (right): https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/benchmark_test.go#newcode1 src/pkg/crypto/cipher/benchmark_test.go:1: package cipher_test copyright header. https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/benchmark_test.go#newcode9 src/pkg/crypto/cipher/benchmark_test.go:9: func BenchmarkAESGCMSeal(b *testing.B) ...
8 years, 8 months ago (2013-12-10 18:18:10 UTC) #11
hanwen-google
https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/benchmark_test.go File src/pkg/crypto/cipher/benchmark_test.go (right): https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/benchmark_test.go#newcode1 src/pkg/crypto/cipher/benchmark_test.go:1: package cipher_test On 2013/12/10 18:18:10, agl1 wrote: > copyright ...
8 years, 8 months ago (2013-12-10 23:10:27 UTC) #12
hanwen-google
btw - this is broken on 386. Don't land just yet.
8 years, 8 months ago (2013-12-10 23:16:23 UTC) #13
hanwen-google
386 seems to work now. It was a problem in GCM, which I don't quite ...
8 years, 8 months ago (2013-12-10 23:20:51 UTC) #14
dfc
On 2013/12/10 23:20:51, hanwen-google wrote: > 386 seems to work now. It was a problem ...
8 years, 8 months ago (2013-12-10 23:41:44 UTC) #15
agl
LGTM with nits. https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/benchmark_test.go File src/pkg/crypto/cipher/benchmark_test.go (right): https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/benchmark_test.go#newcode46 src/pkg/crypto/cipher/benchmark_test.go:46: buf := make([]byte, 1023) On 2013/12/10 ...
8 years, 8 months ago (2013-12-11 16:28:25 UTC) #16
hanwen-google
On Wed, Dec 11, 2013 at 5:28 PM, <agl@chromium.org> wrote: >> On 2013/12/10 18:18:10, agl1 ...
8 years, 8 months ago (2013-12-11 16:31:02 UTC) #17
hanwen-google
https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/ctr.go File src/pkg/crypto/cipher/ctr.go (right): https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/ctr.go#newcode22 src/pkg/crypto/cipher/ctr.go:22: // ? do something for cipher with blocksz > ...
8 years, 8 months ago (2013-12-11 20:15:50 UTC) #18
agl1
*** Submitted as https://code.google.com/p/go/source/detail?r=a073d65e6f8c *** crypto/cipher: speed up xor operations in CBC, CFB, OBF, CTR ...
8 years, 8 months ago (2013-12-11 21:05:26 UTC) #19
dfc
8 years, 8 months ago (2013-12-11 22:34:36 UTC) #20
Bloody hell guys, this is amazing

1.2 vs tip

# cipher
benchmark                 old ns/op    new ns/op    delta
BenchmarkAESCFBEncrypt         7079         3878  -45.22%
BenchmarkAESCFBDecrypt         7723         3873  -49.85%
BenchmarkAESOFB                6383         2655  -58.41%
BenchmarkAESCTR                6235         2515  -59.66%
BenchmarkAESCBCEncrypt         6235         4102  -34.21%
BenchmarkAESCBCDecrypt         8298         4282  -48.40%

benchmark                  old MB/s     new MB/s  speedup
BenchmarkAESCFBEncrypt       144.50       263.79    1.83x
BenchmarkAESCFBDecrypt       132.45       264.12    1.99x
BenchmarkAESOFB              160.25       385.23    2.40x
BenchmarkAESCTR              164.07       406.65    2.48x
BenchmarkAESCBCEncrypt       164.22       249.60    1.52x
BenchmarkAESCBCDecrypt       123.39       239.10    1.94x

On Thu, Dec 12, 2013 at 8:05 AM,  <agl@golang.org> wrote:
> *** Submitted as
> https://code.google.com/p/go/source/detail?r=a073d65e6f8c ***
>
> crypto/cipher: speed up xor operations in CBC, CFB, OBF, CTR
> and GCM on 386 and amd64
>
> Intel(R) Core(TM) i5-2540M CPU @ 2.60GHz:
>
>
> benchmark                    old MB/s     new MB/s  speedup
> BenchmarkAESGCMSeal1K           82.39        92.05    1.12x
> BenchmarkAESGCMOpen1K           82.28        91.88    1.12x
> BenchmarkAESCFBEncrypt1K       141.54       277.59    1.96x
> BenchmarkAESCFBDecrypt1K       133.06       278.07    2.09x
> BenchmarkAESOFB1K              160.51       380.24    2.37x
> BenchmarkAESCTR1K              164.07       429.25    2.62x
> BenchmarkAESCBCEncrypt1K       170.99       263.74    1.54x
> BenchmarkAESCBCDecrypt1K       124.96       249.14    1.99x
>
> Fixes issue 6741.
>
> R=agl, dave, agl
> CC=golang-dev
> https://codereview.appspot.com/24250044
>
> Committer: Adam Langley <agl@golang.org>
>
>
> https://codereview.appspot.com/24250044/
Sign in to reply to this message.

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