code review 7763049: crypto/sha1: faster amd64, 386 implementations

crypto/sha1: faster amd64, 386 implementations

-- amd64 --

On a MacBookPro10,2 (Core i5):

benchmark              old ns/op    new ns/op    delta
BenchmarkHash8Bytes          785          592  -24.59%
BenchmarkHash1K             8727         3014  -65.46%
BenchmarkHash8K            64926        20723  -68.08%

benchmark               old MB/s     new MB/s  speedup
BenchmarkHash8Bytes        10.19        13.50    1.32x
BenchmarkHash1K           117.34       339.71    2.90x
BenchmarkHash8K           126.17       395.31    3.13x

For comparison, on the same machine, openssl 0.9.8r reports
its sha1 speed as 341 MB/s for 1K and 404 MB/s for 8K.

On an Intel Xeon E5520:

benchmark              old ns/op    new ns/op    delta
BenchmarkHash8Bytes          984          707  -28.15%
BenchmarkHash1K            11141         3466  -68.89%
BenchmarkHash8K            82435        23411  -71.60%

benchmark               old MB/s     new MB/s  speedup
BenchmarkHash8Bytes         8.13        11.31    1.39x
BenchmarkHash1K            91.91       295.36    3.21x
BenchmarkHash8K            99.37       349.91    3.52x

For comparison, on the same machine, openssl 1.0.1 reports
its sha1 speed as 286 MB/s for 1K and 394 MB/s for 8K.

-- 386 --

On a MacBookPro10,2 (Core i5):

benchmark              old ns/op    new ns/op    delta
BenchmarkHash8Bytes         1041          713  -31.51%
BenchmarkHash1K            15612         3382  -78.34%
BenchmarkHash8K           110152        22733  -79.36%

benchmark               old MB/s     new MB/s  speedup
BenchmarkHash8Bytes         7.68        11.21    1.46x
BenchmarkHash1K            65.59       302.76    4.62x
BenchmarkHash8K            74.37       360.36    4.85x

On an Intel Xeon E5520:

benchmark              old ns/op    new ns/op    delta
BenchmarkHash8Bytes         1221          842  -31.04%
BenchmarkHash1K            14643         4137  -71.75%
BenchmarkHash8K           108722        27394  -74.80%

benchmark               old MB/s     new MB/s  speedup
BenchmarkHash8Bytes         6.55         9.49    1.45x
BenchmarkHash1K            69.93       247.51    3.54x
BenchmarkHash8K            75.35       299.04    3.97x

[rsc, 2013-03-21 05:31:41]

