crypto/cipher: speed up xor operations in CBC, OBF, CTR...

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.

[minux1, 2013-11-11 12:10:03]

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....
src/pkg/crypto/cipher/xor.go:19: dw := *(*[]uintptr)(unsafe.Pointer(&dst))
this assumes the dst, a, b are uintptr-aligned or the process is
capable of doing unaligned read/write.

Is this the case?

[hanwen-google, 2013-11-11 12:17:54]

yes, the process should support unaligned writes, which I guess is OK for x86
and x86_64. Which architectures don't? (Sparc, ARM, NaCl ?)

[hanwen-google, 2013-11-11 12:18:14]

On 2013/11/11 12:17:54, hanwen-google wrote:
> yes, the process should support unaligned writes, which I guess is OK for x86
> and x86_64. Which architectures don't? (Sparc, ARM, NaCl ?)

(and unaligned read too, of course.)

[minux1, 2013-11-11 13:49:07]

On Mon, Nov 11, 2013 at 7:17 AM, <hanwen@google.com> wrote:

> yes, the process should support unaligned writes, which I guess is OK
> for x86 and x86_64. Which architectures don't? (Sparc, ARM, NaCl ?)
>
virtually all RISC processors don't support unaligned read.

ARM doesn't support it, so if you want to submit this change, you need to
special case x86 here.

An example for this kind of trick: src/pkg/crypto/md5/md5block.go

[bradfitz, 2013-11-11 20:17:25]

use $GOROOT/misc/benchcmp in CL description

[hanwen-google, 2013-11-12 11:58:08]

thanks, updated description, added CFB and restricted the unsafe code to
x86-only

[minux1, 2013-11-12 20:04:06]

On 2013/11/12 11:58:08, hanwen-google wrote:
> thanks, updated description, added CFB and restricted the unsafe code to
> x86-only
you can also use the unsafe path on other architectures if the arguments are
indeed aligned.

see src/pkg/crypto/md5/md5block.go for an example.

[hanwen-google, 2013-12-02 16:06:27]

Hi Adam,

This should not be landed yet, per the freeze instructions, but it might be
worth a gander now?

[dfc, 2013-12-02 22:39:45]

On 2013/12/02 16:06:27, hanwen-google wrote:
> Hi Adam,
> 
> This should not be landed yet, per the freeze instructions, but it might be
> worth a gander now?

Hello,

I poached your benchmarks for autobench, you may be interested in the current
results

go 1.1.2 vs 1.2, linux/amd64

# cipher
benchmark                 old ns/op    new ns/op    delta
BenchmarkAESCFBEncrypt         7460         7192   -3.59%
BenchmarkAESCFBDecrypt         7728         7796   +0.88%
BenchmarkAESOFB                6576         6438   -2.10%
BenchmarkAESCTR                6190         6311   +1.95%
BenchmarkAESCBCEncrypt         6133         6320   +3.05%
BenchmarkAESCBCDecrypt         7153         8581  +19.96%

benchmark                  old MB/s     new MB/s  speedup
BenchmarkAESCFBEncrypt       137.11       142.23    1.04x
BenchmarkAESCFBDecrypt       132.37       131.21    0.99x
BenchmarkAESOFB              155.54       158.88    1.02x
BenchmarkAESCTR              165.27       162.08    0.98x
BenchmarkAESCBCEncrypt       166.94       162.02    0.97x
BenchmarkAESCBCDecrypt       143.14       119.33    0.83x

[dfc, 2013-12-02 23:06:55]

