rootmulti/internal/maps: remove duplicated code, simplify and speed up KVPair (#6689)

Noticed during an audit, we've got duplicated code that's unused.
There were 2 type definitions:
* type kvPair types.Pair
* type KVPair types.Pair

and each had a .bytes() and .Bytes() method respectively that
then used some extra code.

This change deletes the duplicated/unnecessary code but also
while here improves the performance by removing the use of a
bytes.Buffer which is unnecessary given that we are only
length prefixed the key, length prefixing the value hence
the various helpers are unnecessary.

The added benchmarks shows the performance boost
```shell
$ benchstat before after
name           old time/op    new time/op    delta
KVPairBytes-8     146µs ± 1%     142µs ± 2%   -3.05%  (p=0.000 n=18+17)

name           old speed      new speed      delta
KVPairBytes-8  6.84GB/s ± 1%  7.06GB/s ± 2%   +3.15%  (p=0.000 n=18+17)

name           old alloc/op   new alloc/op   delta
KVPairBytes-8    1.01MB ± 0%    1.01MB ± 0%   -0.04%  (p=0.000 n=17+20)

name           old allocs/op  new allocs/op  delta
KVPairBytes-8      6.00 ± 0%      1.00 ± 0%  -83.33%  (p=0.000 n=20+20)
```

Closes: #6688
This commit is contained in:
Emmanuel T Odeke 2020-07-11 04:44:10 -07:00 committed by GitHub
parent e7554bb3b0
commit 152ac24931
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 31 additions and 48 deletions

View File

@ -0,0 +1,13 @@
package maps
import "testing"
func BenchmarkKVPairBytes(b *testing.B) {
kvp := NewKVPair(make([]byte, 128), make([]byte, 1e6))
b.ResetTimer()
b.ReportAllocs()
for i := 0; i < b.N; i++ {
b.SetBytes(int64(len(kvp.Bytes())))
}
}

View File

@ -1,9 +1,7 @@
package maps
import (
"bytes"
"encoding/binary"
"io"
"github.com/tendermint/tendermint/crypto/merkle"
"github.com/tendermint/tendermint/crypto/tmhash"
@ -55,47 +53,12 @@ func (sm *merkleMap) sort() {
sm.sorted = true
}
// kvPair defines a type alias for kv.Pair so that we can create bytes to hash
// when constructing the merkle root. Note, key and values are both length-prefixed.
type kvPair kv.Pair
// bytes returns a byte slice representation of the kvPair where the key and value
// are length-prefixed.
func (kv kvPair) bytes() []byte {
var b bytes.Buffer
err := encodeByteSlice(&b, kv.Key)
if err != nil {
panic(err)
}
err = encodeByteSlice(&b, kv.Value)
if err != nil {
panic(err)
}
return b.Bytes()
}
func encodeByteSlice(w io.Writer, bz []byte) error {
var buf [8]byte
n := binary.PutUvarint(buf[:], uint64(len(bz)))
_, err := w.Write(buf[:n])
if err != nil {
return err
}
_, err = w.Write(bz)
return err
}
// hashKVPairs hashes a kvPair and creates a merkle tree where the leaves are
// byte slices.
func hashKVPairs(kvs kv.Pairs) []byte {
kvsH := make([][]byte, len(kvs))
for i, kvp := range kvs {
kvsH[i] = kvPair(kvp).bytes()
kvsH[i] = KVPair(kvp).Bytes()
}
return merkle.SimpleHashFromByteSlices(kvsH)
@ -177,16 +140,23 @@ func NewKVPair(key, value []byte) KVPair {
// Bytes returns key || value, with both the
// key and value length prefixed.
func (kv KVPair) Bytes() []byte {
var b bytes.Buffer
err := encodeByteSlice(&b, kv.Key)
if err != nil {
panic(err)
}
err = encodeByteSlice(&b, kv.Value)
if err != nil {
panic(err)
}
return b.Bytes()
// In the worst case:
// * 8 bytes to Uvarint encode the length of the key
// * 8 bytes to Uvarint encode the length of the value
// So preallocate for the worst case, which will in total
// be a maximum of 14 bytes wasted, if len(key)=1, len(value)=1,
// but that's going to rare.
buf := make([]byte, 8+len(kv.Key)+8+len(kv.Value))
// Encode the key, prefixed with its length.
nlk := binary.PutUvarint(buf, uint64(len(kv.Key)))
nk := copy(buf[nlk:], kv.Key)
// Encode the value, prefixing with its length.
nlv := binary.PutUvarint(buf[nlk+nk:], uint64(len(kv.Value)))
nv := copy(buf[nlk+nk+nlv:], kv.Value)
return buf[:nlk+nk+nlv+nv]
}
// SimpleHashFromMap computes a merkle tree from sorted map and returns the merkle