Hello agl@golang.org (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go/

[dave_cheney.net, 2013-03-21 05:38:02]

On 2013/03/21 05:31:41, rsc wrote:
> Hello mailto:agl@golang.org (cc: mailto:golang-dev@googlegroups.com),
> 
> I'd like you to review this change to
> https://code.google.com/p/go/

Is there any benefit to using the //go:noescape pragma ?

[rsc, 2013-03-21 05:42:08]

On Thu, Mar 21, 2013 at 1:38 AM, <dave@cheney.net> wrote:

> On 2013/03/21 05:31:41, rsc wrote:
>
>> Hello mailto:agl@golang.org (cc:
mailto:golang-dev@**googlegroups.com<golang-dev@googlegroups.com>
>> ),
>>
>
>  I'd like you to review this change to
>> https://code.google.com/p/go/
>>
>
> Is there any benefit to using the //go:noescape pragma ?
>

No. Nearly all code being hashed is going to be in heap buffers already,
and noescape cannot propagate through interfaces anyway.

Russ

[dave_cheney.net, 2013-03-21 05:48:34]

Fair enough, just thought I would ask.

On Thu, Mar 21, 2013 at 4:42 PM, Russ Cox <rsc@golang.org> wrote:
> On Thu, Mar 21, 2013 at 1:38 AM, <dave@cheney.net> wrote:
>>
>> On 2013/03/21 05:31:41, rsc wrote:
>>>
>>> Hello mailto:agl@golang.org (cc: mailto:golang-dev@googlegroups.com),
>>
>>
>>> I'd like you to review this change to
>>> https://code.google.com/p/go/
>>
>>
>> Is there any benefit to using the //go:noescape pragma ?
>
>
> No. Nearly all code being hashed is going to be in heap buffers already, and
> noescape cannot propagate through interfaces anyway.
>
> Russ

[nigeltao, 2013-03-21 07:14:50]

https://codereview.appspot.com/7763049/diff/8001/src/pkg/crypto/sha1/sha1bloc...
File src/pkg/crypto/sha1/sha1block_386.s (right):

https://codereview.appspot.com/7763049/diff/8001/src/pkg/crypto/sha1/sha1bloc...
src/pkg/crypto/sha1/sha1block_386.s:22: // To free up the word pointer (R10), we
add it to e during LOAD/SHUFFLE instead
s/R10/DI/

https://codereview.appspot.com/7763049/diff/8001/src/pkg/crypto/sha1/sha1bloc...
File src/pkg/crypto/sha1/sha1block_amd64.s (right):

https://codereview.appspot.com/7763049/diff/8001/src/pkg/crypto/sha1/sha1bloc...
src/pkg/crypto/sha1/sha1block_amd64.s:102: MOVL	(4*4)(BP), BP
There's a tabs/spaces issue here, compared to the surrounding lines, and
similarly for "MOVL (4*4)(BP), BP" in sha1block_386.s.

[agl1, 2013-03-21 11:26:32]

LGTM

https://codereview.appspot.com/7763049/diff/8001/src/pkg/crypto/sha1/sha1bloc...
File src/pkg/crypto/sha1/sha1block_386.s (right):

https://codereview.appspot.com/7763049/diff/8001/src/pkg/crypto/sha1/sha1bloc...
src/pkg/crypto/sha1/sha1block_386.s:26: MOVL	88(SP), SI; \
It is possible to use the same form as in SHUFFLE and merge this line with the
next? e.g. MOVL ((index+22)*4)(SI), DI.

https://codereview.appspot.com/7763049/diff/8001/src/pkg/crypto/sha1/sha1bloc...
src/pkg/crypto/sha1/sha1block_386.s:95: 
// func block(*digest, []byte)

https://codereview.appspot.com/7763049/diff/8001/src/pkg/crypto/sha1/sha1bloc...
src/pkg/crypto/sha1/sha1block_386.s:108: MOVL	(4*4)(BP), BP
alignment of "BP"

[rsc, 2013-03-21 15:31:51]

https://codereview.appspot.com/7763049/diff/8001/src/pkg/crypto/sha1/sha1bloc...
File src/pkg/crypto/sha1/sha1block_386.s (right):

https://codereview.appspot.com/7763049/diff/8001/src/pkg/crypto/sha1/sha1bloc...
src/pkg/crypto/sha1/sha1block_386.s:22: // To free up the word pointer (R10), we
add it to e during LOAD/SHUFFLE instead
On 2013/03/21 07:14:51, nigeltao wrote:
> s/R10/DI/

Done.

https://codereview.appspot.com/7763049/diff/8001/src/pkg/crypto/sha1/sha1bloc...
src/pkg/crypto/sha1/sha1block_386.s:26: MOVL	88(SP), SI; \
On 2013/03/21 11:26:32, agl1 wrote:
> It is possible to use the same form as in SHUFFLE and merge this line with the
> next? e.g. MOVL ((index+22)*4)(SI), DI.

I need a double-indirect here. The first load fetches the source pointer from
its saved location on the stack. The second load fetches the data word from the
source pointer. I don't think that's possible with an ordinary mov on x86, but
it wouldn't be the first time I've underestimated the instruction set. :-)

https://codereview.appspot.com/7763049/diff/8001/src/pkg/crypto/sha1/sha1bloc...
src/pkg/crypto/sha1/sha1block_386.s:95: 
On 2013/03/21 11:26:32, agl1 wrote:
> // func block(*digest, []byte)

Done.

https://codereview.appspot.com/7763049/diff/8001/src/pkg/crypto/sha1/sha1bloc...
src/pkg/crypto/sha1/sha1block_386.s:108: MOVL	(4*4)(BP), BP
On 2013/03/21 11:26:32, agl1 wrote:
> alignment of "BP"

Done.

https://codereview.appspot.com/7763049/diff/8001/src/pkg/crypto/sha1/sha1bloc...
File src/pkg/crypto/sha1/sha1block_amd64.s (right):

https://codereview.appspot.com/7763049/diff/8001/src/pkg/crypto/sha1/sha1bloc...
src/pkg/crypto/sha1/sha1block_amd64.s:102: MOVL	(4*4)(BP), BP
On 2013/03/21 07:14:51, nigeltao wrote:
> There's a tabs/spaces issue here, compared to the surrounding lines, and
> similarly for "MOVL (4*4)(BP), BP" in sha1block_386.s.

Done.

[rsc, 2013-03-21 15:32:06]

*** Submitted as https://code.google.com/p/go/source/detail?r=e73043345273 ***

crypto/sha1: faster amd64, 386 implementations

-- amd64 --

On a MacBookPro10,2 (Core i5):

benchmark              old ns/op    new ns/op    delta
BenchmarkHash8Bytes          785          592  -24.59%
BenchmarkHash1K             8727         3014  -65.46%
BenchmarkHash8K            64926        20723  -68.08%

benchmark               old MB/s     new MB/s  speedup
BenchmarkHash8Bytes        10.19        13.50    1.32x
BenchmarkHash1K           117.34       339.71    2.90x
BenchmarkHash8K           126.17       395.31    3.13x

For comparison, on the same machine, openssl 0.9.8r reports
its sha1 speed as 341 MB/s for 1K and 404 MB/s for 8K.

On an Intel Xeon E5520:

benchmark              old ns/op    new ns/op    delta
BenchmarkHash8Bytes          984          707  -28.15%
BenchmarkHash1K            11141         3466  -68.89%
BenchmarkHash8K            82435        23411  -71.60%

benchmark               old MB/s     new MB/s  speedup
BenchmarkHash8Bytes         8.13        11.31    1.39x
BenchmarkHash1K            91.91       295.36    3.21x
BenchmarkHash8K            99.37       349.91    3.52x

For comparison, on the same machine, openssl 1.0.1 reports
its sha1 speed as 286 MB/s for 1K and 394 MB/s for 8K.

-- 386 --

On a MacBookPro10,2 (Core i5):

benchmark              old ns/op    new ns/op    delta
BenchmarkHash8Bytes         1041          713  -31.51%
BenchmarkHash1K            15612         3382  -78.34%
BenchmarkHash8K           110152        22733  -79.36%

benchmark               old MB/s     new MB/s  speedup
BenchmarkHash8Bytes         7.68        11.21    1.46x
BenchmarkHash1K            65.59       302.76    4.62x
BenchmarkHash8K            74.37       360.36    4.85x

On an Intel Xeon E5520:

benchmark              old ns/op    new ns/op    delta
BenchmarkHash8Bytes         1221          842  -31.04%
BenchmarkHash1K            14643         4137  -71.75%
BenchmarkHash8K           108722        27394  -74.80%

benchmark               old MB/s     new MB/s  speedup
BenchmarkHash8Bytes         6.55         9.49    1.45x
BenchmarkHash1K            69.93       247.51    3.54x
BenchmarkHash8K            75.35       299.04    3.97x

R=agl, dave
CC=golang-dev
https://codereview.appspot.com/7763049