Skip to content

#1311: Start side effects DEFAULT instead of LAZY. #1312

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

steve-the-edwards
Copy link
Contributor

@steve-the-edwards steve-the-edwards commented May 13, 2025

See #1311 for more information on the problem.

The first commit is a red test that will test that behaviour on Android with the Main.immediate dispatcher.

The second commit makes that test green!!

It does so by launching side effects with DEFAULT instead of LAZY. That means that with an eager dispatcher like Main.immediate your workers and side effects will be eagerly entered as they are created during render. We want to maintain the distinction that we cannot send to the sink in render() but we need to be able to do so in a worker/side effect. So we freeze the RenderContext whenever we dispatch the coroutine for that side effect (and then unfreeze it as necessary when that is complete) using a ContinuationInterceptor.

Note that this is a pretty significant change to the contract for workers/side effects; we've explored this before without making any significant changes, preferring instead to provide the CoroutineScope of the Workflow with a SessionWorkflow ( #1102, #1093 ).

In this instance our motivation is to enable further optimization and it is a pretty good motivation!

We will want to reflect on ramifications of this semantics change.

@steve-the-edwards steve-the-edwards changed the title 1311: Add failing test for Main.immediate conflation #1311: Add failing test for Main.immediate conflation May 13, 2025
@steve-the-edwards steve-the-edwards changed the title #1311: Add failing test for Main.immediate conflation #1311: Start side effects DEFAULT instead of LAZY. May 13, 2025
@steve-the-edwards steve-the-edwards force-pushed the sedwards/fix-conflate-tests branch from c438b1c to 130dc0d Compare May 13, 2025 20:07
@steve-the-edwards steve-the-edwards force-pushed the sedwards/fix-conflate-tests branch from 130dc0d to 92c81b5 Compare May 13, 2025 20:31
@steve-the-edwards
Copy link
Contributor Author

steve-the-edwards commented May 13, 2025

errr.... if that's possible. Need to do some more reading on that thread.

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.

1 participant