-
Notifications
You must be signed in to change notification settings - Fork 41
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
Added support for hashmaps in Smt
and SimpleSmt
#363
Conversation
@polydez - merging the latest |
@bobbinth, now it works! |
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.
Thank you! Looks good! I've reviewed all non-test code so far and left some comments inline.
I also ran the benchmarks (1M tree size and 1K insertions) and got about 20% improvement:
Using BTreeMap
Constructed a SMT with 1000000 key-value pairs in 41.5 seconds
Number of leaf nodes: 1000000
Running an insertion benchmark:
An average insertion time measured by 1000 inserts into an SMT with 1000000 leaves is 404 μs
Running a batched insertion benchmark:
An average insert-batch computation time measured by a 1000-batch into an SMT with 1001000 leaves over 405.3 ms is 405 μs
An average insert-batch application time measured by a 1000-batch into an SMT with 1001000 leaves over 32.5 ms is 32 μs
An average batch insertion time measured by a 1k-batch into an SMT with 1001000 leaves totals to 437.8 ms
Running a batched update benchmark:
An average update-batch computation time measured by a 1000-batch into an SMT with 1002000 leaves over 402.0 ms is 402 μs
An average update-batch application time measured by a 1000-batch into an SMT with 1002000 leaves over 28.1 ms is 28 μs
An average batch update time measured by a 1k-batch into an SMT with 1002000 leaves totals to 430.1 ms
Running a proof generation benchmark:
An average proving time measured by 100 value proofs in an SMT with 1001807 leaves in 9 μs
Using Hashbrown
Constructed a SMT with 1000000 key-value pairs in 42.1 seconds
Number of leaf nodes: 1000000
Running an insertion benchmark:
An average insertion time measured by 1000 inserts into an SMT with 1000000 leaves is 367 μs
Running a batched insertion benchmark:
An average insert-batch computation time measured by a 1000-batch into an SMT with 1001000 leaves over 360.1 ms is 360 μs
An average insert-batch application time measured by a 1000-batch into an SMT with 1001000 leaves over 4.8 ms is 5 μs
An average batch insertion time measured by a 1k-batch into an SMT with 1001000 leaves totals to 364.9 ms
Running a batched update benchmark:
An average update-batch computation time measured by a 1000-batch into an SMT with 1002000 leaves over 370.1 ms is 370 μs
An average update-batch application time measured by a 1000-batch into an SMT with 1002000 leaves over 4.2 ms is 4 μs
An average batch update time measured by a 1k-batch into an SMT with 1002000 leaves totals to 374.2 ms
Running a proof generation benchmark:
An average proving time measured by 100 value proofs in an SMT with 1001806 leaves in 0 μs
I tested only application of mutation set, which does a lot of inserts/updates and in your benchmark this operation gave us up to ~7x improvement. Computation time is still slow, but I hope, paralleling of the computation can give us significant speed-up. |
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.
Looks good! Thank you! There are 2 more things we should do:
- Update the "Crate features" section in the README to mention
smt_hashmaps
feature (I noticed thatconcurrent
feature description is also missing there). - Probably update the Makefile to make sure we also run test with
smt_hashmaps
feature enabled.
I will update the list, thank you for noticing! But what about Line 63 in a797a9e
|
Ah yes! I was looking at an old version of the file. |
Quality Gate passedIssues Measures |
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.
All looks good! Thank you!
After benchmarking of
compute_mutations
andapply_mutations
we noticed poor performance of multiple key-value insertions intoBTreeMap
which is used in our SMT implementations. Rewriting to hashbrown'sHashMap
(which supports no-std) gave us more than 10x boost inapply_mutations
operation for large trees.apply_mutations/SimpleSmt: apply_mutations/10000
time: [154.52 ms 163.09 ms 174.46 ms]
change: [-93.242% -92.753% -92.142%] (p = 0.00 < 0.05)
Performance has improved.
(More context in the PR: #355)
In this implementation we also introduced
smt_hashmaps
feature. If it's switched off, the SMT uses binary-tree implementation, as before. This might be useful for backward-compatibility with code, which relies on entry ordering in SMTs.