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

Replace HashMap with BTreeMap in storage #1007

Merged
merged 4 commits into from
Aug 30, 2024
Merged

Conversation

kpp
Copy link
Contributor

@kpp kpp commented Aug 16, 2024

Description

This may probably affect cycle counts

Linked Issues

Copy link

codecov bot commented Aug 16, 2024

Codecov Report

Attention: Patch coverage is 84.61538% with 2 lines in your changes missing coverage. Please review.

Project coverage is 78.6%. Comparing base (a5f9485) to head (e3cf5a9).
Report is 18 commits behind head on nightly.

Files with missing lines Patch % Lines
...odule-system/sov-modules-core/src/storage/cache.rs 60.0% 2 Missing ⚠️
Additional details and impacted files
Files with missing lines Coverage Δ
...-system/sov-modules-core/src/storage/scratchpad.rs 89.1% <100.0%> (+0.1%) ⬆️
...odule-system/sov-modules-core/src/storage/cache.rs 89.3% <60.0%> (+0.2%) ⬆️

... and 3 files with indirect coverage changes

@eyusufatik
Copy link
Member

I need benchmarks for this both in zk (which is probably better) and for native execution.

@eyusufatik eyusufatik mentioned this pull request Aug 20, 2024
49 tasks
@kpp
Copy link
Contributor Author

kpp commented Aug 29, 2024

Before:

2024-08-29T10:37:34.663863Z  INFO citrea_risc0_bonsai_adapter::host: Added hint to guest with size 129482
Running sequencer commitments in DA slot
going into apply_soft_confirmations_from_sequencer_commitments
out of apply_soft_confirmations_from_sequencer_commitments
2024-08-29T10:37:50.372488Z  INFO executor: risc0_zkvm::host::server::exec::executor: execution time: 13.772917002s
2024-08-29T10:37:50.372542Z  INFO executor: risc0_zkvm::host::server::session: number of segments: 36
2024-08-29T10:37:50.372554Z  INFO executor: risc0_zkvm::host::server::session: total cycles: 37748736
2024-08-29T10:37:50.372564Z  INFO executor: risc0_zkvm::host::server::session: user cycles: 24851773

After:

2024-08-29T10:49:41.680428Z  INFO citrea_risc0_bonsai_adapter::host: Added hint to guest with size 131298
Running sequencer commitments in DA slot
going into apply_soft_confirmations_from_sequencer_commitments
out of apply_soft_confirmations_from_sequencer_commitments
2024-08-29T10:49:57.389553Z  INFO executor: risc0_zkvm::host::server::exec::executor: execution time: 13.76639099s
2024-08-29T10:49:57.389604Z  INFO executor: risc0_zkvm::host::server::session: number of segments: 36
2024-08-29T10:49:57.389616Z  INFO executor: risc0_zkvm::host::server::session: total cycles: 37224448
2024-08-29T10:49:57.389626Z  INFO executor: risc0_zkvm::host::server::session: user cycles: 24589729

@kpp
Copy link
Contributor Author

kpp commented Aug 29, 2024

        for i in 0..10 {
            for _ in 0..5 {
                let _ = sequencer.client.send_eth(to, None, None, None, 1u128).await;
            }
            sequencer.client.send_publish_batch_request().await;
            sequencer.wait_for_l2_height(seq_height0 + i + 1, None).await;
        }

Before:

2024-08-29T13:06:04.029375Z  INFO citrea_risc0_bonsai_adapter::host: Added hint to guest with size 215674
Running sequencer commitments in DA slot
going into apply_soft_confirmations_from_sequencer_commitments
out of apply_soft_confirmations_from_sequencer_commitments
2024-08-29T13:07:55.164017Z  INFO executor: risc0_zkvm::host::server::exec::executor: execution time: 108.700436027s
2024-08-29T13:07:55.164091Z  INFO executor: risc0_zkvm::host::server::session: number of segments: 196
2024-08-29T13:07:55.164106Z  INFO executor: risc0_zkvm::host::server::session: total cycles: 204734464
2024-08-29T13:07:55.164129Z  INFO executor: risc0_zkvm::host::server::session: user cycles: 151283298

After:

2024-08-29T13:12:29.137427Z  INFO citrea_risc0_bonsai_adapter::host: Added hint to guest with size 214444
Running sequencer commitments in DA slot
going into apply_soft_confirmations_from_sequencer_commitments
out of apply_soft_confirmations_from_sequencer_commitments
2024-08-29T13:14:20.776245Z  INFO executor: risc0_zkvm::host::server::exec::executor: execution time: 109.512394883s
2024-08-29T13:14:20.776331Z  INFO executor: risc0_zkvm::host::server::session: number of segments: 194
2024-08-29T13:14:20.776346Z  INFO executor: risc0_zkvm::host::server::session: total cycles: 203423744
2024-08-29T13:14:20.776377Z  INFO executor: risc0_zkvm::host::server::session: user cycles: 149839975

@eyusufatik eyusufatik merged commit daa6da4 into nightly Aug 30, 2024
12 checks passed
@eyusufatik eyusufatik deleted the kpp/btree_storage branch August 30, 2024 11:49
jfldde pushed a commit that referenced this pull request Aug 30, 2024
* Replace HashMap with BTreeMap in storage

* Fix lint

* cargo.lock
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