On 2013/12/02 22:39:45, dfc wrote:
> On 2013/12/02 16:06:27, hanwen-google wrote:
> > Hi Adam,
> > 
> > This should not be landed yet, per the freeze instructions, but it might be
> > worth a gander now?
> 
> Hello,
> 
> I poached your benchmarks for autobench, you may be interested in the current
> results
> 
> go 1.1.2 vs 1.2, linux/amd64
> 
> # cipher
> benchmark                 old ns/op    new ns/op    delta
> BenchmarkAESCFBEncrypt         7460         7192   -3.59%
> BenchmarkAESCFBDecrypt         7728         7796   +0.88%
> BenchmarkAESOFB                6576         6438   -2.10%
> BenchmarkAESCTR                6190         6311   +1.95%
> BenchmarkAESCBCEncrypt         6133         6320   +3.05%
> BenchmarkAESCBCDecrypt         7153         8581  +19.96%
> 
> benchmark                  old MB/s     new MB/s  speedup
> BenchmarkAESCFBEncrypt       137.11       142.23    1.04x
> BenchmarkAESCFBDecrypt       132.37       131.21    0.99x
> BenchmarkAESOFB              155.54       158.88    1.02x
> BenchmarkAESCTR              165.27       162.08    0.98x
> BenchmarkAESCBCEncrypt       166.94       162.02    0.97x
> BenchmarkAESCBCDecrypt       143.14       119.33    0.83x

Here is the same test with this CL patched onto 1.2

# cipher
benchmark                 old ns/op    new ns/op    delta
BenchmarkAESCFBEncrypt         7434         3883  -47.77%
BenchmarkAESCFBDecrypt         7736         3866  -50.03%
BenchmarkAESOFB                6579         2651  -59.71%
BenchmarkAESCTR                6236         2685  -56.94%
BenchmarkAESCBCEncrypt         6142         3898  -36.54%
BenchmarkAESCBCDecrypt         7087         4127  -41.77%

benchmark                  old MB/s     new MB/s  speedup
BenchmarkAESCFBEncrypt       137.59       263.40    1.91x
BenchmarkAESCFBDecrypt       132.23       264.61    2.00x
BenchmarkAESOFB              155.49       385.85    2.48x
BenchmarkAESCTR              164.03       380.90    2.32x
BenchmarkAESCBCEncrypt       166.72       262.68    1.58x
BenchmarkAESCBCDecrypt       144.48       248.11    1.72x

