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.

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

Conversation

jsign
Copy link
Collaborator

@jsign jsign commented Dec 5, 2022

This PR adds two benchmarks:

  • A comparison of MPT and VKTs inserting a different number of key-values
  • A similar insertion benchmark for VKT, but against a stateless VKT so we can understand deserialization overheads.

Running these benchmarks on my desktop computer shows significant gap in performance between the two:

$ go test ./tests -run=none -bench=BenchmarkTriesRandom
goos: linux
goarch: amd64
pkg: github.com/ethereum/go-ethereum/tests
cpu: AMD Ryzen 7 3800XT 8-Core Processor            
BenchmarkTriesRandom/MPT/1000_accounts-16                   1066           1071067 ns/op          166047 B/op       3024 allocs/op
BenchmarkTriesRandom/VKT/1000_accounts-16                     20          61900194 ns/op         9494036 B/op      32842 allocs/op
BenchmarkTriesRandom/MPT/5000_accounts-16                    262           4042749 ns/op          832332 B/op      15024 allocs/op
BenchmarkTriesRandom/VKT/5000_accounts-16                      4         282367474 ns/op        43465564 B/op     150956 allocs/op
BenchmarkTriesRandom/MPT/10000_accounts-16                   133           8312897 ns/op         1740491 B/op      30891 allocs/op
BenchmarkTriesRandom/VKT/10000_accounts-16                     2         575018258 ns/op        87431768 B/op     303385 allocs/op
BenchmarkTriesRandomVKTStateless/1000_accounts-16             14          75025065 ns/op        10103659 B/op      37719 allocs/op
BenchmarkTriesRandomVKTStateless/5000_accounts-16              4         320815470 ns/op        48202616 B/op     173598 allocs/op
BenchmarkTriesRandomVKTStateless/10000_accounts-16                     2         680707633 ns/op        104035452 B/op    369356 allocs/op
PASS
ok      github.com/ethereum/go-ethereum/tests   18.057s

I've been looking a bit closer on possible whys, and I've some ideas to start experimenting. But here we have at least the current baseline to try making the gap smaller.

Note that these benchmarks are 100% focused on comparing the tries performance. In a real block execution, the block execution is composed on many other tasks. For example, if trie insertions are 40% of the work of executing a block, then their performance difference would only impact ~40% of the block execution. (40% is an example).

@jsign jsign force-pushed the jsign/benchtries branch 3 times, most recently from 8b7f84d to 00cb7c7 Compare December 5, 2022 22:26
)

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.

numAccounts := []int{1_000, 5_000, 10_000}

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.


for _, numAccounts := range numAccounts {
rs := rand.New(rand.NewSource(42))
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!

Comment on lines +37 to +38
// Warmup VKT configuration
trie.NewVerkleTrie(verkle.New(), trie.NewDatabase(memorydb.New())).TryUpdate([]byte("00000000000000000000000000000012"), []byte("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 is a warmup to let precomp be generated if isn't there and not mess up with benchmark results.

Comment on lines +91 to +95
prevTrie, _ := trieDB.OpenTrie(common.Hash{})
for k := 0; k < len(accounts); k++ {
prevTrie.TryUpdateAccount(accounts[k].address[:], &accounts[k].stateAccount)
}
prevTrie.Commit(false)
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.

}
prevTrie.Commit(false)

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.


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.

Comment on lines +263 to +268
if TestVKTOpenStateless {
r, err = verkle.ParseStatelessNode(payload, 0, root[:])
if err != nil {
panic(err)
}
}
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

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.

Comment on lines +100 to +109
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)
}
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

This was referenced Dec 13, 2022
@jsign
Copy link
Collaborator Author

jsign commented Dec 18, 2022

Closing this PR since these benchmarks were moved to #147, where we're making performance improvements.

@jsign jsign closed this Dec 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants