Skip to content

Commit bd6d8e4

Browse files
qdm12qdm12
authored
feat(trie): finer deep copy of nodes (#2384)
- Do not copy cached fields when not needed - Do not copy key field when not needed - Do not copy value field when not needed
1 parent 50652e9 commit bd6d8e4

File tree

5 files changed

+182
-60
lines changed

5 files changed

+182
-60
lines changed

‎internal/trie/node/copy.go

Lines changed: 80 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,76 +3,134 @@
33

44
package node
55

6+
var (
7+
// DefaultCopySettings contains the following copy settings:
8+
// - children are NOT deep copied recursively
9+
// - the HashDigest field is left empty on the copy
10+
// - the Encoding field is left empty on the copy
11+
// - the key field is deep copied
12+
// - the value field is deep copied
13+
DefaultCopySettings = CopySettings{
14+
CopyKey: true,
15+
CopyValue: true,
16+
}
17+
18+
// DeepCopySettings returns the following copy settings:
19+
// - children are deep copied recursively
20+
// - the HashDigest field is deep copied
21+
// - the Encoding field is deep copied
22+
// - the key field is deep copied
23+
// - the value field is deep copied
24+
DeepCopySettings = CopySettings{
25+
CopyChildren: true,
26+
CopyCached: true,
27+
CopyKey: true,
28+
CopyValue: true,
29+
}
30+
)
31+
32+
// CopySettings contains settings to configure the deep copy
33+
// of a node.
34+
type CopySettings struct {
35+
// CopyChildren can be set to true to recursively deep copy the eventual
36+
// children of the node. This is false by default and should only be used
37+
// in tests since it is quite inefficient.
38+
CopyChildren bool
39+
// CopyCached can be set to true to deep copy the cached digest
40+
// and encoding fields on the current node copied.
41+
// This is false by default because in production, a node is copied
42+
// when it is about to be mutated, hence making its cached fields
43+
// no longer valid.
44+
CopyCached bool
45+
// CopyKey can be set to true to deep copy the key field of
46+
// the node. This is useful when false if the key is about to
47+
// be assigned after the Copy operation, to save a memory operation.
48+
CopyKey bool
49+
// CopyValue can be set to true to deep copy the value field of
50+
// the node. This is useful when false if the value is about to
51+
// be assigned after the Copy operation, to save a memory operation.
52+
CopyValue bool
53+
}
54+
655
// Copy deep copies the branch.
756
// Setting copyChildren to true will deep copy
857
// children as well.
9-
func (b *Branch) Copy(copyChildren bool) Node {
58+
func (b *Branch) Copy(settings CopySettings) Node {
1059
cpy := &Branch{
1160
Dirty: b.Dirty,
1261
Generation: b.Generation,
1362
}
1463

15-
if copyChildren {
64+
if settings.CopyChildren {
65+
// Copy all fields of children if we deep copy children
66+
childSettings := settings
67+
childSettings.CopyKey = true
68+
childSettings.CopyValue = true
69+
childSettings.CopyCached = true
1670
for i, child := range b.Children {
1771
if child == nil {
1872
continue
1973
}
20-
cpy.Children[i] = child.Copy(copyChildren)
74+
cpy.Children[i] = child.Copy(childSettings)
2175
}
2276
} else {
2377
cpy.Children = b.Children // copy interface pointers only
2478
}
2579

26-
if b.Key != nil {
80+
if settings.CopyKey && b.Key != nil {
2781
cpy.Key = make([]byte, len(b.Key))
2882
copy(cpy.Key, b.Key)
2983
}
3084

3185
// nil and []byte{} are encoded differently, watch out!
32-
if b.Value != nil {
86+
if settings.CopyValue && b.Value != nil {
3387
cpy.Value = make([]byte, len(b.Value))
3488
copy(cpy.Value, b.Value)
3589
}
3690

37-
if b.HashDigest != nil {
38-
cpy.HashDigest = make([]byte, len(b.HashDigest))
39-
copy(cpy.HashDigest, b.HashDigest)
40-
}
91+
if settings.CopyCached {
92+
if b.HashDigest != nil {
93+
cpy.HashDigest = make([]byte, len(b.HashDigest))
94+
copy(cpy.HashDigest, b.HashDigest)
95+
}
4196

42-
if b.Encoding != nil {
43-
cpy.Encoding = make([]byte, len(b.Encoding))
44-
copy(cpy.Encoding, b.Encoding)
97+
if b.Encoding != nil {
98+
cpy.Encoding = make([]byte, len(b.Encoding))
99+
copy(cpy.Encoding, b.Encoding)
100+
}
45101
}
46102

47103
return cpy
48104
}
49105

50106
// Copy deep copies the leaf.
51-
func (l *Leaf) Copy(_ bool) Node {
107+
func (l *Leaf) Copy(settings CopySettings) Node {
52108
cpy := &Leaf{
53109
Dirty: l.Dirty,
54110
Generation: l.Generation,
55111
}
56112

57-
if l.Key != nil {
113+
if settings.CopyKey && l.Key != nil {
58114
cpy.Key = make([]byte, len(l.Key))
59115
copy(cpy.Key, l.Key)
60116
}
61117

62118
// nil and []byte{} are encoded differently, watch out!
63-
if l.Value != nil {
119+
if settings.CopyValue && l.Value != nil {
64120
cpy.Value = make([]byte, len(l.Value))
65121
copy(cpy.Value, l.Value)
66122
}
67123

68-
if l.HashDigest != nil {
69-
cpy.HashDigest = make([]byte, len(l.HashDigest))
70-
copy(cpy.HashDigest, l.HashDigest)
71-
}
124+
if settings.CopyCached {
125+
if l.HashDigest != nil {
126+
cpy.HashDigest = make([]byte, len(l.HashDigest))
127+
copy(cpy.HashDigest, l.HashDigest)
128+
}
72129

73-
if l.Encoding != nil {
74-
cpy.Encoding = make([]byte, len(l.Encoding))
75-
copy(cpy.Encoding, l.Encoding)
130+
if l.Encoding != nil {
131+
cpy.Encoding = make([]byte, len(l.Encoding))
132+
copy(cpy.Encoding, l.Encoding)
133+
}
76134
}
77135

78136
return cpy

‎internal/trie/node/copy_test.go

Lines changed: 51 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package node
55

66
import (
7+
"reflect"
78
"testing"
89

910
"github.com/stretchr/testify/assert"
@@ -12,8 +13,7 @@ import (
1213

1314
func testForSliceModif(t *testing.T, original, copied []byte) {
1415
t.Helper()
15-
require.Equal(t, len(original), len(copied))
16-
if len(copied) == 0 {
16+
if !reflect.DeepEqual(original, copied) || len(copied) == 0 {
1717
// cannot test for modification
1818
return
1919
}
@@ -26,7 +26,7 @@ func Test_Branch_Copy(t *testing.T) {
2626

2727
testCases := map[string]struct {
2828
branch *Branch
29-
copyChildren bool
29+
settings CopySettings
3030
expectedBranch *Branch
3131
}{
3232
"empty branch": {
@@ -44,15 +44,14 @@ func Test_Branch_Copy(t *testing.T) {
4444
HashDigest: []byte{5},
4545
Encoding: []byte{6},
4646
},
47+
settings: DefaultCopySettings,
4748
expectedBranch: &Branch{
4849
Key: []byte{1, 2},
4950
Value: []byte{3, 4},
5051
Children: [16]Node{
5152
nil, nil, &Leaf{Key: []byte{9}},
5253
},
53-
Dirty: true,
54-
HashDigest: []byte{5},
55-
Encoding: []byte{6},
54+
Dirty: true,
5655
},
5756
},
5857
"branch with children copied": {
@@ -61,21 +60,46 @@ func Test_Branch_Copy(t *testing.T) {
6160
nil, nil, &Leaf{Key: []byte{9}},
6261
},
6362
},
64-
copyChildren: true,
63+
settings: CopySettings{
64+
CopyChildren: true,
65+
},
6566
expectedBranch: &Branch{
6667
Children: [16]Node{
6768
nil, nil, &Leaf{Key: []byte{9}},
6869
},
6970
},
7071
},
72+
"deep copy": {
73+
branch: &Branch{
74+
Key: []byte{1, 2},
75+
Value: []byte{3, 4},
76+
Children: [16]Node{
77+
nil, nil, &Leaf{Key: []byte{9}},
78+
},
79+
Dirty: true,
80+
HashDigest: []byte{5},
81+
Encoding: []byte{6},
82+
},
83+
settings: DeepCopySettings,
84+
expectedBranch: &Branch{
85+
Key: []byte{1, 2},
86+
Value: []byte{3, 4},
87+
Children: [16]Node{
88+
nil, nil, &Leaf{Key: []byte{9}},
89+
},
90+
Dirty: true,
91+
HashDigest: []byte{5},
92+
Encoding: []byte{6},
93+
},
94+
},
7195
}
7296

7397
for name, testCase := range testCases {
7498
testCase := testCase
7599
t.Run(name, func(t *testing.T) {
76100
t.Parallel()
77101

78-
nodeCopy := testCase.branch.Copy(testCase.copyChildren)
102+
nodeCopy := testCase.branch.Copy(testCase.settings)
79103

80104
branchCopy, ok := nodeCopy.(*Branch)
81105
require.True(t, ok)
@@ -97,10 +121,12 @@ func Test_Leaf_Copy(t *testing.T) {
97121

98122
testCases := map[string]struct {
99123
leaf *Leaf
124+
settings CopySettings
100125
expectedLeaf *Leaf
101126
}{
102127
"empty leaf": {
103128
leaf: &Leaf{},
129+
settings: DefaultCopySettings,
104130
expectedLeaf: &Leaf{},
105131
},
106132
"non empty leaf": {
@@ -111,6 +137,22 @@ func Test_Leaf_Copy(t *testing.T) {
111137
HashDigest: []byte{5},
112138
Encoding: []byte{6},
113139
},
140+
settings: DefaultCopySettings,
141+
expectedLeaf: &Leaf{
142+
Key: []byte{1, 2},
143+
Value: []byte{3, 4},
144+
Dirty: true,
145+
},
146+
},
147+
"deep copy": {
148+
leaf: &Leaf{
149+
Key: []byte{1, 2},
150+
Value: []byte{3, 4},
151+
Dirty: true,
152+
HashDigest: []byte{5},
153+
Encoding: []byte{6},
154+
},
155+
settings: DeepCopySettings,
114156
expectedLeaf: &Leaf{
115157
Key: []byte{1, 2},
116158
Value: []byte{3, 4},
@@ -126,8 +168,7 @@ func Test_Leaf_Copy(t *testing.T) {
126168
t.Run(name, func(t *testing.T) {
127169
t.Parallel()
128170

129-
const copyChildren = false
130-
nodeCopy := testCase.leaf.Copy(copyChildren)
171+
nodeCopy := testCase.leaf.Copy(testCase.settings)
131172

132173
leafCopy, ok := nodeCopy.(*Leaf)
133174
require.True(t, ok)

‎internal/trie/node/node.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ type Node interface {
2020
GetValue() (value []byte)
2121
GetGeneration() (generation uint64)
2222
SetGeneration(generation uint64)
23-
Copy(copyChildren bool) Node
23+
Copy(settings CopySettings) (cpy Node)
2424
Type() Type
2525
StringNode() (stringNode *gotree.Node)
2626
}

0 commit comments

Comments
 (0)