-
Notifications
You must be signed in to change notification settings - Fork 109
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
Split Iter & IterByColRange types into Tx and MutTxId versions #2043
Conversation
benchmarks please |
Benchmarking failed. Please check the workflow run for details. |
Callgrind benchmark resultsCallgrind Benchmark ReportThese benchmarks were run using callgrind, Measurement changes larger than five percent are in bold. In-memory benchmarkscallgrind: empty transaction
callgrind: filter
callgrind: insert bulk
callgrind: iterate
callgrind: serialize_product_value
callgrind: update bulk
On-disk benchmarkscallgrind: empty transaction
callgrind: filter
callgrind: insert bulk
callgrind: iterate
callgrind: serialize_product_value
callgrind: update bulk
|
4684925
to
0796849
Compare
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.
Almost there, this is looking very promising in terms of performance improvements.
crates/core/src/db/datastore/locking_tx_datastore/committed_state.rs
Outdated
Show resolved
Hide resolved
crates/core/src/db/datastore/locking_tx_datastore/committed_state.rs
Outdated
Show resolved
Hide resolved
crates/core/src/db/datastore/locking_tx_datastore/state_view.rs
Outdated
Show resolved
Hide resolved
crates/core/src/db/datastore/locking_tx_datastore/state_view.rs
Outdated
Show resolved
Hide resolved
ea1c16f
to
c4f7934
Compare
crates/core/src/db/datastore/locking_tx_datastore/state_view.rs
Outdated
Show resolved
Hide resolved
c4f7934
to
ba55f96
Compare
ba55f96
to
d5baa13
Compare
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 now :)
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.
I've reviewed just what I am the code owner of. The changes to the datastore trait look good to me.
Description of Changes
Refactor
Iter / CommittedIndexIter
forTx
&MutTx
variants, and solves early the logic for decide the state machine.Closes #2024
Expected complexity level and risk
1
Testing
Describe any testing you've done, and any testing you'd like your reviewers to do,
so that you're confident that all the changes work as expected!
sudo nice -n -20 taskpolicy -B -t 5 -l 5 -c utility cargo bench --bench subscription -- --save-baseline subs