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

tests: add mpt vs vkt insertion benchmarks #146

Closed
wants to merge 1 commit into from
Closed

tests: add mpt vs vkt insertion benchmarks #146

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
9 changes: 9 additions & 0 deletions core/state/database.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ const (
codeCacheSize = 64 * 1024 * 1024
)

var TestVKTOpenStateless = false

// Database wraps access to tries and contract code.
type Database interface {
// OpenTrie opens the main account trie.
Expand Down Expand Up @@ -257,6 +259,13 @@ func (db *VerkleDB) OpenTrie(root common.Hash) (Trie, error) {
if err != nil {
panic(err)
}

if TestVKTOpenStateless {
r, err = verkle.ParseStatelessNode(payload, 0, root[:])
if err != nil {
panic(err)
}
}
Comment on lines +263 to +268
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting this to work was interesting to understand the stateless.go implementation in go-verkle.
I think this is the correct way to do what I intend. (Look at the second bullet of the PR description)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm, if you need to open a stateless node, you are using an older, slower version. Please rebase and re-run the benchmarks.

Copy link
Collaborator Author

@jsign jsign Dec 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh no, there wasn't a need for it. I just remembered some old pprof image you shared and I remembered seeing StatelessNodes there, so wanted to also include a benchmark for that case.

If the replay-block benchmark isn't using a stateless VKT, I can remove that benchmark since isn't relevant anymore.
Is that the case?

(Actually, adding this stateless VKT benchmark was friction here, so glad if that isn't meaningful anymore)
(This PR is on top of the latest beverly-hills so should be using the latest stuff)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something that might not have been obvious: StatelessNode will disappear in the mid-term. I've got a PR that I need to dust-off, for which the code in tree.go can do both stateful and stateless. In any case, stateful trees are more relevant to benchmark at the moment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gballet, great. I'll prune things a bit in this PR to avoid this flag, and remove the stateless benchmark.

return trie.NewVerkleTrie(r, db.db), err
}

Expand Down
112 changes: 112 additions & 0 deletions tests/tries_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
package tests

import (
"fmt"
"math/big"
"math/rand"
"testing"

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/core/rawdb"
"github.com/ethereum/go-ethereum/core/state"
"github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/ethdb/memorydb"
"github.com/ethereum/go-ethereum/trie"
"github.com/gballet/go-verkle"
)

func BenchmarkTriesRandom(b *testing.B) {
numAccounts := []int{1_000, 5_000, 10_000}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll run the benchmark for 1000, 5000, and 10000 key-value insertions.
We can tune these cases if we have a more accurate ballpark.


for _, numAccounts := range numAccounts {
rs := rand.New(rand.NewSource(42))
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the sake of consistency, let's try to make these benchmark deterministic.

accounts := getRandomStateAccounts(rs, numAccounts)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a helper function I created to create random state accounts. I'll touch on this later in this file.
Note that the benchmark is for State Accounts. We could play also with storage slots, but I've the sense that most of the overhead will be the same in both. Maybe some nits difference in trie key generation; but as a first exploration I think state accounts are reasonable.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure it's the same, because in most cases the account trie will be very small, whereas the verkle tree is always full.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah, that's a fair comment. I think I explained incorrectly that I meant to compare inserting a state account vs a storage slot in a VKT. In this new scenario, both insertion would be in the same tree so I'd expect their performance be the same.

It's true that if we compare it with MPT, then both cases aren't similar for the reason you said!


b.Run(fmt.Sprintf("MPT/%d accounts", numAccounts), func(b *testing.B) {
trie, _ := trie.NewStateTrie(trie.TrieID(common.Hash{}), trie.NewDatabase(memorydb.New()))
b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
for k := 0; k < len(accounts); k++ {
trie.TryUpdateAccount(accounts[k].address[:], &accounts[k].stateAccount)
}
trie.Commit(true)
}
})
b.Run(fmt.Sprintf("VKT/%d accounts", numAccounts), func(b *testing.B) {
// Warmup VKT configuration
trie.NewVerkleTrie(verkle.New(), trie.NewDatabase(memorydb.New())).TryUpdate([]byte("00000000000000000000000000000012"), []byte("B"))
Comment on lines +37 to +38
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a warmup to let precomp be generated if isn't there and not mess up with benchmark results.


b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
trie := trie.NewVerkleTrie(verkle.New(), trie.NewDatabase(memorydb.New()))
for k := 0; k < len(accounts); k++ {
trie.TryUpdateAccount(accounts[k].address[:], &accounts[k].stateAccount)
}
trie.Commit(true)
}
})
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So TL;DR both tests are exactly equal both in:

  • Structure
  • Data used for the benchmark.

So apparently looks like a fair comparison.

}

type randomAccount struct {
address common.Address
stateAccount types.StateAccount
}

func getRandomStateAccounts(rand *rand.Rand, count int) []randomAccount {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noting surprising here. Maybe at some point it might be interesting to add CodeHash values; but still there's a big gap with the current simple state accounts to look into.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I concur: it would be very interesting to add code, especially larger codes, because it will impact the comparison.

randomBytes := func(size int) []byte {
ret := make([]byte, size)
rand.Read(ret)
return ret
}

accounts := make([]randomAccount, count)
for i := range accounts {
accounts[i] = randomAccount{
address: common.BytesToAddress(randomBytes(common.AddressLength)),
stateAccount: types.StateAccount{
Nonce: rand.Uint64(),
Balance: big.NewInt(int64(rand.Uint64())),
Root: common.Hash{},
CodeHash: nil,
},
}
}
return accounts
}

func BenchmarkTriesRandomVKTStateless(b *testing.B) {
numAccounts := []int{1_000, 5_000, 10_000}
state.TestVKTOpenStateless = true
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We set the flag mentioned before so whenever OpenTrie(...) is called internally, it will open the verkle trie in stateless mode.


for _, numAccounts := range numAccounts {
rs := rand.New(rand.NewSource(42))
accounts := getRandomStateAccounts(rs, numAccounts)

b.Run(fmt.Sprintf("%d accounts", numAccounts), func(b *testing.B) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This benchmark is structured a bit differently.

Here we start with state.NewDatabaseWithConfig(...) and set the flag UseVerkle to signal the internal implementation to create a VKT.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed, that's how it should be done, and you shouldn't have to need to hack a flag in it. If you do, it's a bug 😱

trieDB := state.NewDatabaseWithConfig(rawdb.NewMemoryDatabase(), &trie.Config{UseVerkle: true})

prevTrie, _ := trieDB.OpenTrie(common.Hash{})
for k := 0; k < len(accounts); k++ {
prevTrie.TryUpdateAccount(accounts[k].address[:], &accounts[k].stateAccount)
}
prevTrie.Commit(false)
Comment on lines +91 to +95
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We pre-populate some random accounts in the trie and commit, which will persist everything in the underlying database.


accounts := getRandomStateAccounts(rs, numAccounts)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We generate a new set of accounts (because if we use the previous set, it will match the same keys).
So the idea here is that there will be nodes that would overlap in the paths; just to make it more interesting.

b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
trie, err := trieDB.OpenTrie(prevTrie.Hash())
if err != nil {
b.Fatal(err)
}
for k := 0; k < len(accounts); k++ {
trie.TryUpdateAccount(accounts[k].address[:], &accounts[k].stateAccount)
}
trie.Commit(true)
}
Comment on lines +100 to +109
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Run the same benchmark as the other, but the underlying VKT is stateless so it will be doing a lot more byte decoding.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, comparing to stateless is also interesting, I'm just saying that it's not as interesting as benchmarking stateful

})
}
}