1093: Yield in runtime to run side effects that were launched lazily #1096
Conversation
steve-the-edwards
force-pushed
the
sedwards/lifecycle-worker-yield
branch
4 times, most recently
from
3202cb7 to
f896709
Compare
| // And emit the Output. | ||
| sendOutput(actionResult, onOutput) | ||
| // Yield if there are any side effects (they were launched lazily) that need starting. | ||
| yield() |
There was a problem hiding this comment.
Are you sure we shouldn't yield() first, before we allow the UI to update?
There was a problem hiding this comment.
in order to get the side effects started before then you mean? Or because we don't want the collectors of the StateFlow of renderings to be in contention for dispatch at the same time?
There was a problem hiding this comment.
The former was what I was thinking of -- seems like a side effect should happen, or start happening, before / while the UI is updating. Am I over or under thinking it?
There was a problem hiding this comment.
Thought exercise: what happens if something on the receiving end of sendOutput kills off the workflow runtime before that yield() happens (if it that's possible). Would we be surprised that the last declared side effects were never run? I think we would.
There was a problem hiding this comment.
all legitimate points. I adjusted the ordering to yield() immediately after rendering in the updated PR.
steve-the-edwards
force-pushed
the
sedwards/lifecycle-worker-yield
branch
from
f896709 to
44d9e39
Compare
Add headlessIntegrationTest to renderWorkflowIn with a nice Turbine attached. Increase perf benchmarks render pass expectations as yield() will do that. Closes #1093
steve-the-edwards
force-pushed
the
sedwards/lifecycle-worker-yield
branch
from
44d9e39 to
4396fb2
Compare
|
This isn't desirable as it creates unnecessary thrash after the |
|
Closing and this is not needed. It doesn't work and we went with #1106 instead. |
Add in some
yield()in the runtime so that we can start any running side effect after each render() before processing the next action, which might tear down that side effect's Job.Side Effect Job's are launched lazily in case they send to teh actionSink (so we can complete the render before they get dispatched).
Also brings in
headlessIntegrationTestwhich was being used internally for some time.Brings this in in order to right some tests for whether or not these side effects are run under certain circumstances.
Closes #1093