chore: let async derived coordinate batches#17955
Conversation
Instead of having batches coordinate whether they should block on other batches, or rebase earlier/later ones, we move the coordination into the async deriveds. More concretely, we always run async operations in order on the async_derived level. During function invocation we temporarily remove batch_values so that the latest value across all batches is retrieved. Because we do that we have to wait on prior runs to resolve before resolving ours, except when the batch in question is a subset of the current batch; in that case we can resolve directly and reject the async derived of the other batch as stale. This combination of things (latest value across all batches but resolving in order except when we don't need to wait) allows us to get rid of the `batch.#commit()` logic.
|
|
| */ | ||
| function is_batch_subset(subset, superset) { | ||
| for (const source of subset.current.keys()) { | ||
| if ((source.f & DERIVED) !== 0) continue; |
There was a problem hiding this comment.
his probably also needs the "this is a writable derived" differentiation
| } finally { | ||
| set_ignore_batch_values(previous_ignore_batch_values); | ||
| set_has_batch_value_differences(previous_has_batch_value_differences); | ||
| } |
There was a problem hiding this comment.
I wonder if this part of the PR is useful outside of it, too
| // ASYNC effects should read canonical signal values instead of batch overlays | ||
| // while they run. |
There was a problem hiding this comment.
maybe this logic should live in async_derived?
There was a problem hiding this comment.
oh wait I see it already does? do we need both?
There was a problem hiding this comment.
might be we no longer need both / only this. fear was that we could rerun async deriveds at a different point that this one.
We definitely need this part because is_dirty can execute deriveds and those already need to compute/return the correct values.
| <button>a</button> | ||
| <button>b</button> | ||
| <button>resolve</button> | ||
| hi | ||
| 1 |
There was a problem hiding this comment.
what is going on with the indentation here 😆
|
Unfortunately the |
2 participants
Instead of having batches coordinate whether they should block on other batches, or rebase earlier/later ones, we move the coordination into the async deriveds.
More concretely, we always run async operations in order on the async_derived level. During function invocation we temporarily ignore batch_values so that the latest value across all batches is retrieved (to get that working properly we may need to execute deriveds twice to get the value in the context of batch_values and the latest value across all batches). Because we do that we have to wait on prior runs to resolve before resolving ours, except when the batch in question is a subset of the current batch; in that case we can resolve directly and reject the async derived of the other batch as stale. This combination of things (latest value across all batches but resolving in order except when we don't need to wait) allows us to get rid of the
batch.#commit()logic.This makes all existing tests pass (only one needed to be adjusted) but one new test and one adjusted test in #17162 fail here.