refactor(hydro_test): migrate paxos cross_singleton to by_ref() singleton mechanism [ci-bench]#2903
Draft
MingweiSamuel wants to merge 4 commits into
Draft
refactor(hydro_test): migrate paxos cross_singleton to by_ref() singleton mechanism [ci-bench]#2903MingweiSamuel wants to merge 4 commits into
MingweiSamuel wants to merge 4 commits into
Conversation
Deploying hydro with
|
| Latest commit: |
2f9004d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5d79373b.hydroflow.pages.dev |
| Branch Preview URL: | https://mingwei-paxos-ref.hydroflow.pages.dev |
Contributor
📊 Benchmark Results✅ Benchmark completed! You can download the results from the links below. Run History:
Last updated: 2026-06-10T19:50:36.699Z |
MingweiSamuel
added a commit
that referenced
this pull request
May 29, 2026
Added `with_singleton_capture` wrapper to `Stream::filter_map` so that `SingletonRef` values can be captured inside `filter_map` closures via `q!()`, matching the existing support in `map`, `filter`, `flat_map_ordered`, `flat_map_unordered`, `partition`, and `inspect`. Co-authored-by: Infinity 🤖 <infinity@hydro.run> PR: #2903
MingweiSamuel
added a commit
that referenced
this pull request
May 29, 2026
…eton mechanism [ci-bench]
We need to remove the global singleton counter and instead use a local counter.
Converted 6 out of 7 cross_singleton usages in paxos.rs to use the new
by_ref() singleton mechanism:
- recommit_after_leader_election: 3 cross_singleton calls replaced with
by_ref() captures in filter_map and map closures
- sequence_payload: 1 cross_singleton replaced with by_ref() in map
- index_payloads (sliced!): 1 cross_singleton replaced with by_ref() in map
- acceptor_p2: 2 cross_singleton calls replaced with by_ref() captures
in filter_map and map closures
Left unchanged:
- acceptor_p1: 2 cross_singleton calls remain because the a_log parameter
comes from a forward_ref cycle, and by_ref() on cycle sources causes a
graph partitioning error ("Handoff succ not in subgraph")
Also added + 'a lifetime bounds where required by by_ref() usage:
- PaxosLike::build, PaxosLike::with_client
- paxos_core, index_payloads
- compartmentalized_paxos_core
Updated IR snapshots to reflect the new singleton ref nodes.
Co-authored-by: Infinity 🤖 <infinity@hydro.run>
PR: #2903
9a6bb50 to
a6d6079
Compare
a6d6079 to
078d557
Compare
MingweiSamuel
added a commit
that referenced
this pull request
Jun 5, 2026
Added `with_singleton_capture` wrapper to `Stream::filter_map` so that `SingletonRef` values can be captured inside `filter_map` closures via `q!()`, matching the existing support in `map`, `filter`, `flat_map_ordered`, `flat_map_unordered`, `partition`, and `inspect`. Co-authored-by: Infinity 🤖 <infinity@hydro.run> PR: #2903
MingweiSamuel
added a commit
that referenced
this pull request
Jun 5, 2026
…eton mechanism [ci-bench]
We need to remove the global singleton counter and instead use a local counter.
Converted 6 out of 7 cross_singleton usages in paxos.rs to use the new
by_ref() singleton mechanism:
- recommit_after_leader_election: 3 cross_singleton calls replaced with
by_ref() captures in filter_map and map closures
- sequence_payload: 1 cross_singleton replaced with by_ref() in map
- index_payloads (sliced!): 1 cross_singleton replaced with by_ref() in map
- acceptor_p2: 2 cross_singleton calls replaced with by_ref() captures
in filter_map and map closures
Left unchanged:
- acceptor_p1: 2 cross_singleton calls remain because the a_log parameter
comes from a forward_ref cycle, and by_ref() on cycle sources causes a
graph partitioning error ("Handoff succ not in subgraph")
Also added + 'a lifetime bounds where required by by_ref() usage:
- PaxosLike::build, PaxosLike::with_client
- paxos_core, index_payloads
- compartmentalized_paxos_core
Updated IR snapshots to reflect the new singleton ref nodes.
Co-authored-by: Infinity 🤖 <infinity@hydro.run>
PR: #2903
e530702 to
b0db6d5
Compare
MingweiSamuel
added a commit
that referenced
this pull request
Jun 5, 2026
Added `with_singleton_capture` wrapper to `Stream::filter_map` so that `SingletonRef` values can be captured inside `filter_map` closures via `q!()`, matching the existing support in `map`, `filter`, `flat_map_ordered`, `flat_map_unordered`, `partition`, and `inspect`. Co-authored-by: Infinity 🤖 <infinity@hydro.run> PR: #2903
b0db6d5 to
84b2c09
Compare
MingweiSamuel
added a commit
that referenced
this pull request
Jun 5, 2026
…eton mechanism [ci-bench]
We need to remove the global singleton counter and instead use a local counter.
Converted 6 out of 7 cross_singleton usages in paxos.rs to use the new
by_ref() singleton mechanism:
- recommit_after_leader_election: 3 cross_singleton calls replaced with
by_ref() captures in filter_map and map closures
- sequence_payload: 1 cross_singleton replaced with by_ref() in map
- index_payloads (sliced!): 1 cross_singleton replaced with by_ref() in map
- acceptor_p2: 2 cross_singleton calls replaced with by_ref() captures
in filter_map and map closures
Left unchanged:
- acceptor_p1: 2 cross_singleton calls remain because the a_log parameter
comes from a forward_ref cycle, and by_ref() on cycle sources causes a
graph partitioning error ("Handoff succ not in subgraph")
Also added + 'a lifetime bounds where required by by_ref() usage:
- PaxosLike::build, PaxosLike::with_client
- paxos_core, index_payloads
- compartmentalized_paxos_core
Updated IR snapshots to reflect the new singleton ref nodes.
Co-authored-by: Infinity 🤖 <infinity@hydro.run>
PR: #2903
078d557 to
adac487
Compare
84b2c09 to
74e5643
Compare
… non-determinism The global `static SINGLETON_REF_COUNTER: AtomicUsize` was shared across all tests in a binary, causing `__hydro_singleton_ref_N` idents to shift depending on test execution order. For example, paxos snapshots expected IDs 0-6 but got 12-18 when singleton_ref tests ran first. Fix: Remove the global counter entirely. Instead, derive the ident index from `refs.len()` within each `with_singleton_capture` scope, so every closure's captured singleton refs are numbered starting from 0. This is safe because each closure gets its own lexical block in the generated code. Changes: - hydro_lang/src/singleton_ref.rs: Remove `SINGLETON_REF_COUNTER` static. Add `singleton_ref_ident(index)` helper. Simplify thread-local from `Vec<(syn::Ident, HydroNode)>` to `Vec<HydroNode>`. Use `refs.len()` as index. - hydro_lang/src/compile/ir/mod.rs: Change `ClosureExpr::singleton_refs` from `Vec<(syn::Ident, HydroNode)>` to `Vec<HydroNode>`. Update Clone, Serialize, deep_clone, transform_children, and emit_tokens to use index-based idents via the new helper. - hydro_test snapshots: Regenerated paxos IR and mermaid snapshots (stable only; nightly snapshots need a nightly CI run). Also adds test to ensure singleton references are handled in the correct order. Co-authored-by: Infinity 🤖 <infinity@hydro.run> PR: #2929
MingweiSamuel
added a commit
that referenced
this pull request
Jun 8, 2026
Added `with_singleton_capture` wrapper to `Stream::filter_map` so that `SingletonRef` values can be captured inside `filter_map` closures via `q!()`, matching the existing support in `map`, `filter`, `flat_map_ordered`, `flat_map_unordered`, `partition`, and `inspect`. Co-authored-by: Infinity 🤖 <infinity@hydro.run> PR: #2903
MingweiSamuel
added a commit
that referenced
this pull request
Jun 8, 2026
…eton mechanism [ci-bench]
We need to remove the global singleton counter and instead use a local counter.
Converted 6 out of 7 cross_singleton usages in paxos.rs to use the new
by_ref() singleton mechanism:
- recommit_after_leader_election: 3 cross_singleton calls replaced with
by_ref() captures in filter_map and map closures
- sequence_payload: 1 cross_singleton replaced with by_ref() in map
- index_payloads (sliced!): 1 cross_singleton replaced with by_ref() in map
- acceptor_p2: 2 cross_singleton calls replaced with by_ref() captures
in filter_map and map closures
Left unchanged:
- acceptor_p1: 2 cross_singleton calls remain because the a_log parameter
comes from a forward_ref cycle, and by_ref() on cycle sources causes a
graph partitioning error ("Handoff succ not in subgraph")
Also added + 'a lifetime bounds where required by by_ref() usage:
- PaxosLike::build, PaxosLike::with_client
- paxos_core, index_payloads
- compartmentalized_paxos_core
Updated IR snapshots to reflect the new singleton ref nodes.
Co-authored-by: Infinity 🤖 <infinity@hydro.run>
PR: #2903
74e5643 to
123a9c6
Compare
adac487 to
23ea024
Compare
- hydro_lang/src/compile/ir/mod.rs: Add an assert before the `ident_stack.drain()` subtraction to catch logical bugs where `ident_stack` is shorter than `singleton_refs`, avoiding a potential underflow in release builds. - hydro_lang/src/singleton_ref.rs: Fix missing space (`=SINGLETON_REFS` → `= SINGLETON_REFS`) so rustfmt formatting check passes. Co-authored-by: Infinity 🤖 <infinity@hydro.run> PR: #2929
Added `with_singleton_capture` wrapper to `Stream::filter_map` so that `SingletonRef` values can be captured inside `filter_map` closures via `q!()`, matching the existing support in `map`, `filter`, `flat_map_ordered`, `flat_map_unordered`, `partition`, and `inspect`. Co-authored-by: Infinity 🤖 <infinity@hydro.run> PR: #2903
…eton mechanism [ci-bench]
We need to remove the global singleton counter and instead use a local counter.
Converted 6 out of 7 cross_singleton usages in paxos.rs to use the new
by_ref() singleton mechanism:
- recommit_after_leader_election: 3 cross_singleton calls replaced with
by_ref() captures in filter_map and map closures
- sequence_payload: 1 cross_singleton replaced with by_ref() in map
- index_payloads (sliced!): 1 cross_singleton replaced with by_ref() in map
- acceptor_p2: 2 cross_singleton calls replaced with by_ref() captures
in filter_map and map closures
Left unchanged:
- acceptor_p1: 2 cross_singleton calls remain because the a_log parameter
comes from a forward_ref cycle, and by_ref() on cycle sources causes a
graph partitioning error ("Handoff succ not in subgraph")
Also added + 'a lifetime bounds where required by by_ref() usage:
- PaxosLike::build, PaxosLike::with_client
- paxos_core, index_payloads
- compartmentalized_paxos_core
Updated IR snapshots to reflect the new singleton ref nodes.
Co-authored-by: Infinity 🤖 <infinity@hydro.run>
PR: #2903
23ea024 to
2f9004d
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
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.
We need to remove the global singleton counter and instead use a local counter.
Converted 6 out of 7 cross_singleton usages in paxos.rs to use the new
by_ref() singleton mechanism:
by_ref() captures in filter_map and map closures
in filter_map and map closures
Left unchanged:
comes from a forward_ref cycle, and by_ref() on cycle sources causes a
graph partitioning error ("Handoff succ not in subgraph")
Also added + 'a lifetime bounds where required by by_ref() usage:
Updated IR snapshots to reflect the new singleton ref nodes.
Co-authored-by: Infinity 🤖 infinity@hydro.run