perf(hydro_lang): avoid allocations in sim scheduler's ready-list partitioning - #3116
Merged
Merged
Conversation
Deploying hydro with
|
| Latest commit: |
7048b97
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://294b3358.hydroflow.pages.dev |
| Branch Preview URL: | https://sandbox-a11652b2-66d7-467b-9.hydroflow.pages.dev |
shadaj
added a commit
that referenced
this pull request
Aug 3, 2026
…titioning The simulator's `LaunchedSim::step` spent significant time in `Iterator::partition`, which allocated two fresh `Vec`s per call (four call sites, invoked on every scheduler step and for every async DFIR that made progress). Each partition predicate additionally performed a `HashMap` lookup keyed by `(LocationId, Option<u32>)`, cloning a `LocationId` (a boxed allocation for `Tick` variants) for every element checked. ## Changes - **Introduce `SimTick` and `SimObservation` structs** that store each tick's / observation's hooks (and inline hooks) *inline*, replacing the anonymous `(LocationId, Option<u32>, DfirErased)` tuples plus the shared `hooks` / `inline_hooks` maps on `LaunchedSim`. The maps are drained once in `start()` (where the generated code's serialized-location string keys can be used directly, before any `LocationId` deserialization), so the scheduler's hot paths no longer do any keyed lookups or `LocationId` clones. A `debug_assert!` checks that every hook is claimed by a tick or observation. - **Replace `drain(..).partition(...)` with `Vec::extract_if`**, which moves matching elements between the ready/not-ready lists in place without allocating intermediate vectors. - **Store the tick's parent location** (`SimTick::parent_location`) instead of the full tick location, since the full location was only ever used to extract the parent for readiness matching; this removes an `unreachable!()` destructure from the hot loop. - **Factor the shared readiness predicate** into `hook_can_release` and `SimTick::can_run` / `SimObservation::can_run` helper methods, so the scheduling conditions are named and documented instead of duplicated inline. - Misc cleanup: `run_hooks` now takes `&mut [Box<dyn SimHook>]`, the empty-`default_hooks` scratch `Vec` for observations is gone, and field names are pluralized (`possibly_ready_observations`, etc.) for consistency. Verified with `cargo check`/`clippy --all-features` (clean) and `cargo test --features sim sim::` (36 passed). Co-authored-by: Infinity 🤖 <infinity@hydro.run> PR: #3116
shadaj
force-pushed
the
sandbox-a11652b2-66d7-467b-9ae1-13e696bd93aa
branch
from
August 3, 2026 22:24
d50f627 to
9caf4cb
Compare
shadaj
marked this pull request as ready for review
August 3, 2026 22:39
shadaj
added a commit
that referenced
this pull request
Aug 3, 2026
…titioning The simulator's `LaunchedSim::step` spent significant time in `Iterator::partition`, which allocated two fresh `Vec`s per call (four call sites, invoked on every scheduler step and for every async DFIR that made progress). Each partition predicate additionally performed a `HashMap` lookup keyed by `(LocationId, Option<u32>)`, cloning a `LocationId` (a boxed allocation for `Tick` variants) for every element checked. ## Changes - **Introduce `SimTick` and `SimObservation` structs** that store each tick's / observation's hooks (and inline hooks) *inline*, replacing the anonymous `(LocationId, Option<u32>, DfirErased)` tuples plus the shared `hooks` / `inline_hooks` maps on `LaunchedSim`. The maps are drained once in `start()` (where the generated code's serialized-location string keys can be used directly, before any `LocationId` deserialization), so the scheduler's hot paths no longer do any keyed lookups or `LocationId` clones. A `debug_assert!` checks that every hook is claimed by a tick or observation. - **Replace `drain(..).partition(...)` with `Vec::extract_if`**, which moves matching elements between the ready/not-ready lists in place without allocating intermediate vectors. - **Store the tick's parent location** (`SimTick::parent_location`) instead of the full tick location, since the full location was only ever used to extract the parent for readiness matching; this removes an `unreachable!()` destructure from the hot loop. - **Factor the shared readiness predicate** into `hook_can_release` and `SimTick::can_run` / `SimObservation::can_run` helper methods, so the scheduling conditions are named and documented instead of duplicated inline. - Misc cleanup: `run_hooks` now takes `&mut [Box<dyn SimHook>]`, the empty-`default_hooks` scratch `Vec` for observations is gone, and field names are pluralized (`possibly_ready_observations`, etc.) for consistency. Verified with `cargo check`/`clippy --all-features` (clean) and `cargo test --features sim sim::` (36 passed). Co-authored-by: Infinity 🤖 <infinity@hydro.run> PR: #3116
shadaj
force-pushed
the
sandbox-a11652b2-66d7-467b-9ae1-13e696bd93aa
branch
from
August 3, 2026 22:59
9caf4cb to
436f0a4
Compare
shadaj
added a commit
that referenced
this pull request
Aug 3, 2026
…titioning The simulator's `LaunchedSim::step` spent significant time in `Iterator::partition`, which allocated two fresh `Vec`s per call (four call sites, invoked on every scheduler step and for every async DFIR that made progress). Each partition predicate additionally performed a `HashMap` lookup keyed by `(LocationId, Option<u32>)`, cloning a `LocationId` (a boxed allocation for `Tick` variants) for every element checked. ## Changes - **Introduce `SimTick` and `SimObservation` structs** that store each tick's / observation's hooks (and inline hooks) *inline*, replacing the anonymous `(LocationId, Option<u32>, DfirErased)` tuples plus the shared `hooks` / `inline_hooks` maps on `LaunchedSim`. The maps are drained once in `start()` (where the generated code's serialized-location string keys can be used directly, before any `LocationId` deserialization), so the scheduler's hot paths no longer do any keyed lookups or `LocationId` clones. A `debug_assert!` checks that every hook is claimed by a tick or observation. - **Replace `drain(..).partition(...)` with `Vec::extract_if`**, which moves matching elements between the ready/not-ready lists in place without allocating intermediate vectors. - **Store the tick's parent location** (`SimTick::parent_location`) instead of the full tick location, since the full location was only ever used to extract the parent for readiness matching; this removes an `unreachable!()` destructure from the hot loop. - **Factor the shared readiness predicate** into `hook_can_release` and `SimTick::can_run` / `SimObservation::can_run` helper methods, so the scheduling conditions are named and documented instead of duplicated inline. - Misc cleanup: `run_hooks` now takes `&mut [Box<dyn SimHook>]`, the empty-`default_hooks` scratch `Vec` for observations is gone, and field names are pluralized (`possibly_ready_observations`, etc.) for consistency. Verified with `cargo check`/`clippy --all-features` (clean) and `cargo test --features sim sim::` (36 passed). Co-authored-by: Infinity 🤖 <infinity@hydro.run> PR: #3116
shadaj
force-pushed
the
sandbox-a11652b2-66d7-467b-9ae1-13e696bd93aa
branch
from
August 3, 2026 23:34
436f0a4 to
15acdfb
Compare
shadaj
force-pushed
the
sandbox-a11652b2-66d7-467b-9ae1-13e696bd93aa
branch
from
August 4, 2026 21:19
15acdfb to
eaa13df
Compare
shadaj
force-pushed
the
sandbox-a11652b2-66d7-467b-9ae1-13e696bd93aa
branch
from
August 4, 2026 21:19
eaa13df to
4ee8ac7
Compare
MingweiSamuel
approved these changes
Aug 4, 2026
shadaj
force-pushed
the
sandbox-a11652b2-66d7-467b-9ae1-13e696bd93aa
branch
2 times, most recently
from
August 4, 2026 22:50
39f7fe3 to
d752b7e
Compare
MingweiSamuel
approved these changes
Aug 5, 2026
MingweiSamuel
left a comment
MingweiSamuel
left a comment
Member
There was a problem hiding this comment.
I guess the hook order matters?
shadaj
force-pushed
the
sandbox-a11652b2-66d7-467b-9ae1-13e696bd93aa
branch
from
August 5, 2026 21:48
d752b7e to
4503a17
Compare
Base automatically changed from
sandbox-db444e4e-78bd-4261-b714-f97435c63d66
to
main
August 11, 2026 01:10
…titioning The simulator's `LaunchedSim::step` spent significant time in `Iterator::partition`, which allocated two fresh `Vec`s per call (four call sites, invoked on every scheduler step and for every async DFIR that made progress). Each partition predicate additionally performed a `HashMap` lookup keyed by `(LocationId, Option<u32>)`, cloning a `LocationId` (a boxed allocation for `Tick` variants) for every element checked. ## Changes - **Introduce `SimTick` and `SimObservation` structs** that store each tick's / observation's hooks (and inline hooks) *inline*, replacing the anonymous `(LocationId, Option<u32>, DfirErased)` tuples plus the shared `hooks` / `inline_hooks` maps on `LaunchedSim`. The maps are drained once in `start()` (where the generated code's serialized-location string keys can be used directly, before any `LocationId` deserialization), so the scheduler's hot paths no longer do any keyed lookups or `LocationId` clones. A `debug_assert!` checks that every hook is claimed by a tick or observation. - **Replace `drain(..).partition(...)` with `Vec::extract_if`**, which moves matching elements between the ready/not-ready lists in place without allocating intermediate vectors. - **Store the tick's parent location** (`SimTick::parent_location`) instead of the full tick location, since the full location was only ever used to extract the parent for readiness matching; this removes an `unreachable!()` destructure from the hot loop. - **Factor the shared readiness predicate** into `hook_can_release` and `SimTick::can_run` / `SimObservation::can_run` helper methods, so the scheduling conditions are named and documented instead of duplicated inline. - Misc cleanup: `run_hooks` now takes `&mut [Box<dyn SimHook>]`, the empty-`default_hooks` scratch `Vec` for observations is gone, and field names are pluralized (`possibly_ready_observations`, etc.) for consistency. Verified with `cargo check`/`clippy --all-features` (clean) and `cargo test --features sim sim::` (36 passed). Co-authored-by: Infinity 🤖 <infinity@hydro.run> PR: #3116
shadaj
force-pushed
the
sandbox-a11652b2-66d7-467b-9ae1-13e696bd93aa
branch
from
August 11, 2026 01:10
4503a17 to
7048b97
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The simulator's
LaunchedSim::stepspent significant time inIterator::partition, which allocated two freshVecs per call (four call sites, invoked on every scheduler step and for every async DFIR that made progress). Each partition predicate additionally performed aHashMaplookup keyed by(LocationId, Option<u32>), cloning aLocationId(a boxed allocation forTickvariants) for every element checked.Changes
SimTickandSimObservationstructs that store each tick's / observation's hooks (and inline hooks) inline, replacing the anonymous(LocationId, Option<u32>, DfirErased)tuples plus the sharedhooks/inline_hooksmaps onLaunchedSim. The maps are drained once instart()(where the generated code's serialized-location string keys can be used directly, before anyLocationIddeserialization), so the scheduler's hot paths no longer do any keyed lookups orLocationIdclones. Adebug_assert!checks that every hook is claimed by a tick or observation.drain(..).partition(...)withVec::extract_if, which moves matching elements between the ready/not-ready lists in place without allocating intermediate vectors.SimTick::parent_location) instead of the full tick location, since the full location was only ever used to extract the parent for readiness matching; this removes anunreachable!()destructure from the hot loop.hook_can_releaseandSimTick::can_run/SimObservation::can_runhelper methods, so the scheduling conditions are named and documented instead of duplicated inline.run_hooksnow takes&mut [Box<dyn SimHook>], the empty-default_hooksscratchVecfor observations is gone, and field names are pluralized (possibly_ready_observations, etc.) for consistency.Verified with
cargo check/clippy --all-features(clean) andcargo test --features sim sim::(36 passed).