Thoughts on decoupling render pass from view updates, and better support support for immediate state change / output

[rjrjr]

Distilling slack monologue thread:

• It'd be nice to have a way to conflate renderings when they're coming "too fast".
• Thinking in particular of attractive techniques people resort to in order to have a workflow immediately emit output "without rendering", leading to cascades of dozens of render passes kicked off by a single user event.
• Typical scheme is to fire off a Observable.just(Unit).asWorker {...} on first render and have it update state / emit output.
• Can we find a way to prevent emitting renderings until the action queue is empty?
• Maybe couple this with the ability to emit output from initialState
• Maybe just restrict it to that — the only optimized way to re-render is via output from initialState

[0legg]

Both initialState and onPropsChanged are similar to the regular action minus emitting an output. Making both methods return WorkflowAction instead of WorkflowState will enable codebase unification (when initial state isn't a special case) and will reduce the described ugliness.

[0legg]

One of usecases where it could be valuable is the case when the workflow is outputting some function of its state (extremely, workflow output is the same model as a state). It's a legit usecase, especially for dumb child workflows that delegate most of handling to the parent workflow. Right now consumers have to implement the ugly hack with one-shot worker, but it's not elegant and could be racey or error-prone.

[rjrjr]

@zach-klippenstein I'm going to first play around with the simplest flavor of this I can think of: simply suppress emitting a rendering while the actionSink is not empty. I expect to find out that it's not simple at all, but at least it's a place to start digging.

I'm less excited about emitting output from initialState, if only because of the unspeakable migration nightmare it would be. I guess my hope is that I can find some low hanging fruit to reduce the penalty when people hack up their own mechanisms to achieve that effect.

I think it's well worth doing because I'm finding it's so hard to avoid an intial render storm as a deep workflow tree is set up, and all kinds of async workers are subscribed to at set up. It's hard to optimize things to avoid an initial redundant update from such workers (if you can even be confident they're redundant), and why should people have to fret about that in the first place?

[zach-klippenstein]

Could you give a more specific example of a workflow that emits output as a function of state @0legg?

[0legg]

@zach-klippenstein Please take a look at AddressWorkflow at internal repo, this is exactly the case of state and output being the same.

[steve-the-edwards]

@rjrjr I'm not so sure that the migration effort would be so tedious if we can find a way to map the WorkflowState to WorkflowAction using just static analysis of the Workflow file. We could do something similar to the WF1 migrator.

[rjrjr]

This would be a lot easier to try out if we had a single eventChannel for the entire tree rather than one per WorkflowNode. Have to wonder if that would make render passes a little faster too.

[rjrjr]

Also seems like we could be caching sub tree renderings. If no actions are queued for a tree and its props don't change, there is no reason to re-render it. Could have a big impact for split views and layers.

This would be easier if we preserve the channel-per-workflow, I think.

[steve-the-edwards]

@rjrjr I'm starting to parse out the different streams of work/thoughts here:

1. Conflating renderings in the WorkflowRunner with some frame rate before they hit the UI layer.
2. Holding off updating running the next render pass in the runtime until the event channel(s) are empty?
3. Returning State and Output from initialState.

I think 3 is about convenience for a 'smell' where these Outputs should likely be folded into the Rendering of the child. Its not performance related per se but we could mitigate the negative performance impact of the smell by changing the API.

1 and 2 are very different areas to work on the same kind of optimizations - possibly could both be used.

What I am trying to decide (and want to talk with you and @RBusarow about more) is how we want to setup a demo app that we can use to measure the impact of these changes.

I am thinking of extending poetry to have more complicated event interactions - clicking each stanza/row/letter will impact the List Rendering as well as the Detail Rendering - i.e. showing 'current' someway in the 'overview pane' and doing it with more layers (perhaps down the letter).

I am also thinking of adding in asynchronous Workers that will do a hash on the poem content to simulate 'real world' work/calculations based on the selections.

What I can't decide is if I should make this app include the commonly seen 'smells' that we have encountered in order to see what kind of optimizations will impact those as well. E.g. - should these layers have 'InitializingStates' that are waiting for that hash to run to give their first Output back to their parent - thus churning the render passes more than needed? Should we have some layers that emit Output immediately to their parent (a la Observable.just(Unit).asWorker())?

If you can't beat em, join em?

This would have the added effect of having our performance demo app be a 'bad' actor until we force it not to be (via API changes etc.). WDYT?

[rjrjr]

tl;dr: I think #2 is the most promising. Square Flow has always worked that way (though its plumbing is much simpler of course), and it worked very well.

I think #1 was just a bad attempt at articulating #2.

I've gone very cold on #3, seems like most times I dig into that it's a sign that people are using OuputT for info that can more easily be passed via RenderingT

[rjrjr]

I don't see this as a ui-1.0 blocker, moving it out of that milestone. @steve-the-edwards if you disagree, maybe want to see us hold on to the @WorkflowUiExperimentalApi annotation a bit longer for more wiggle room, please sing out.

[steve-the-edwards]

The issue with a naive implementation of (2) - "holding off on render() until action queues are empty" is that we would need to pump all the individual node's actions into one shared queue in order to be able to quickly check if it is empty. I naively implemented queue checking and it resulted in much slower performance when there wasn't an action storm.

I think we can take a hint from (1) regarding frame rate and use that as one of the conditions for re-rendering.

When we process actions:

First wait for an event to happen or props to update. If the event caused Output or there were new Props, go and render(). After this continue to loop on selecting our tick() and end on 1 of 3 conditions:

1. Output
2. Props changed
3. 'Frame' timeout of some arbitrary time - i.e. 16ms if we are talking 60fps.