Closed prestonvanloon closed 4 years ago
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? https://github.com/herumi/bls-eth-go-binary/commit/e21bab7e964fc86640e359be0d2b0c70618fe0f3
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
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.
https://github.com/herumi/bls-eth-go-binary/commit/a2665c5609f01026b3c1493f56c966a879f5d523
% 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
.
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!
I've already fixed it at master branch.
Key changes:
I wrote a quick benchmark to test this
https://gist.github.com/prestonvanloon/0e4645135a43c155b908939ec1428536