Skip to content

Commit 856780b

Browse files
qdm12qdm12
authored
fix(trie): no in-memory caching of node encoding (#2919)
1 parent d769d1c commit 856780b

File tree

13 files changed

+40
-306
lines changed

13 files changed

+40
-306
lines changed

‎internal/trie/node/copy.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,6 @@ func (n *Node) Copy(settings CopySettings) *Node {
9999
cpy.MerkleValue = make([]byte, len(n.MerkleValue))
100100
copy(cpy.MerkleValue, n.MerkleValue)
101101
}
102-
103-
if n.Encoding != nil {
104-
cpy.Encoding = make([]byte, len(n.Encoding))
105-
copy(cpy.Encoding, n.Encoding)
106-
}
107102
}
108103

109104
return cpy

‎internal/trie/node/copy_test.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ func Test_Node_Copy(t *testing.T) {
4848
}),
4949
Dirty: true,
5050
MerkleValue: []byte{5},
51-
Encoding: []byte{6},
5251
},
5352
settings: DefaultCopySettings,
5453
expectedNode: &Node{
@@ -96,7 +95,6 @@ func Test_Node_Copy(t *testing.T) {
9695
}),
9796
Dirty: true,
9897
MerkleValue: []byte{5},
99-
Encoding: []byte{6},
10098
},
10199
settings: DeepCopySettings,
102100
expectedNode: &Node{
@@ -110,7 +108,6 @@ func Test_Node_Copy(t *testing.T) {
110108
}),
111109
Dirty: true,
112110
MerkleValue: []byte{5},
113-
Encoding: []byte{6},
114111
},
115112
},
116113
"non empty leaf": {
@@ -119,7 +116,6 @@ func Test_Node_Copy(t *testing.T) {
119116
SubValue: []byte{3, 4},
120117
Dirty: true,
121118
MerkleValue: []byte{5},
122-
Encoding: []byte{6},
123119
},
124120
settings: DefaultCopySettings,
125121
expectedNode: &Node{
@@ -134,15 +130,13 @@ func Test_Node_Copy(t *testing.T) {
134130
SubValue: []byte{3, 4},
135131
Dirty: true,
136132
MerkleValue: []byte{5},
137-
Encoding: []byte{6},
138133
},
139134
settings: DeepCopySettings,
140135
expectedNode: &Node{
141136
Key: []byte{1, 2},
142137
SubValue: []byte{3, 4},
143138
Dirty: true,
144139
MerkleValue: []byte{5},
145-
Encoding: []byte{6},
146140
},
147141
},
148142
}
@@ -158,7 +152,6 @@ func Test_Node_Copy(t *testing.T) {
158152
testForSliceModif(t, testCase.node.Key, nodeCopy.Key)
159153
testForSliceModif(t, testCase.node.SubValue, nodeCopy.SubValue)
160154
testForSliceModif(t, testCase.node.MerkleValue, nodeCopy.MerkleValue)
161-
testForSliceModif(t, testCase.node.Encoding, nodeCopy.Encoding)
162155

163156
if testCase.node.Kind() == Branch {
164157
testCase.node.Children[15] = &Node{Key: []byte("modified")}

‎internal/trie/node/dirty.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,7 @@ package node
77
func (n *Node) SetDirty() {
88
n.Dirty = true
99
// A node is marked dirty if its key or value is modified.
10-
// This means its cached encoding and hash fields are no longer
11-
// valid. To improve memory usage, we clear these fields.
12-
n.Encoding = nil
10+
// This means its Merkle value field is no longer valid.
1311
n.MerkleValue = nil
1412
}
1513

‎internal/trie/node/dirty_test.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,12 @@ func Test_Node_SetDirty(t *testing.T) {
1818
}{
1919
"not dirty to dirty": {
2020
node: Node{
21-
Encoding: []byte{1},
2221
MerkleValue: []byte{1},
2322
},
2423
expected: Node{Dirty: true},
2524
},
2625
"dirty to dirty": {
2726
node: Node{
28-
Encoding: []byte{1},
2927
MerkleValue: []byte{1},
3028
Dirty: true,
3129
},
@@ -54,22 +52,18 @@ func Test_Node_SetClean(t *testing.T) {
5452
}{
5553
"not dirty to not dirty": {
5654
node: Node{
57-
Encoding: []byte{1},
5855
MerkleValue: []byte{1},
5956
},
6057
expected: Node{
61-
Encoding: []byte{1},
6258
MerkleValue: []byte{1},
6359
},
6460
},
6561
"dirty to not dirty": {
6662
node: Node{
67-
Encoding: []byte{1},
6863
MerkleValue: []byte{1},
6964
Dirty: true,
7065
},
7166
expected: Node{
72-
Encoding: []byte{1},
7367
MerkleValue: []byte{1},
7468
},
7569
},

‎internal/trie/node/encode.go

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,6 @@ import (
1616
// of this package, and specified in the Polkadot spec at
1717
// https://spec.polkadot.network/#sect-state-storage
1818
func (n *Node) Encode(buffer Buffer) (err error) {
19-
if !n.Dirty && n.Encoding != nil {
20-
_, err = buffer.Write(n.Encoding)
21-
if err != nil {
22-
return fmt.Errorf("cannot write stored encoding to buffer: %w", err)
23-
}
24-
return nil
25-
}
26-
2719
err = encodeHeader(n, buffer)
2820
if err != nil {
2921
return fmt.Errorf("cannot encode header: %w", err)
@@ -66,13 +58,5 @@ func (n *Node) Encode(buffer Buffer) (err error) {
6658
}
6759
}
6860

69-
if kind == Leaf {
70-
// TODO cache this for branches too and update test cases.
71-
// TODO remove this copying since it defeats the purpose of `buffer`
72-
// and the sync.Pool.
73-
n.Encoding = make([]byte, buffer.Len())
74-
copy(n.Encoding, buffer.Bytes())
75-
}
76-
7761
return nil
7862
}

‎internal/trie/node/encode_test.go

Lines changed: 0 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -26,37 +26,10 @@ func Test_Node_Encode(t *testing.T) {
2626
testCases := map[string]struct {
2727
node *Node
2828
writes []writeCall
29-
bufferLenCall bool
30-
bufferBytesCall bool
3129
expectedEncoding []byte
3230
wrappedErr error
3331
errMessage string
3432
}{
35-
"clean leaf with encoding": {
36-
node: &Node{
37-
Encoding: []byte{1, 2, 3},
38-
},
39-
writes: []writeCall{
40-
{
41-
written: []byte{1, 2, 3},
42-
},
43-
},
44-
expectedEncoding: []byte{1, 2, 3},
45-
},
46-
"write error for clean leaf with encoding": {
47-
node: &Node{
48-
Encoding: []byte{1, 2, 3},
49-
},
50-
writes: []writeCall{
51-
{
52-
written: []byte{1, 2, 3},
53-
err: errTest,
54-
},
55-
},
56-
expectedEncoding: []byte{1, 2, 3},
57-
wrappedErr: errTest,
58-
errMessage: "cannot write stored encoding to buffer: test error",
59-
},
6033
"leaf header encoding error": {
6134
node: &Node{
6235
Key: make([]byte, 1),
@@ -123,8 +96,6 @@ func Test_Node_Encode(t *testing.T) {
12396
written: []byte{12, 4, 5, 6},
12497
},
12598
},
126-
bufferLenCall: true,
127-
bufferBytesCall: true,
12899
expectedEncoding: []byte{1, 2, 3},
129100
},
130101
"leaf with empty value success": {
@@ -142,35 +113,8 @@ func Test_Node_Encode(t *testing.T) {
142113
written: []byte{0},
143114
},
144115
},
145-
bufferLenCall: true,
146-
bufferBytesCall: true,
147116
expectedEncoding: []byte{1, 2, 3},
148117
},
149-
"clean branch with encoding": {
150-
node: &Node{
151-
Children: make([]*Node, ChildrenCapacity),
152-
Encoding: []byte{1, 2, 3},
153-
},
154-
writes: []writeCall{
155-
{ // stored encoding
156-
written: []byte{1, 2, 3},
157-
},
158-
},
159-
},
160-
"write error for clean branch with encoding": {
161-
node: &Node{
162-
Children: make([]*Node, ChildrenCapacity),
163-
Encoding: []byte{1, 2, 3},
164-
},
165-
writes: []writeCall{
166-
{ // stored encoding
167-
written: []byte{1, 2, 3},
168-
err: errTest,
169-
},
170-
},
171-
wrappedErr: errTest,
172-
errMessage: "cannot write stored encoding to buffer: test error",
173-
},
174118
"branch header encoding error": {
175119
node: &Node{
176120
Children: make([]*Node, ChildrenCapacity),
@@ -362,12 +306,6 @@ func Test_Node_Encode(t *testing.T) {
362306
}
363307
previousCall = call
364308
}
365-
if testCase.bufferLenCall {
366-
buffer.EXPECT().Len().Return(len(testCase.expectedEncoding))
367-
}
368-
if testCase.bufferBytesCall {
369-
buffer.EXPECT().Bytes().Return(testCase.expectedEncoding)
370-
}
371309

372310
err := testCase.node.Encode(buffer)
373311

‎internal/trie/node/hash.go

Lines changed: 6 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -90,14 +90,12 @@ func (n *Node) CalculateRootMerkleValue() (merkleValue []byte, err error) {
9090
// and a merkle value writer, such that buffer sync pools can be used
9191
// by the caller.
9292
func (n *Node) EncodeAndHash() (encoding, merkleValue []byte, err error) {
93-
if !n.Dirty && n.Encoding != nil && n.MerkleValue != nil {
94-
return n.Encoding, n.MerkleValue, nil
95-
}
96-
97-
encoding, err = n.encodeIfNeeded()
93+
encodingBuffer := bytes.NewBuffer(nil)
94+
err = n.Encode(encodingBuffer)
9895
if err != nil {
9996
return nil, nil, fmt.Errorf("encoding node: %w", err)
10097
}
98+
encoding = encodingBuffer.Bytes()
10199

102100
const maxMerkleValueSize = 32
103101
merkleValueBuffer := bytes.NewBuffer(make([]byte, 0, maxMerkleValueSize))
@@ -118,15 +116,12 @@ func (n *Node) EncodeAndHash() (encoding, merkleValue []byte, err error) {
118116
// and a merkle value writer, such that buffer sync pools can be used
119117
// by the caller.
120118
func (n *Node) EncodeAndHashRoot() (encoding, merkleValue []byte, err error) {
121-
const rootMerkleValueLength = 32
122-
if !n.Dirty && n.Encoding != nil && len(n.MerkleValue) == rootMerkleValueLength {
123-
return n.Encoding, n.MerkleValue, nil
124-
}
125-
126-
encoding, err = n.encodeIfNeeded()
119+
encodingBuffer := bytes.NewBuffer(nil)
120+
err = n.Encode(encodingBuffer)
127121
if err != nil {
128122
return nil, nil, fmt.Errorf("encoding node: %w", err)
129123
}
124+
encoding = encodingBuffer.Bytes()
130125

131126
const merkleValueSize = 32
132127
merkleValueBuffer := bytes.NewBuffer(make([]byte, 0, merkleValueSize))
@@ -139,19 +134,3 @@ func (n *Node) EncodeAndHashRoot() (encoding, merkleValue []byte, err error) {
139134

140135
return encoding, merkleValue, nil
141136
}
142-
143-
func (n *Node) encodeIfNeeded() (encoding []byte, err error) {
144-
if !n.Dirty && n.Encoding != nil {
145-
return n.Encoding, nil // no need to copy
146-
}
147-
148-
buffer := bytes.NewBuffer(nil)
149-
err = n.Encode(buffer)
150-
if err != nil {
151-
return nil, fmt.Errorf("encoding: %w", err)
152-
}
153-
154-
n.Encoding = buffer.Bytes()
155-
156-
return n.Encoding, nil // no need to copy
157-
}

0 commit comments

Comments
 (0)