Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

trie: no in-memory caching of node encoding #2919

Merged
merged 1 commit into from
Nov 11, 2022
Merged

trie: no in-memory caching of node encoding #2919

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions internal/trie/node/copy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,6 @@ func (n *Node) Copy(settings CopySettings) *Node {
cpy.MerkleValue = make([]byte, len(n.MerkleValue))
copy(cpy.MerkleValue, n.MerkleValue)
}

if n.Encoding != nil {
cpy.Encoding = make([]byte, len(n.Encoding))
copy(cpy.Encoding, n.Encoding)
}
}

return cpy
Expand Down
7 changes: 0 additions & 7 deletions internal/trie/node/copy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ func Test_Node_Copy(t *testing.T) {
}),
Dirty: true,
MerkleValue: []byte{5},
Encoding: []byte{6},
},
settings: DefaultCopySettings,
expectedNode: &Node{
Expand Down Expand Up @@ -96,7 +95,6 @@ func Test_Node_Copy(t *testing.T) {
}),
Dirty: true,
MerkleValue: []byte{5},
Encoding: []byte{6},
},
settings: DeepCopySettings,
expectedNode: &Node{
Expand All @@ -110,7 +108,6 @@ func Test_Node_Copy(t *testing.T) {
}),
Dirty: true,
MerkleValue: []byte{5},
Encoding: []byte{6},
},
},
"non empty leaf": {
Expand All @@ -119,7 +116,6 @@ func Test_Node_Copy(t *testing.T) {
SubValue: []byte{3, 4},
Dirty: true,
MerkleValue: []byte{5},
Encoding: []byte{6},
},
settings: DefaultCopySettings,
expectedNode: &Node{
Expand All @@ -134,15 +130,13 @@ func Test_Node_Copy(t *testing.T) {
SubValue: []byte{3, 4},
Dirty: true,
MerkleValue: []byte{5},
Encoding: []byte{6},
},
settings: DeepCopySettings,
expectedNode: &Node{
Key: []byte{1, 2},
SubValue: []byte{3, 4},
Dirty: true,
MerkleValue: []byte{5},
Encoding: []byte{6},
},
},
}
Expand All @@ -158,7 +152,6 @@ func Test_Node_Copy(t *testing.T) {
testForSliceModif(t, testCase.node.Key, nodeCopy.Key)
testForSliceModif(t, testCase.node.SubValue, nodeCopy.SubValue)
testForSliceModif(t, testCase.node.MerkleValue, nodeCopy.MerkleValue)
testForSliceModif(t, testCase.node.Encoding, nodeCopy.Encoding)

if testCase.node.Kind() == Branch {
testCase.node.Children[15] = &Node{Key: []byte("modified")}
Expand Down
4 changes: 1 addition & 3 deletions internal/trie/node/dirty.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@ package node
func (n *Node) SetDirty() {
n.Dirty = true
// A node is marked dirty if its key or value is modified.
// This means its cached encoding and hash fields are no longer
// valid. To improve memory usage, we clear these fields.
n.Encoding = nil
// This means its Merkle value field is no longer valid.
n.MerkleValue = nil
}

Expand Down
6 changes: 0 additions & 6 deletions internal/trie/node/dirty_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,12 @@ func Test_Node_SetDirty(t *testing.T) {
}{
"not dirty to dirty": {
node: Node{
Encoding: []byte{1},
MerkleValue: []byte{1},
},
expected: Node{Dirty: true},
},
"dirty to dirty": {
node: Node{
Encoding: []byte{1},
MerkleValue: []byte{1},
Dirty: true,
},
Expand Down Expand Up @@ -54,22 +52,18 @@ func Test_Node_SetClean(t *testing.T) {
}{
"not dirty to not dirty": {
node: Node{
Encoding: []byte{1},
MerkleValue: []byte{1},
},
expected: Node{
Encoding: []byte{1},
MerkleValue: []byte{1},
},
},
"dirty to not dirty": {
node: Node{
Encoding: []byte{1},
MerkleValue: []byte{1},
Dirty: true,
},
expected: Node{
Encoding: []byte{1},
MerkleValue: []byte{1},
},
},
Expand Down
16 changes: 0 additions & 16 deletions internal/trie/node/encode.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,6 @@ import (
// of this package, and specified in the Polkadot spec at
// https://spec.polkadot.network/#sect-state-storage
func (n *Node) Encode(buffer Buffer) (err error) {
if !n.Dirty && n.Encoding != nil {
_, err = buffer.Write(n.Encoding)
if err != nil {
return fmt.Errorf("cannot write stored encoding to buffer: %w", err)
}
return nil
}

err = encodeHeader(n, buffer)
if err != nil {
return fmt.Errorf("cannot encode header: %w", err)
Expand Down Expand Up @@ -66,13 +58,5 @@ func (n *Node) Encode(buffer Buffer) (err error) {
}
}

if kind == Leaf {
// TODO cache this for branches too and update test cases.
// TODO remove this copying since it defeats the purpose of `buffer`
// and the sync.Pool.
n.Encoding = make([]byte, buffer.Len())
copy(n.Encoding, buffer.Bytes())
}

return nil
}
62 changes: 0 additions & 62 deletions internal/trie/node/encode_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,37 +26,10 @@ func Test_Node_Encode(t *testing.T) {
testCases := map[string]struct {
node *Node
writes []writeCall
bufferLenCall bool
bufferBytesCall bool
expectedEncoding []byte
wrappedErr error
errMessage string
}{
"clean leaf with encoding": {
node: &Node{
Encoding: []byte{1, 2, 3},
},
writes: []writeCall{
{
written: []byte{1, 2, 3},
},
},
expectedEncoding: []byte{1, 2, 3},
},
"write error for clean leaf with encoding": {
node: &Node{
Encoding: []byte{1, 2, 3},
},
writes: []writeCall{
{
written: []byte{1, 2, 3},
err: errTest,
},
},
expectedEncoding: []byte{1, 2, 3},
wrappedErr: errTest,
errMessage: "cannot write stored encoding to buffer: test error",
},
"leaf header encoding error": {
node: &Node{
Key: make([]byte, 1),
Expand Down Expand Up @@ -123,8 +96,6 @@ func Test_Node_Encode(t *testing.T) {
written: []byte{12, 4, 5, 6},
},
},
bufferLenCall: true,
bufferBytesCall: true,
expectedEncoding: []byte{1, 2, 3},
},
"leaf with empty value success": {
Expand All @@ -142,35 +113,8 @@ func Test_Node_Encode(t *testing.T) {
written: []byte{0},
},
},
bufferLenCall: true,
bufferBytesCall: true,
expectedEncoding: []byte{1, 2, 3},
},
"clean branch with encoding": {
node: &Node{
Children: make([]*Node, ChildrenCapacity),
Encoding: []byte{1, 2, 3},
},
writes: []writeCall{
{ // stored encoding
written: []byte{1, 2, 3},
},
},
},
"write error for clean branch with encoding": {
node: &Node{
Children: make([]*Node, ChildrenCapacity),
Encoding: []byte{1, 2, 3},
},
writes: []writeCall{
{ // stored encoding
written: []byte{1, 2, 3},
err: errTest,
},
},
wrappedErr: errTest,
errMessage: "cannot write stored encoding to buffer: test error",
},
"branch header encoding error": {
node: &Node{
Children: make([]*Node, ChildrenCapacity),
Expand Down Expand Up @@ -362,12 +306,6 @@ func Test_Node_Encode(t *testing.T) {
}
previousCall = call
}
if testCase.bufferLenCall {
buffer.EXPECT().Len().Return(len(testCase.expectedEncoding))
}
if testCase.bufferBytesCall {
buffer.EXPECT().Bytes().Return(testCase.expectedEncoding)
}

err := testCase.node.Encode(buffer)

Expand Down
33 changes: 6 additions & 27 deletions internal/trie/node/hash.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,14 +90,12 @@ func (n *Node) CalculateRootMerkleValue() (merkleValue []byte, err error) {
// and a merkle value writer, such that buffer sync pools can be used
// by the caller.
func (n *Node) EncodeAndHash() (encoding, merkleValue []byte, err error) {
if !n.Dirty && n.Encoding != nil && n.MerkleValue != nil {
return n.Encoding, n.MerkleValue, nil
}

encoding, err = n.encodeIfNeeded()
encodingBuffer := bytes.NewBuffer(nil)
err = n.Encode(encodingBuffer)
if err != nil {
return nil, nil, fmt.Errorf("encoding node: %w", err)
}
encoding = encodingBuffer.Bytes()

const maxMerkleValueSize = 32
merkleValueBuffer := bytes.NewBuffer(make([]byte, 0, maxMerkleValueSize))
Expand All @@ -118,15 +116,12 @@ func (n *Node) EncodeAndHash() (encoding, merkleValue []byte, err error) {
// and a merkle value writer, such that buffer sync pools can be used
// by the caller.
func (n *Node) EncodeAndHashRoot() (encoding, merkleValue []byte, err error) {
const rootMerkleValueLength = 32
if !n.Dirty && n.Encoding != nil && len(n.MerkleValue) == rootMerkleValueLength {
return n.Encoding, n.MerkleValue, nil
}

encoding, err = n.encodeIfNeeded()
encodingBuffer := bytes.NewBuffer(nil)
err = n.Encode(encodingBuffer)
if err != nil {
return nil, nil, fmt.Errorf("encoding node: %w", err)
}
encoding = encodingBuffer.Bytes()

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

return encoding, merkleValue, nil
}

func (n *Node) encodeIfNeeded() (encoding []byte, err error) {
if !n.Dirty && n.Encoding != nil {
return n.Encoding, nil // no need to copy
}

buffer := bytes.NewBuffer(nil)
err = n.Encode(buffer)
if err != nil {
return nil, fmt.Errorf("encoding: %w", err)
}

n.Encoding = buffer.Bytes()

return n.Encoding, nil // no need to copy
}
Loading