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

Optimize Ledger.Get() by making Forest.Read() use ~5x fewer allocs/op and run faster #2476

Closed
wants to merge 3 commits into from
Closed

Optimize Ledger.Get() by making Forest.Read() use ~5x fewer allocs/op and run faster #2476

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7cebaf5
Reduce allocs/op by 79% in ledger read
fxamacker May 24, 2022
10bd9aa
Fix tests
fxamacker May 24, 2022
f026cc3
Update Forest.Read() callers to use new API
fxamacker May 24, 2022
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
4 changes: 2 additions & 2 deletions cmd/util/cmd/read-execution-state/list-accounts/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,12 @@ func run(*cobra.Command, []string) {
},
}

payload, err := forest.Read(read)
values, err := forest.Read(read)
if err != nil {
return nil, err
}

return payload[0].Value, nil
return values[0], nil
})

sth := state.NewStateHolder(state.NewState(ldg))
Expand Down
6 changes: 1 addition & 5 deletions ledger/complete/ledger.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,11 +142,7 @@ func (l *Ledger) Get(query *ledger.Query) (values []ledger.Value, err error) {
return nil, err
}
trieRead := &ledger.TrieRead{RootHash: ledger.RootHash(query.State()), Paths: paths}
payloads, err := l.forest.Read(trieRead)
if err != nil {
return nil, err
}
values, err = pathfinder.PayloadsToValues(payloads)
values, err = l.forest.Read(trieRead)
if err != nil {
return nil, err
}
Expand Down
10 changes: 5 additions & 5 deletions ledger/complete/mtrie/forest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,10 @@ func (f *Forest) ValueSizes(r *ledger.TrieRead) ([]int, error) {

// Read reads values for an slice of paths and returns values and error (if any)
// TODO: can be optimized further if we don't care about changing the order of the input r.Paths
func (f *Forest) Read(r *ledger.TrieRead) ([]*ledger.Payload, error) {
func (f *Forest) Read(r *ledger.TrieRead) ([]ledger.Value, error) {

if len(r.Paths) == 0 {
return []*ledger.Payload{}, nil
return []ledger.Value{}, nil
}

// lookup the trie by rootHash
Expand Down Expand Up @@ -156,20 +156,20 @@ func (f *Forest) Read(r *ledger.TrieRead) ([]*ledger.Payload, error) {
payloads := trie.UnsafeRead(deduplicatedPaths) // this sorts deduplicatedPaths IN-PLACE

// reconstruct the payloads in the same key order that called the method
orderedPayloads := make([]*ledger.Payload, len(r.Paths))
orderedValues := make([]ledger.Value, len(r.Paths))
totalPayloadSize := 0
for i, p := range deduplicatedPaths {
payload := payloads[i]
indices := pathOrgIndex[p]
for _, j := range indices {
orderedPayloads[j] = payload.DeepCopy()
orderedValues[j] = payload.Value.DeepCopy()
}
totalPayloadSize += len(indices) * payload.Size()
}
// TODO rename the metrics
f.metrics.ReadValuesSize(uint64(totalPayloadSize))

return orderedPayloads, nil
return orderedValues, nil
}

// Update updates the Values for the registers and returns rootHash and error (if any).
Expand Down
103 changes: 51 additions & 52 deletions ledger/complete/mtrie/forest_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"github.com/stretchr/testify/require"

"github.com/onflow/flow-go/ledger"
"github.com/onflow/flow-go/ledger/common/encoding"
prf "github.com/onflow/flow-go/ledger/common/proof"
"github.com/onflow/flow-go/ledger/common/utils"
"github.com/onflow/flow-go/ledger/complete/mtrie/trie"
Expand Down Expand Up @@ -67,9 +66,9 @@ func TestTrieUpdate(t *testing.T) {
require.NoError(t, err)

read := &ledger.TrieRead{RootHash: updatedRoot, Paths: paths}
retPayloads, err := forest.Read(read)
retValues, err := forest.Read(read)
require.NoError(t, err)
require.True(t, bytes.Equal(encoding.EncodePayload(retPayloads[0]), encoding.EncodePayload(payloads[0])))
require.Equal(t, retValues[0], payloads[0].Value)
}

// TestLeftEmptyInsert tests inserting a new value into an empty sub-trie:
Expand Down Expand Up @@ -122,10 +121,10 @@ func TestLeftEmptyInsert(t *testing.T) {
paths = []ledger.Path{p1, p2, p3}
payloads = []*ledger.Payload{v1, v2, v3}
read := &ledger.TrieRead{RootHash: updatedRoot, Paths: paths}
retPayloads, err := forest.Read(read)
retValues, err := forest.Read(read)
require.NoError(t, err)
for i := range paths {
require.True(t, bytes.Equal(encoding.EncodePayload(retPayloads[i]), encoding.EncodePayload(payloads[i])))
require.Equal(t, retValues[i], payloads[i].Value)
}
}

Expand Down Expand Up @@ -180,10 +179,10 @@ func TestRightEmptyInsert(t *testing.T) {
paths = []ledger.Path{p1, p2, p3}
payloads = []*ledger.Payload{v1, v2, v3}
read := &ledger.TrieRead{RootHash: updatedRoot, Paths: paths}
retPayloads, err := forest.Read(read)
retValues, err := forest.Read(read)
require.NoError(t, err)
for i := range paths {
require.True(t, bytes.Equal(encoding.EncodePayload(retPayloads[i]), encoding.EncodePayload(payloads[i])))
require.Equal(t, retValues[i], payloads[i].Value)
}
}

Expand Down Expand Up @@ -236,10 +235,10 @@ func TestExpansionInsert(t *testing.T) {
paths = []ledger.Path{p1, p2}
payloads = []*ledger.Payload{v1, v2}
read := &ledger.TrieRead{RootHash: updatedRoot, Paths: paths}
retPayloads, err := forest.Read(read)
retValues, err := forest.Read(read)
require.NoError(t, err)
for i := range paths {
require.True(t, bytes.Equal(encoding.EncodePayload(retPayloads[i]), encoding.EncodePayload(payloads[i])))
require.Equal(t, retValues[i], payloads[i].Value)
}
}

Expand Down Expand Up @@ -304,10 +303,10 @@ func TestFullHouseInsert(t *testing.T) {
paths = []ledger.Path{p1, p2, p3}
payloads = []*ledger.Payload{v1, v2, v3}
read := &ledger.TrieRead{RootHash: updatedRoot, Paths: paths}
retPayloads, err := forest.Read(read)
retValues, err := forest.Read(read)
require.NoError(t, err)
for i := range paths {
require.True(t, bytes.Equal(encoding.EncodePayload(retPayloads[i]), encoding.EncodePayload(payloads[i])))
require.Equal(t, retValues[i], payloads[i].Value)
}
}

Expand Down Expand Up @@ -346,10 +345,10 @@ func TestLeafInsert(t *testing.T) {
require.Equal(t, uint64(v1.Size()+v2.Size()), updatedTrie.AllocatedRegSize())

read := &ledger.TrieRead{RootHash: updatedRoot, Paths: paths}
retPayloads, err := forest.Read(read)
retValues, err := forest.Read(read)
require.NoError(t, err)
for i := range paths {
require.True(t, bytes.Equal(encoding.EncodePayload(retPayloads[i]), encoding.EncodePayload(payloads[i])))
require.Equal(t, retValues[i], payloads[i].Value)
}
}

Expand Down Expand Up @@ -384,9 +383,9 @@ func TestOverrideValue(t *testing.T) {
require.NoError(t, err)

read := &ledger.TrieRead{RootHash: updatedRoot, Paths: paths}
retPayloads, err := forest.Read(read)
retValues, err := forest.Read(read)
require.NoError(t, err)
require.True(t, bytes.Equal(encoding.EncodePayload(retPayloads[0]), encoding.EncodePayload(payloads[0])))
require.Equal(t, retValues[0], payloads[0].Value)

}

Expand Down Expand Up @@ -417,9 +416,9 @@ func TestDuplicateOverride(t *testing.T) {

paths = []ledger.Path{p0}
read := &ledger.TrieRead{RootHash: updatedRoot, Paths: paths}
retPayloads, err := forest.Read(read)
retValues, err := forest.Read(read)
require.NoError(t, err)
require.True(t, bytes.Equal(encoding.EncodePayload(retPayloads[0]), encoding.EncodePayload(v2)))
require.Equal(t, retValues[0], v2.Value)

}

Expand All @@ -443,16 +442,16 @@ func TestReadSafety(t *testing.T) {
require.NoError(t, err)

require.Len(t, data, 1)
require.Equal(t, v0, data[0])
require.Equal(t, v0.Value, data[0])

// modify returned slice
data[0].Value = []byte("new value")
data[0] = []byte("new value")

// read again
data2, err := forest.Read(read)
require.NoError(t, err)
require.Len(t, data2, 1)
require.Equal(t, v0, data2[0])
require.Equal(t, v0.Value, data2[0])
}

// TestReadOrder tests that payloads from reading a trie are delivered in the order as specified by the paths
Expand All @@ -474,18 +473,18 @@ func TestReadOrder(t *testing.T) {
require.NoError(t, err)

read := &ledger.TrieRead{RootHash: testRoot, Paths: []ledger.Path{p1, p2}}
retPayloads, err := forest.Read(read)
retValues, err := forest.Read(read)
require.NoError(t, err)
require.Equal(t, len(read.Paths), len(retPayloads))
require.True(t, bytes.Equal(encoding.EncodePayload(retPayloads[0]), encoding.EncodePayload(payloads[0])))
require.True(t, bytes.Equal(encoding.EncodePayload(retPayloads[1]), encoding.EncodePayload(payloads[1])))
require.Equal(t, len(read.Paths), len(retValues))
require.Equal(t, retValues[0], payloads[0].Value)
require.Equal(t, retValues[1], payloads[1].Value)

read = &ledger.TrieRead{RootHash: testRoot, Paths: []ledger.Path{p2, p1}}
retPayloads, err = forest.Read(read)
retValues, err = forest.Read(read)
require.NoError(t, err)
require.Equal(t, len(read.Paths), len(retPayloads))
require.True(t, bytes.Equal(encoding.EncodePayload(retPayloads[1]), encoding.EncodePayload(payloads[0])))
require.True(t, bytes.Equal(encoding.EncodePayload(retPayloads[0]), encoding.EncodePayload(payloads[1])))
require.Equal(t, len(read.Paths), len(retValues))
require.Equal(t, retValues[1], payloads[0].Value)
require.Equal(t, retValues[0], payloads[1].Value)
}

// TestMixRead tests reading a mixture of set and unset registers.
Expand Down Expand Up @@ -521,10 +520,10 @@ func TestMixRead(t *testing.T) {
expectedPayloads := []*ledger.Payload{v1, v2, v3, v4}

read := &ledger.TrieRead{RootHash: baseRoot, Paths: readPaths}
retPayloads, err := forest.Read(read)
retValues, err := forest.Read(read)
require.NoError(t, err)
for i := range paths {
require.True(t, bytes.Equal(encoding.EncodePayload(retPayloads[i]), encoding.EncodePayload(expectedPayloads[i])))
require.Equal(t, retValues[i], expectedPayloads[i].Value)
}
}

Expand All @@ -549,11 +548,11 @@ func TestReadWithDuplicatedKeys(t *testing.T) {
paths = []ledger.Path{p1, p2, p3}
expectedPayloads := []*ledger.Payload{v1, v2, v1}
read := &ledger.TrieRead{RootHash: updatedRoot, Paths: paths}
retPayloads, err := forest.Read(read)
retValues, err := forest.Read(read)
require.NoError(t, err)
require.Equal(t, len(read.Paths), len(retPayloads))
require.Equal(t, len(read.Paths), len(retValues))
for i := range paths {
require.True(t, bytes.Equal(encoding.EncodePayload(retPayloads[i]), encoding.EncodePayload(expectedPayloads[i])))
require.Equal(t, retValues[i], expectedPayloads[i].Value)
}
}

Expand All @@ -573,9 +572,9 @@ func TestReadNonExistingPath(t *testing.T) {

p2 := pathByUint8s([]uint8{uint8(116), uint8(129)})
read := &ledger.TrieRead{RootHash: updatedRoot, Paths: []ledger.Path{p2}}
retPayloads, err := forest.Read(read)
retValues, err := forest.Read(read)
require.NoError(t, err)
require.True(t, retPayloads[0].IsEmpty())
require.Equal(t, 0, len(retValues[0]))
}

// TestForkingUpdates updates a base trie in two different ways. We expect
Expand Down Expand Up @@ -617,24 +616,24 @@ func TestForkingUpdates(t *testing.T) {

// Verify payloads are preserved
read := &ledger.TrieRead{RootHash: baseRoot, Paths: paths}
retPayloads, err := forest.Read(read) // reading from original Trie
retValues, err := forest.Read(read) // reading from original Trie
require.NoError(t, err)
for i := range paths {
require.True(t, bytes.Equal(encoding.EncodePayload(retPayloads[i]), encoding.EncodePayload(payloads[i])))
require.Equal(t, retValues[i], payloads[i].Value)
}

readA := &ledger.TrieRead{RootHash: updatedRootA, Paths: pathsA}
retPayloads, err = forest.Read(readA) // reading from updatedTrieA
retValues, err = forest.Read(readA) // reading from updatedTrieA
require.NoError(t, err)
for i := range paths {
require.True(t, bytes.Equal(encoding.EncodePayload(retPayloads[i]), encoding.EncodePayload(payloadsA[i])))
require.Equal(t, retValues[i], payloadsA[i].Value)
}

readB := &ledger.TrieRead{RootHash: updatedRootB, Paths: pathsB}
retPayloads, err = forest.Read(readB) // reading from updatedTrieB
retValues, err = forest.Read(readB) // reading from updatedTrieB
require.NoError(t, err)
for i := range paths {
require.True(t, bytes.Equal(encoding.EncodePayload(retPayloads[i]), encoding.EncodePayload(payloadsB[i])))
require.Equal(t, retValues[i], payloadsB[i].Value)
}
}

Expand Down Expand Up @@ -668,17 +667,17 @@ func TestIdenticalUpdateAppliedTwice(t *testing.T) {
paths = []ledger.Path{p1, p2, p3}
payloads = []*ledger.Payload{v1, v2, v3}
read := &ledger.TrieRead{RootHash: updatedRootA, Paths: paths}
retPayloadsA, err := forest.Read(read)
retValuesA, err := forest.Read(read)
require.NoError(t, err)
for i := range paths {
require.True(t, bytes.Equal(encoding.EncodePayload(retPayloadsA[i]), encoding.EncodePayload(payloads[i])))
require.Equal(t, retValuesA[i], payloads[i].Value)
}

read = &ledger.TrieRead{RootHash: updatedRootB, Paths: paths}
retPayloadsB, err := forest.Read(read)
retValuesB, err := forest.Read(read)
require.NoError(t, err)
for i := range paths {
require.True(t, bytes.Equal(encoding.EncodePayload(retPayloadsB[i]), encoding.EncodePayload(payloads[i])))
require.Equal(t, retValuesB[i], payloads[i].Value)
}
}

Expand Down Expand Up @@ -720,10 +719,10 @@ func TestRandomUpdateReadProofValueSizes(t *testing.T) {
}
}
read := &ledger.TrieRead{RootHash: activeRoot, Paths: nonExistingPaths}
retPayloads, err := forest.Read(read)
retValues, err := forest.Read(read)
require.NoError(t, err, "error reading - non existing paths")
for _, p := range retPayloads {
require.True(t, p.IsEmpty())
for _, p := range retValues {
require.Equal(t, 0, len(p))
}

// test value sizes for non-existent keys
Expand All @@ -741,10 +740,10 @@ func TestRandomUpdateReadProofValueSizes(t *testing.T) {

// test read
read = &ledger.TrieRead{RootHash: activeRoot, Paths: paths}
retPayloads, err = forest.Read(read)
retValues, err = forest.Read(read)
require.NoError(t, err, "error reading")
for i := range payloads {
require.True(t, bytes.Equal(encoding.EncodePayload(retPayloads[i]), encoding.EncodePayload(payloads[i])))
require.Equal(t, retValues[i], payloads[i].Value)
}

// test value sizes for existing keys
Expand Down Expand Up @@ -792,10 +791,10 @@ func TestRandomUpdateReadProofValueSizes(t *testing.T) {
}

read = &ledger.TrieRead{RootHash: activeRoot, Paths: allPaths}
retPayloads, err = forest.Read(read)
retValues, err = forest.Read(read)
require.NoError(t, err)
for i, v := range allPayloads {
assert.True(t, v.Equals(retPayloads[i]))
assert.Equal(t, retValues[i], v.Value)
}

// check value sizes for all existing paths
Expand Down
Loading