Async execution RFC - #4385
Open
TrueDoctor wants to merge 3 commits into
Open
Conversation
Performance Benchmark Results
|
TrueDoctor
marked this pull request as ready for review
July 31, 2026 13:37
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 1 file
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
2 issues found across 1 file (changes from recent commits).
Confidence score: 2/5
- In
node-graph/rfcs/async-execution-model.md, the newFinalityaxis is not surfaced through the evaluation API, so executor-side handling can still conflate incomplete-inputFallbackstates with final document errors, leading to incorrect error routing and recovery behavior — extend the executor boundary contract to carry finality explicitly. - In
node-graph/rfcs/async-execution-model.md, task removal is not reliably inert when a rebuilt graph reuses the same stableSourceId, so stale completions can advance the new source generation and spuriously wake the host, causing race-driven churn or wrong scheduling — add generation/source-instance guards so old-slot completions cannot mutate reintroduced sources.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="node-graph/rfcs/async-execution-model.md">
<violation number="1" location="node-graph/rfcs/async-execution-model.md:37">
P1: The proposed separate `Finality` axis is not represented in the evaluation API, so a `Fallback` caused by an incomplete input still cannot be distinguished from a final document error at the executor boundary. Defining an explicit result/status wrapper (and updating generated eval, `Node::eval`, memoization, and propagation rules to carry it) would make the provisional-error behavior implementable.</violation>
<violation number="2" location="node-graph/rfcs/async-execution-model.md:122">
P2: A removed task is not reliably inert when a rebuilt graph reintroduces the same stable `SourceId`: its completion can bump the new source's generation and wake the host even though the result was written to the old slot map. Associating each task with a per-compilation/source incarnation token and checking that token before applying the generation bump would prevent stale completions from affecting a replacement source.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| The variants split into "has a value" (`Final`, `Partial`, `Fallback`) and "has no value" (`Pending`, `Error`). Value-carrying statuses flow through computation: a node receiving `Partial` computes normally and downgrades its own output; a node receiving `Fallback` computes on the stand-in and forwards the error. Valueless statuses stop the cone for this frame. Statuses combine with a meet (`Final ∧ Final = Final`, anything else drags the result down; `Error` dominates `Pending`). | ||
|
|
||
| `meet(Partial, Fallback)` collapses to `Fallback`, which alone would make an error from a still-incomplete source indistinguishable from a final document error. Finality therefore rides a separate `Finality` (`AllFinal ∧ Partial`) tracked beside the status rather than a sixth variant, so the executor can withhold provisional errors from the document without widening `GPoll`. |
Contributor
There was a problem hiding this comment.
P1: The proposed separate Finality axis is not represented in the evaluation API, so a Fallback caused by an incomplete input still cannot be distinguished from a final document error at the executor boundary. Defining an explicit result/status wrapper (and updating generated eval, Node::eval, memoization, and propagation rules to carry it) would make the provisional-error behavior implementable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/rfcs/async-execution-model.md, line 37:
<comment>The proposed separate `Finality` axis is not represented in the evaluation API, so a `Fallback` caused by an incomplete input still cannot be distinguished from a final document error at the executor boundary. Defining an explicit result/status wrapper (and updating generated eval, `Node::eval`, memoization, and propagation rules to carry it) would make the provisional-error behavior implementable.</comment>
<file context>
@@ -34,6 +34,8 @@ enum GPoll<T> {
The variants split into "has a value" (`Final`, `Partial`, `Fallback`) and "has no value" (`Pending`, `Error`). Value-carrying statuses flow through computation: a node receiving `Partial` computes normally and downgrades its own output; a node receiving `Fallback` computes on the stand-in and forwards the error. Valueless statuses stop the cone for this frame. Statuses combine with a meet (`Final ∧ Final = Final`, anything else drags the result down; `Error` dominates `Pending`).
+`meet(Partial, Fallback)` collapses to `Fallback`, which alone would make an error from a still-incomplete source indistinguishable from a final document error. Finality therefore rides a separate `Finality` (`AllFinal ∧ Partial`) tracked beside the status rather than a sixth variant, so the executor can withhold provisional errors from the document without widening `GPoll`.
+
Layout: one tag plus one payload word for pointer-sized values (`GPoll<Arc<T>>` is 16 bytes); the stand-in and its error share a box, and `Error` boxes the error alone, so it crosses node boundaries without reallocating. Progress percentages are deliberately not part of the value: they change at high frequency and must not invalidate the dataflow, so they ride the monitor side channel.
</file context>
|
|
||
| Wakers are deliberately not the graph-level notification mechanism: wakers resume suspended computations and this graph never suspends; a re-evaluation is a fresh call. Wake also does not mean completion, and wakers do not survive composition across host executors. The host avoids busy-waiting with one coarse notification (park/unpark or event-loop message) shared by task wakes and the dirty epilogue. | ||
|
|
||
| Scheduling semantics: generations are read once per evaluation (completions landing mid-frame affect the next one); the dirty flag is drained once per frame (N completions, one re-render); on recompile, `retain_sources` drops removed sources from the table, so their epilogue still runs but finds no entry and bumps nothing, neither invalidating nor waking the host. Tasks are not cancelled; they are made inert. |
Contributor
There was a problem hiding this comment.
P2: A removed task is not reliably inert when a rebuilt graph reintroduces the same stable SourceId: its completion can bump the new source's generation and wake the host even though the result was written to the old slot map. Associating each task with a per-compilation/source incarnation token and checking that token before applying the generation bump would prevent stale completions from affecting a replacement source.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/rfcs/async-execution-model.md, line 122:
<comment>A removed task is not reliably inert when a rebuilt graph reintroduces the same stable `SourceId`: its completion can bump the new source's generation and wake the host even though the result was written to the old slot map. Associating each task with a per-compilation/source incarnation token and checking that token before applying the generation bump would prevent stale completions from affecting a replacement source.</comment>
<file context>
@@ -114,7 +119,7 @@ The execution path itself is synchronous: executor construction, update, and `ex
Wakers are deliberately not the graph-level notification mechanism: wakers resume suspended computations and this graph never suspends; a re-evaluation is a fresh call. Wake also does not mean completion, and wakers do not survive composition across host executors. The host avoids busy-waiting with one coarse notification (park/unpark or event-loop message) shared by task wakes and the dirty epilogue.
-Scheduling semantics: generations are read once per evaluation (completions landing mid-frame affect the next one); the dirty flag is drained once per frame (N completions, one re-render); on recompile, tasks of removed sources are dropped and the epilogue never runs, so no spurious invalidation.
+Scheduling semantics: generations are read once per evaluation (completions landing mid-frame affect the next one); the dirty flag is drained once per frame (N completions, one re-render); on recompile, `retain_sources` drops removed sources from the table, so their epilogue still runs but finds no entry and bumps nothing, neither invalidating nor waking the host. Tasks are not cancelled; they are made inert.
# Reference-level explanation
</file context>
Performance Benchmark Results
|
Performance Benchmark Results
|
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
1 participant
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.
No description provided.