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

Add benches for the merkle package #794

Closed
wants to merge 57 commits into from

Conversation

iFrostizz
Copy link
Contributor

Addresses #689 (comment)

This PR adds a bench to measure the time of generation of the merkle root of transactions.
We execute it for 10, 100, 1000, and 10000 iterations, here are the results:

goos: linux
goarch: amd64
pkg: github.com/ava-labs/hypersdk/merkle
cpu: AMD Ryzen 9 5950X 16-Core Processor
BenchmarkMerkleTxRoot/10-32 	     973	   1855691 ns/op
BenchmarkMerkleTxRoot/100-32         	      15	  87687055 ns/op
BenchmarkMerkleTxRoot/1000-32        	       1	6838312818 ns/op
BenchmarkMerkleTxRoot/10000-32       	       1	618511353824 ns/op
PASS
ok  	github.com/ava-labs/hypersdk/merkle	628.683s

The average time it takes per operation doesn't scale linearly which makes sense given the tree data-structure.

chain/block.go Outdated Show resolved Hide resolved
@patrick-ogrady
Copy link
Contributor

BenchmarkMerkleTxRoot/1000-32        	       1	6838312818 ns/op
BenchmarkMerkleTxRoot/10000-32       	       1	618511353824 ns/op

We need to speed this up if we want to merge this. 618s for 10k items seems really wrong or broken.

merkle/merkle.go Outdated
})
}

db, err := merkledb.New(ctx, memdb.New(), merkledb.Config{
Copy link
Contributor

@patrick-ogrady patrick-ogrady Mar 19, 2024

Choose a reason for hiding this comment

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

It is possible that memdb could be the cause of this.

I talked to @danlaine about writing a much thinner merkle wrapper just for this function (which shouldn't require caches/history/etc).

merkle/merkle.go Outdated Show resolved Hide resolved
merkle/merkle.go Outdated Show resolved Hide resolved
merkle/merkle.go Outdated Show resolved Hide resolved
merkle/merkle.go Outdated Show resolved Hide resolved
var db merkledb.MerkleDB
var err error

defaultConfig := merkledb.Config{
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't really want to merge this until this config is largely removed. We need a "single instance" trie that is merkleDB-compatible for #950

Copy link

This PR has become stale because it has been open for 30 days with no activity. Adding the lifecycle/frozen label will exempt this PR from future lifecycle events..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants