Skip to content

Commit

Permalink
Revert "trie: improve node rlp decoding performance (#25357)"
Browse files Browse the repository at this point in the history 
This reverts commit a1b8892.
  • Loading branch information
@MariusVanDerWijden
MariusVanDerWijden committed Aug 22, 2022
1 parent 6d711f0 commit b9606ab
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 158 deletions.
14 changes: 3 additions & 11 deletions trie/database.go
View file
Expand Up @@ -163,10 +163,7 @@ func (n *cachedNode) rlp() []byte {
// or by regenerating it from the rlp encoded blob.
func (n *cachedNode) obj(hash common.Hash) node {
if node, ok := n.node.(rawNode); ok {
// The raw-blob format nodes are loaded from either from
// clean cache or the database, they are all in their own
// copy and safe to use unsafe decoder.
return mustDecodeNodeUnsafe(hash[:], node)
return mustDecodeNode(hash[:], node)
}
return expandNode(hash[:], n.node)
}
Expand Down Expand Up @@ -349,10 +346,7 @@ func (db *Database) node(hash common.Hash) node {
if enc := db.cleans.Get(nil, hash[:]); enc != nil {
memcacheCleanHitMeter.Mark(1)
memcacheCleanReadMeter.Mark(int64(len(enc)))

// The returned value from cache is in its own copy,
// safe to use mustDecodeNodeUnsafe for decoding.
return mustDecodeNodeUnsafe(hash[:], enc)
return mustDecodeNode(hash[:], enc)
}
}
// Retrieve the node from the dirty cache if available
Expand All @@ -377,9 +371,7 @@ func (db *Database) node(hash common.Hash) node {
memcacheCleanMissMeter.Mark(1)
memcacheCleanWriteMeter.Mark(int64(len(enc)))
}
// The returned value from database is in its own copy,
// safe to use mustDecodeNodeUnsafe for decoding.
return mustDecodeNodeUnsafe(hash[:], enc)
return mustDecodeNode(hash[:], enc)
}

// Node retrieves an encoded cached trie node from memory. If it cannot be found
Expand Down
30 changes: 4 additions & 26 deletions trie/node.go
View file
Expand Up @@ -99,7 +99,6 @@ func (n valueNode) fstring(ind string) string {
return fmt.Sprintf("%x ", []byte(n))
}

// mustDecodeNode is a wrapper of decodeNode and panic if any error is encountered.
func mustDecodeNode(hash, buf []byte) node {
n, err := decodeNode(hash, buf)
if err != nil {
Expand All @@ -108,29 +107,8 @@ func mustDecodeNode(hash, buf []byte) node {
return n
}

// mustDecodeNodeUnsafe is a wrapper of decodeNodeUnsafe and panic if any error is
// encountered.
func mustDecodeNodeUnsafe(hash, buf []byte) node {
n, err := decodeNodeUnsafe(hash, buf)
if err != nil {
panic(fmt.Sprintf("node %x: %v", hash, err))
}
return n
}

// decodeNode parses the RLP encoding of a trie node. It will deep-copy the passed
// byte slice for decoding, so it's safe to modify the byte slice afterwards. The-
// decode performance of this function is not optimal, but it is suitable for most
// scenarios with low performance requirements and hard to determine whether the
// byte slice be modified or not.
// decodeNode parses the RLP encoding of a trie node.
func decodeNode(hash, buf []byte) (node, error) {
return decodeNodeUnsafe(hash, common.CopyBytes(buf))
}

// decodeNodeUnsafe parses the RLP encoding of a trie node. The passed byte slice
// will be directly referenced by node without bytes deep copy, so the input MUST
// not be changed after.
func decodeNodeUnsafe(hash, buf []byte) (node, error) {
if len(buf) == 0 {
return nil, io.ErrUnexpectedEOF
}
Expand Down Expand Up @@ -163,7 +141,7 @@ func decodeShort(hash, elems []byte) (node, error) {
if err != nil {
return nil, fmt.Errorf("invalid value node: %v", err)
}
return &shortNode{key, valueNode(val), flag}, nil
return &shortNode{key, append(valueNode{}, val...), flag}, nil
}
r, _, err := decodeRef(rest)
if err != nil {
Expand All @@ -186,7 +164,7 @@ func decodeFull(hash, elems []byte) (*fullNode, error) {
return n, err
}
if len(val) > 0 {
n.Children[16] = valueNode(val)
n.Children[16] = append(valueNode{}, val...)
}
return n, nil
}
Expand All @@ -212,7 +190,7 @@ func decodeRef(buf []byte) (node, []byte, error) {
// empty node
return nil, rest, nil
case kind == rlp.String && len(val) == 32:
return hashNode(val), rest, nil
return append(hashNode{}, val...), rest, nil
default:
return nil, nil, fmt.Errorf("invalid RLP string size %d (want 0 or 32)", len(val))
}
Expand Down
121 changes: 0 additions & 121 deletions trie/node_test.go
View file
Expand Up @@ -20,7 +20,6 @@ import (
"bytes"
"testing"

"github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/rlp"
)

Expand Down Expand Up @@ -93,123 +92,3 @@ func TestDecodeFullNode(t *testing.T) {
t.Fatalf("decode full node err: %v", err)
}
}

// goos: darwin
// goarch: arm64
// pkg: github.com/ethereum/go-ethereum/trie
// BenchmarkEncodeShortNode
// BenchmarkEncodeShortNode-8 16878850 70.81 ns/op 48 B/op 1 allocs/op
func BenchmarkEncodeShortNode(b *testing.B) {
node := &shortNode{
Key: []byte{0x1, 0x2},
Val: hashNode(randBytes(32)),
}
b.ResetTimer()
b.ReportAllocs()

for i := 0; i < b.N; i++ {
nodeToBytes(node)
}
}

// goos: darwin
// goarch: arm64
// pkg: github.com/ethereum/go-ethereum/trie
// BenchmarkEncodeFullNode
// BenchmarkEncodeFullNode-8 4323273 284.4 ns/op 576 B/op 1 allocs/op
func BenchmarkEncodeFullNode(b *testing.B) {
node := &fullNode{}
for i := 0; i < 16; i++ {
node.Children[i] = hashNode(randBytes(32))
}
b.ResetTimer()
b.ReportAllocs()

for i := 0; i < b.N; i++ {
nodeToBytes(node)
}
}

// goos: darwin
// goarch: arm64
// pkg: github.com/ethereum/go-ethereum/trie
// BenchmarkDecodeShortNode
// BenchmarkDecodeShortNode-8 7925638 151.0 ns/op 157 B/op 4 allocs/op
func BenchmarkDecodeShortNode(b *testing.B) {
node := &shortNode{
Key: []byte{0x1, 0x2},
Val: hashNode(randBytes(32)),
}
blob := nodeToBytes(node)
hash := crypto.Keccak256(blob)

b.ResetTimer()
b.ReportAllocs()

for i := 0; i < b.N; i++ {
mustDecodeNode(hash, blob)
}
}

// goos: darwin
// goarch: arm64
// pkg: github.com/ethereum/go-ethereum/trie
// BenchmarkDecodeShortNodeUnsafe
// BenchmarkDecodeShortNodeUnsafe-8 9027476 128.6 ns/op 109 B/op 3 allocs/op
func BenchmarkDecodeShortNodeUnsafe(b *testing.B) {
node := &shortNode{
Key: []byte{0x1, 0x2},
Val: hashNode(randBytes(32)),
}
blob := nodeToBytes(node)
hash := crypto.Keccak256(blob)

b.ResetTimer()
b.ReportAllocs()

for i := 0; i < b.N; i++ {
mustDecodeNodeUnsafe(hash, blob)
}
}

// goos: darwin
// goarch: arm64
// pkg: github.com/ethereum/go-ethereum/trie
// BenchmarkDecodeFullNode
// BenchmarkDecodeFullNode-8 1597462 761.9 ns/op 1280 B/op 18 allocs/op
func BenchmarkDecodeFullNode(b *testing.B) {
node := &fullNode{}
for i := 0; i < 16; i++ {
node.Children[i] = hashNode(randBytes(32))
}
blob := nodeToBytes(node)
hash := crypto.Keccak256(blob)

b.ResetTimer()
b.ReportAllocs()

for i := 0; i < b.N; i++ {
mustDecodeNode(hash, blob)
}
}

// goos: darwin
// goarch: arm64
// pkg: github.com/ethereum/go-ethereum/trie
// BenchmarkDecodeFullNodeUnsafe
// BenchmarkDecodeFullNodeUnsafe-8 1789070 687.1 ns/op 704 B/op 17 allocs/op
func BenchmarkDecodeFullNodeUnsafe(b *testing.B) {
node := &fullNode{}
for i := 0; i < 16; i++ {
node.Children[i] = hashNode(randBytes(32))
}
blob := nodeToBytes(node)
hash := crypto.Keccak256(blob)

b.ResetTimer()
b.ReportAllocs()

for i := 0; i < b.N; i++ {
mustDecodeNodeUnsafe(hash, blob)
}
}

0 comments on commit b9606ab

Please sign in to comment.