[agl1, 2013-12-10 18:18:10]

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/benc...
File src/pkg/crypto/cipher/benchmark_test.go (right):

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/benc...
src/pkg/crypto/cipher/benchmark_test.go:1: package cipher_test
copyright header.

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/benc...
src/pkg/crypto/cipher/benchmark_test.go:9: func BenchmarkAESGCMSeal(b
*testing.B) {
Please put 1K at the end of the test name.

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/benc...
src/pkg/crypto/cipher/benchmark_test.go:25: func BenchmarkAESGCMOpen(b
*testing.B) {
Please put 1K at the end of the test name.

(ditto for all.)

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/benc...
src/pkg/crypto/cipher/benchmark_test.go:38: _, err := aesgcm.Open(buf, nonce[:],
out, nonce[:])
buf[:0]? Otherwise I think it'll try to append to buf each time, allocing and
copying a lot.

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/benc...
src/pkg/crypto/cipher/benchmark_test.go:46: buf := make([]byte, 1023)
please comment the 1023 value to say something like:

// 1024 - 5 in order to exercise padding code for non-integral number of blocks.

and change 1023 to 1024 - 5 as one byte of padding is a special case.

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/cfb.go
File src/pkg/crypto/cipher/cfb.go (right):

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/cfb....
src/pkg/crypto/cipher/cfb.go:15: // We can precompute a larger segment of the
keystream on
Is this a TODO?

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/cfb....
src/pkg/crypto/cipher/cfb.go:58: panic("cipher.NewCFB: IV length must equal
block size")
s/NewCFB/newCFB/

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....
src/pkg/crypto/cipher/ctr.go:22: // ? do something for cipher with blocksz > 512
?
delete comment?

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/ctr....
src/pkg/crypto/cipher/ctr.go:46: for remain < cap(x.out)-x.b.BlockSize() {
can replace cap() with len() here I believe to make the loop more canonical.

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/ctr....
src/pkg/crypto/cipher/ctr.go:48: remain += x.b.BlockSize()
may wish to cache the result of BlockSize() at the top of this function because
I think this will be an interface call every time.

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/ctr....
src/pkg/crypto/cipher/ctr.go:54: // why not constant time?
because the counter values aren't secret dependent.

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/ofb.go
File src/pkg/crypto/cipher/ofb.go (right):

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/ofb....
src/pkg/crypto/cipher/ofb.go:44: for remain < cap(x.out)-bs {
again, s/cap/len/ to make the loop clearer.

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/xor.go
File src/pkg/crypto/cipher/xor.go (right):

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/xor....
src/pkg/crypto/cipher/xor.go:1: package cipher
copyright header.

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/xor....
src/pkg/crypto/cipher/xor.go:8: const wordsize = int(unsafe.Sizeof(uintptr(0)))
wordSize

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/xor....
src/pkg/crypto/cipher/xor.go:11: // fastXorBytes xors in bulk. It only works on
architectures that
write either "XOR" or "xor" throughout in order to be consistent with the rest
of crypto/

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/xor....
src/pkg/crypto/cipher/xor.go:47: // xorBytes xors the bytes in a and b. dst is
assumed to have enough
s/dst/The destination/ (Rob likes sentences to start with capitals.)

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/xor_...
File src/pkg/crypto/cipher/xor_test.go (right):

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/xor_...
src/pkg/crypto/cipher/xor_test.go:1: package cipher
copyright header.

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/xor_...
src/pkg/crypto/cipher/xor_test.go:8: func TestXor(t *testing.T) {
TestXOR

[hanwen-google, 2013-12-10 23:10:27]

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/benc...
File src/pkg/crypto/cipher/benchmark_test.go (right):

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/benc...
src/pkg/crypto/cipher/benchmark_test.go:1: package cipher_test
On 2013/12/10 18:18:10, agl1 wrote:
> copyright header.

Done.

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/benc...
src/pkg/crypto/cipher/benchmark_test.go:9: func BenchmarkAESGCMSeal(b
*testing.B) {
On 2013/12/10 18:18:10, agl1 wrote:
> Please put 1K at the end of the test name.

Done.

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/benc...
src/pkg/crypto/cipher/benchmark_test.go:25: func BenchmarkAESGCMOpen(b
*testing.B) {
On 2013/12/10 18:18:10, agl1 wrote:
> Please put 1K at the end of the test name.
> 
> (ditto for all.)

Done.

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/benc...
src/pkg/crypto/cipher/benchmark_test.go:38: _, err := aesgcm.Open(buf, nonce[:],
out, nonce[:])
On 2013/12/10 18:18:10, agl1 wrote:
> buf[:0]? Otherwise I think it'll try to append to buf each time, allocing and
> copying a lot.

Done.

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/benc...
src/pkg/crypto/cipher/benchmark_test.go:46: buf := make([]byte, 1023)
On 2013/12/10 18:18:10, agl1 wrote:
> please comment the 1023 value to say something like:
> 
> // 1024 - 5 in order to exercise padding code for non-integral number of
blocks.
> 
> and change 1023 to 1024 - 5 as one byte of padding is a special case.

AFAICT, the stream ciphers don't have padding. Am I missing something?

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/cfb.go
File src/pkg/crypto/cipher/cfb.go (right):

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/cfb....
src/pkg/crypto/cipher/cfb.go:15: // We can precompute a larger segment of the
keystream on
On 2013/12/10 18:18:10, agl1 wrote:
> Is this a TODO?

perhaps? You think I should rather remove the comment? I'm not sure I want to
put my name on it.

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/cfb....
src/pkg/crypto/cipher/cfb.go:58: panic("cipher.NewCFB: IV length must equal
block size")
On 2013/12/10 18:18:10, agl1 wrote:
> s/NewCFB/newCFB/

Done.

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....
src/pkg/crypto/cipher/ctr.go:22: // ? do something for cipher with blocksz > 512
?
On 2013/12/10 18:18:10, agl1 wrote:
> delete comment?

so I can stop worrying about ultra-large blocksizes for now?

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/ctr....
src/pkg/crypto/cipher/ctr.go:46: for remain < cap(x.out)-x.b.BlockSize() {
On 2013/12/10 18:18:10, agl1 wrote:
> can replace cap() with len() here I believe to make the loop more canonical.

Done

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/ctr....
src/pkg/crypto/cipher/ctr.go:48: remain += x.b.BlockSize()
On 2013/12/10 18:18:10, agl1 wrote:
> may wish to cache the result of BlockSize() at the top of this function
because
> I think this will be an interface call every time.

Done.

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/ctr....
src/pkg/crypto/cipher/ctr.go:54: // why not constant time?
On 2013/12/10 18:18:10, agl1 wrote:
> because the counter values aren't secret dependent.

OK, but they are in GCM? (since gcmInc32 is constant time) (just curious)

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/ofb.go
File src/pkg/crypto/cipher/ofb.go (right):

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/ofb....
src/pkg/crypto/cipher/ofb.go:44: for remain < cap(x.out)-bs {
On 2013/12/10 18:18:10, agl1 wrote:
> again, s/cap/len/ to make the loop clearer.

Done.

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/xor.go
File src/pkg/crypto/cipher/xor.go (right):

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/xor....
src/pkg/crypto/cipher/xor.go:1: package cipher
On 2013/12/10 18:18:10, agl1 wrote:
> copyright header.

Done.

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/xor....
src/pkg/crypto/cipher/xor.go:8: const wordsize = int(unsafe.Sizeof(uintptr(0)))
On 2013/12/10 18:18:10, agl1 wrote:
> wordSize

Done.

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/xor....
src/pkg/crypto/cipher/xor.go:11: // fastXorBytes xors in bulk. It only works on
architectures that
On 2013/12/10 18:18:10, agl1 wrote:
> write either "XOR" or "xor" throughout in order to be consistent with the rest
> of crypto/

Done.

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/xor....
src/pkg/crypto/cipher/xor.go:47: // xorBytes xors the bytes in a and b. dst is
assumed to have enough
On 2013/12/10 18:18:10, agl1 wrote:
> s/dst/The destination/ (Rob likes sentences to start with capitals.)

Done.

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/xor_...
File src/pkg/crypto/cipher/xor_test.go (right):

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/xor_...
src/pkg/crypto/cipher/xor_test.go:1: package cipher
On 2013/12/10 18:18:10, agl1 wrote:
> copyright header.

Done.

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/xor_...
src/pkg/crypto/cipher/xor_test.go:8: func TestXor(t *testing.T) {
On 2013/12/10 18:18:10, agl1 wrote:
> TestXOR

Done.

[hanwen-google, 2013-12-10 23:16:23]

btw - this is broken on 386. Don't land just yet.

[hanwen-google, 2013-12-10 23:20:51]

386 seems to work now. It was a problem in GCM, which I don't quite understand,
so it could do with extra scrutiny.

[dfc, 2013-12-10 23:41:44]

On 2013/12/10 23:20:51, hanwen-google wrote:
> 386 seems to work now. It was a problem in GCM, which I don't quite
understand,
> so it could do with extra scrutiny.

LGTM on linux/386

benchmark                   old ns/op    new ns/op    delta
BenchmarkAESGCMSeal1K          286754       281869   -1.70%
BenchmarkAESGCMOpen1K          284856       282009   -1.00%
BenchmarkAESCFBEncrypt1K       174355       143971  -17.43%
BenchmarkAESCFBDecrypt1K       187719       144110  -23.23%
BenchmarkAESOFB1K              161220       144450  -10.40%
BenchmarkAESCTR1K              159490       143386  -10.10%
BenchmarkAESCBCEncrypt1K       167613       152130   -9.24%
BenchmarkAESCBCDecrypt1K       168893       149707  -11.36%

benchmark                    old MB/s     new MB/s  speedup
BenchmarkAESGCMSeal1K            3.57         3.63    1.02x
BenchmarkAESGCMOpen1K            3.59         3.63    1.01x
BenchmarkAESCFBEncrypt1K         5.84         7.08    1.21x
BenchmarkAESCFBDecrypt1K         5.43         7.07    1.30x
BenchmarkAESOFB1K                6.32         7.05    1.12x
BenchmarkAESCTR1K                6.39         7.11    1.11x
BenchmarkAESCBCEncrypt1K         6.11         6.73    1.10x
BenchmarkAESCBCDecrypt1K         6.06         6.84    1.13x

[agl, 2013-12-11 16:28:25]

LGTM with nits.

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/benc...
File src/pkg/crypto/cipher/benchmark_test.go (right):

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/benc...
src/pkg/crypto/cipher/benchmark_test.go:46: buf := make([]byte, 1023)
On 2013/12/10 23:10:28, hanwen-google wrote:
> AFAICT, the stream ciphers don't have padding. Am I missing something?

Sorry, brain fart. Was thinking that this was a block mode.

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/cfb.go
File src/pkg/crypto/cipher/cfb.go (right):

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/cfb....
src/pkg/crypto/cipher/cfb.go:15: // We can precompute a larger segment of the
keystream on
On 2013/12/10 23:10:28, hanwen-google wrote:
> On 2013/12/10 18:18:10, agl1 wrote:
> > Is this a TODO?
> 
> perhaps? You think I should rather remove the comment? I'm not sure I want to
> put my name on it.

In which case I think it's ok to keep the comment as is, but to move it inside
XORKeyStream.

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....
src/pkg/crypto/cipher/ctr.go:22: // ? do something for cipher with blocksz > 512
?
On 2013/12/10 23:10:28, hanwen-google wrote:
> On 2013/12/10 18:18:10, agl1 wrote:
> > delete comment?
> 
> so I can stop worrying about ultra-large blocksizes for now?

Well, you could handle it in NewCTR by making the cap of |out| equal to
max(streamBufferSize, block.BlockSize()).

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/ctr....
src/pkg/crypto/cipher/ctr.go:54: // why not constant time?
On 2013/12/10 23:10:28, hanwen-google wrote:
> On 2013/12/10 18:18:10, agl1 wrote:
> > because the counter values aren't secret dependent.
> 
> OK, but they are in GCM? (since gcmInc32 is constant time) (just curious)

They're not secret in gcm either. The constant time code was probably force of
habit.

[hanwen-google, 2013-12-11 16:31:02]

On Wed, Dec 11, 2013 at 5:28 PM,  <agl@chromium.org> wrote:
>> On 2013/12/10 18:18:10, agl1 wrote:
>> > because the counter values aren't secret dependent.
>
>
>> OK, but they are in GCM? (since gcmInc32 is constant time) (just
>
> curious)
>
> They're not secret in gcm either. The constant time code was probably
> force of habit.

There is another 10% to be had by breaking out early there. I'll send
a follow-up CL.

--
Google Germany GmbH - ABC-Str. 19 - 20354 Hamburg
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg - Geschäftsführer: Graham Law,
Christine Elizabeth Flores

[hanwen-google, 2013-12-11 20:15:50]

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....
src/pkg/crypto/cipher/ctr.go:22: // ? do something for cipher with blocksz > 512
?
On 2013/12/11 16:28:26, agl wrote:
> On 2013/12/10 23:10:28, hanwen-google wrote:
> > On 2013/12/10 18:18:10, agl1 wrote:
> > > delete comment?
> > 
> > so I can stop worrying about ultra-large blocksizes for now?
> 
> Well, you could handle it in NewCTR by making the cap of |out| equal to
> max(streamBufferSize, block.BlockSize()).

Done.

https://codereview.appspot.com/24250044/diff/80001/src/pkg/crypto/cipher/ctr....
src/pkg/crypto/cipher/ctr.go:54: // why not constant time?
On 2013/12/11 16:28:26, agl wrote:
> On 2013/12/10 23:10:28, hanwen-google wrote:
> > On 2013/12/10 18:18:10, agl1 wrote:
> > > because the counter values aren't secret dependent.
> > 
> > OK, but they are in GCM? (since gcmInc32 is constant time) (just curious)
> 
> They're not secret in gcm either. The constant time code was probably force of
> habit.

ack.

[agl1, 2013-12-11 21:05:26]

*** 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>

[dfc, 2013-12-11 22:34:36]

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/