Skip to content

Conversation

@shadaj
Copy link
Member

@shadaj shadaj commented Sep 20, 2025

Previously, the Hydro -> Hydro IR layer did a bunch of work to translate between Flo semantics and DFIR semantics, so that Hydro IR -> DFIR was generally 1:1. In preparation for the simulator and DFIR's migration to Flo semantics, we now use Flo semantics in the Hydro IR (top level operators do not reset their state on batch boundaries), and shift the Flo -> DFIR semantics translation to the IR -> DFIR layer.

This also comes with a breaking API change to clean up naming and avoid function overloading. In particular, batch / snapshot are renamed to batch_atomic and snapshot_atomic for the case where the batched result is supposed to be in-sync with the atomic execution context.

There were a couple of bugs during the transition to the new IR semantics that were not caught by existing unit tests, so I added additional tests that cover each of them.

Also makes some minor improvements / bug fixes the way:

  • Restricts KeyedSingleton::snapshot to unbounded-value only, since bounded value keyed singletons do not replay their elements
  • Groups together definitions for resolve_futures and resolve_futures_ordered
  • Removes various unnecessary NoAtomic trait bounds

Stack created with Sapling. Best reviewed with ReviewStack.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Sep 20, 2025

Deploying hydro with  Cloudflare Pages  Cloudflare Pages

Latest commit: 62094c0
Status: ✅  Deploy successful!
Preview URL: https://f3c7c47f.hydroflow.pages.dev
Branch Preview URL: https://pr2136.hydroflow.pages.dev

View logs

@shadaj shadaj marked this pull request as draft September 22, 2025 18:12
@shadaj
Copy link
Member Author

shadaj commented Sep 22, 2025

Draft until I add unit tests for regressions I caught during development.

@shadaj shadaj marked this pull request as ready for review September 22, 2025 21:42
@shadaj shadaj closed this Sep 22, 2025
@shadaj shadaj reopened this Sep 22, 2025
@shadaj shadaj force-pushed the pr2136 branch 4 times, most recently from 862275b to 0b8922d Compare September 22, 2025 23:21
…levels

Previously, the Hydro -> Hydro IR layer did a bunch of work to translate between Flo semantics and DFIR semantics, so that Hydro IR -> DFIR was generally 1:1. In preparation for the simulator and DFIR's migration to Flo semantics, we now use Flo semantics in the Hydro IR (top level operators do not reset their state on batch boundaries), and shift the Flo -> DFIR semantics translation to the IR -> DFIR layer.

This also comes with a breaking API change to clean up naming and avoid function overloading. In particular, `batch` / `snapshot` are renamed to `batch_atomic` and `snapshot_atomic` for the case where the batched result is supposed to be in-sync with the atomic execution context.

There were a couple of bugs during the transition to the new IR semantics that were not caught by existing unit tests, so I added additional tests that cover each of them.

Also makes some minor improvements / bug fixes the way:
- Restricts `KeyedSingleton::snapshot` to unbounded-value only, since bounded value keyed singletons do not replay their elements
- Groups together definitions for `resolve_futures` and `resolve_futures_ordered`
- Removes various unnecessary `NoAtomic` trait bounds
metadata: HydroIrMetadata,
},

YieldConcat {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is YieldConcat?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Flo-terminology, yields the bounded collection inside the tick to outside the tick by using the "concat" operator (++) for that collection type. The concat is the "natural" combination method, appending elements for stream, replacing the value for singleton, etc.

Comment on lines +2030 to +2032
// if both inputs are root, the output is expected to have streamy semantics, so we need
// a multiset_delta() to negate the replay behavior
parse_quote! {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have an issue to fix this at the DFIR level?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#source_ident = source_iter(#expr);
}
} else {
// TODO(shadaj): a more natural semantics would be to to re-evaluate the expression on each tick
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue for this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shadaj shadaj merged commit a4d8af6 into main Sep 23, 2025
18 checks passed
@shadaj shadaj deleted the pr2136 branch September 23, 2025 18:14
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.

5 participants