-
Notifications
You must be signed in to change notification settings - Fork 25
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
Fil/benchmarks #69
base: master
Are you sure you want to change the base?
Fil/benchmarks #69
Conversation
hamt_bench_test.go
Outdated
hamts := []hamtParams{ | ||
hamtParams{ | ||
id: "init.AddressMap", | ||
count: 55649, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These numbers are taken from a snapshot of chainstate two weeks ago. We will want to change them periodically to run benchmarks that are relevant. For the final v3 tuning we will want to extrapolate values we expect a few months out. Merging temporary values to master is ugly but setting values in source is a nice lightweight mechanism for updating.
} | ||
} | ||
|
||
func doBenchmarkResetSuite(num int, bitWidth int, datasize int, keysize int) func(b *testing.B) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We added this benchmark for resetting the set of existing keys to better match the expected workloads on some filecoin state tree HAMTs, i.e. power.Claims.
benchparse/README.md
Outdated
@@ -0,0 +1,24 @@ | |||
# Benchparse |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may make sense to extract this from this package so we can avoid perf dep in HAMT. Additionally I expect to tweak and use this for measuring AMT so decoupling might make logical sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, probably.
@acruikshank couldn't tag you for review but please take a look FYI @anorth @Stebalien @rvagg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
High level looks good
benchparse/README.md
Outdated
@@ -0,0 +1,24 @@ | |||
# Benchparse |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, probably.
b3b492b
to
a4e9e84
Compare
WIP using and extending existing benchmarks to help tune filecoin hamt parameters.