code review 224081: This package implements the RIPEMD-160 hash algorithm.

This package implements the RIPEMD-160 hash algorithm.

[rsn, 2010-03-04 08:56:48]

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

I'd like you to review this change.

[rsc, 2010-03-04 18:06:58]

Looks pretty good, thanks.

http://codereview.appspot.com/224081/diff/1001/2001
File src/pkg/Makefile (right):

http://codereview.appspot.com/224081/diff/1001/2001#newcode42
src/pkg/Makefile:42: crypto/ripemd160\
move down one line (sort)

http://codereview.appspot.com/224081/diff/1001/2003
File src/pkg/crypto/ripemd160/ripemd160.go (right):

http://codereview.appspot.com/224081/diff/1001/2003#newcode8
src/pkg/crypto/ripemd160/ripemd160.go:8: // This is a port of the RIPEMD-160
software written
Porting other software requires bringing over its
license, and I'd like to avoid the license here, because
it adds constraints not present in the standard Go license
and just complicates things.

There's nothing in this file from that C code anyway
(it's all essentially taken from the existing sha1 and
md5 implementations).

So please delete this comment (I will address the C code
in the other file in the other file) but do leave a pointer
to a description of the RIPEMD-160 algorithm.

http://codereview.appspot.com/224081/diff/1001/2005
File src/pkg/crypto/ripemd160/ripemd160block.go (right):

http://codereview.appspot.com/224081/diff/1001/2005#newcode11
src/pkg/crypto/ripemd160/ripemd160block.go:11: // The basic functions with ROLL,
F, G, H, I and J in-lined.
This file is very clearly a copy of the RIPEMD C code,
so I'd like to replace it with something completely different
that is original code.

I suggest changing _Block to have the same structure
as in md4/md4block.go, with fixed-length for loops
and rotating variables.  Feel free to copy the comment
about unrolling too.  Getting rid of all the function calls
may well make the code faster anyway.

[rsn, 2010-03-05 20:15:06]

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

Please take another look.

[rsn, 2010-03-05 20:16:06]

http://codereview.appspot.com/224081/diff/1001/2001
File src/pkg/Makefile (right):

http://codereview.appspot.com/224081/diff/1001/2001#newcode42
src/pkg/Makefile:42: crypto/ripemd160\
On 2010/03/04 18:06:58, rsc wrote:
> move down one line (sort)
> 

Done.

http://codereview.appspot.com/224081/diff/1001/2003
File src/pkg/crypto/ripemd160/ripemd160.go (right):

http://codereview.appspot.com/224081/diff/1001/2003#newcode8
src/pkg/crypto/ripemd160/ripemd160.go:8: // This is a port of the RIPEMD-160
software written
On 2010/03/04 18:06:58, rsc wrote:
> Porting other software requires bringing over its
> license, and I'd like to avoid the license here, because
> it adds constraints not present in the standard Go license
> and just complicates things.
> 
> There's nothing in this file from that C code anyway
> (it's all essentially taken from the existing sha1 and
> md5 implementations).
> 
> So please delete this comment (I will address the C code
> in the other file in the other file) but do leave a pointer
> to a description of the RIPEMD-160 algorithm.
> 

Done.

http://codereview.appspot.com/224081/diff/1001/2005
File src/pkg/crypto/ripemd160/ripemd160block.go (right):

http://codereview.appspot.com/224081/diff/1001/2005#newcode11
src/pkg/crypto/ripemd160/ripemd160block.go:11: // The basic functions with ROLL,
F, G, H, I and J in-lined.
On 2010/03/04 18:06:58, rsc wrote:
> This file is very clearly a copy of the RIPEMD C code,
> so I'd like to replace it with something completely different
> that is original code.
> 
> I suggest changing _Block to have the same structure
> as in md4/md4block.go, with fixed-length for loops
> and rotating variables.  Feel free to copy the comment
> about unrolling too.  Getting rid of all the function calls
> may well make the code faster anyway. 
> 

Done.

[rsn, 2010-03-07 01:53:18]

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

Please take another look.

[rsn, 2010-03-07 01:57:27]

On 2010/03/07 01:53:18, rsn wrote:
> Hello rsc (cc: mailto:golang-dev@googlegroups.com),
> 
> Please take another look.

i managed to squeeze a few more KB/s with this latest version of _Block.  i now
get ~7.5MB/s compared to 7.1MB/s.

this is still slower than SHA-160 but i ran out of ideas on how to improve its
performance:

ripemd160.Benchmark_RIPEMD160 10  132859600 ns/op  7.53 MB/s
ripemd160.Benchmark_SHA160    20  105904650 ns/op  9.44 MB/s

[rsc, 2010-03-09 02:03:32]

LGTM

thanks

[rsc, 2010-03-09 02:12:39]

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

crypto/ripemd160: new package

R=rsc
CC=golang-dev
http://codereview.appspot.com/224081

Committer: Russ Cox <rsc@golang.org>