AreAllMsgDifferent: Use [32]byte instead of string for less memory allocation

[prestonvanloon]

Key changes:

• Added godoc to AreAllMsgDifferent
• Changed signature of AreAllMsgDifferent to only use 32 byte message sizes
• Changed the set to use [32]byte instead of string

I wrote a quick benchmark to test this

goos: linux
goarch: amd64
BenchmarkString-8           3574            343511 ns/op          269024 B/op       3074 allocs/op
BenchmarkByte32-8           5408            203741 ns/op          165675 B/op         38 allocs/op
PASS

https://gist.github.com/prestonvanloon/0e4645135a43c155b908939ec1428536

[herumi]

Thank you for the useful patch.
I have a question.

set := make(map[[MSG_SIZE]byte]struct{}, MSG_SIZE)

What does this mean? set is not array.

set := map[[MSG_SIZE]byte]struct{}{} // ok?

And I reduced the copy. Is it okay?
e21bab7

[prestonvanloon]

You can make a map of a given size. Without make this method is almost as slow as it was before with more allocation.

BenchmarkString-8                   2961            347201 ns/op          269026 B/op       3074 allocs/op
BenchmarkByte32-8                   6016            214258 ns/op          165688 B/op         38 allocs/op
BenchmarkByte32NoMake-8             3427            338612 ns/op          331992 B/op         94 allocs/op

I have tested benchmarked these implementations below.

My changes:

goos: linux
goarch: amd64
pkg: github.com/herumi/bls-eth-go-binary/bls
BenchmarkAreAllMsgDifferent-8               3282            351492 ns/op          329837 B/op         89 allocs/op
PASS
ok      github.com/herumi/bls-eth-go-binary/bls 1.730s

Your changes:

goos: linux
goarch: amd64
pkg: github.com/herumi/bls-eth-go-binary/bls
BenchmarkAreAllMsgDifferent-8               3174            345904 ns/op          331970 B/op         94 allocs/op
PASS
ok      github.com/herumi/bls-eth-go-binary/bls 1.672s

Your changes with make on the map

goos: linux
goarch: amd64
pkg: github.com/herumi/bls-eth-go-binary/bls
BenchmarkAreAllMsgDifferent-8               3398            341624 ns/op          329801 B/op         88 allocs/op
PASS
ok      github.com/herumi/bls-eth-go-binary/bls 1.740s

Performance is pretty close, but the number of allocations is different.

Given these results, I would recommend not using unsafe.Pointer to save 1 alloc.
What do you think?

Some additional reading on golang maps can be found here, if it's helpful. https://blog.golang.org/maps

[herumi]

You can make a map of a given size.

I see, then It seems to be better to set not MSG_SIZE but n.
It reduces the number of reallocation of a map for large data.

unsafe.Pointer reduce not alloc but memory copy.
But the difference seems small, then I'll remove unsafe.Pionter.

a2665c5

% go test --bench Are . -benchmem
goos: linux
goarch: amd64
pkg: github.com/herumi/bls-eth-go-binary/bls
BenchmarkAreAllMsgDifferent-4               5341            203365 ns/op          160755 B/op         21 allocs/op
BenchmarkAreAllMsgDifferent2-4              5463            214458 ns/op          160755 B/op         21 allocs/op

The two results shows w/ and wo/ unsafe.Pointer.

[prestonvanloon]

I see, then It seems to be better to set not MSG_SIZE but n.

Oops! Yes, I meant to make it size n. I’ll follow up tomorrow morning, thanks!

[prestonvanloon]

Sorry, I forgot to fix this

[herumi]

I've already fixed it at master branch.