Break DataStore's implementation into modules #1441
Conversation
|
Pretty cool, didn't know this would have this effect. 10% on incremental builds is pretty good! |
|
@smklein suggested it when we were chatting about ideas as a way to speed up incremental times since diesel is one of the biggest time sinks. I don't understand why this would speed up the clean builds, though! |
smklein
left a comment
smklein
left a comment
There was a problem hiding this comment.
From my own tests:
Before this patch, incremental rebuild (touch datastore.rs && cargo build): In the range of 19-21 seconds
After this patch, incremental rebuild (touch datastore/mod.rs && cargo build): In the range of 19-21 seconds
I'm not seeing much of a difference here - I'm on a 16 core, 32 GiB machine.
That being said, I still like this PR regardless of the performance implications, so LGTM, but it's curious that I'm not replicating your results.
| use std::sync::Arc; | ||
| use uuid::Uuid; | ||
|
|
||
| #[tokio::test] |
There was a problem hiding this comment.
It'll probably make sense to move some of these out too, but we can punt on that for now
|
Out of curiosity, what happens if you modify |
Crucible changes:
Make crutest use BlockIO trait instead of a Guest (#1452)
Use new syncfs syscall (#1427)
Increment write backpressure before deferred encryption (#1444)
Use RAII handles for backpressure (#1443)
Fine-tuning backpressure clamping, and API cleanups (#1442)
Fix outdated comment (#1447)
Update Rust crate rusqlite to 0.32 (#1418)
Fix write reordering bug (#1448)
Remove `history_file` (#1446)
Remove optionality for `BlockRes` in `DeferredWrite` (#1441)
Propolis changes
instance spec rework: tighten up component naming (#761)
instance spec rework: remove most dependencies on `InstanceSpecV0` from propolis-server (#757)
fix new 1.81.0 warning and clippy error (#760)
standalone: be more helpful with bad block device configs (#758)
Crucible changes:
Make crutest use BlockIO trait instead of a Guest (#1452)
Use new syncfs syscall (#1427)
Increment write backpressure before deferred encryption (#1444)
Use RAII handles for backpressure (#1443)
Fine-tuning backpressure clamping, and API cleanups (#1442)
Fix outdated comment (#1447)
Update Rust crate rusqlite to 0.32 (#1418)
Fix write reordering bug (#1448)
Remove `history_file` (#1446)
Remove optionality for `BlockRes` in `DeferredWrite` (#1441)
Propolis changes
instance spec rework: tighten up component naming (#761)
instance spec rework: remove most dependencies on `InstanceSpecV0` from
propolis-server (#757)
fix new 1.81.0 warning and clippy error (#760)
standalone: be more helpful with bad block device configs (#758)
Co-authored-by: Alan Hanson <alan@oxide.computer>
This is a purely mechanical movement of the majority of
DataStore's methods into separate module files in an attempt to speed up compilation (particularly incremental compilation). On my machine, it looks like a modest success, cutting incremental compilation down by ~7 seconds for both test and non-test builds:2a2a7f7e)647e336e)datastore.rsmodifieddatastore.rsmodifiedwhere "lib+bin" is
cargo build -p omicron-nexusand "tests" iscargo build -p omicron-nexus --tests(i.e., the build of the tests without running them). Modifying files underneathdatastore/on this branch results in identical timings to modifyingdatastore/mod.rs(which is surprising to me, but it's quite reliable).These timings are all debug builds; if there's interest I can collect them for release builds also.