Skip to content
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

Add Configurable Recursive Rendering to Performance Poetry #903

Merged
merged 1 commit into from
Dec 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,20 @@ import com.squareup.benchmarks.performance.complex.poetry.PerformancePoemsBrowse
import com.squareup.benchmarks.performance.complex.poetry.PerformancePoemsBrowserWorkflow.State.ComplexCall
import com.squareup.benchmarks.performance.complex.poetry.PerformancePoemsBrowserWorkflow.State.Initializing
import com.squareup.benchmarks.performance.complex.poetry.PerformancePoemsBrowserWorkflow.State.NoSelection
import com.squareup.benchmarks.performance.complex.poetry.PerformancePoemsBrowserWorkflow.State.Recurse
import com.squareup.benchmarks.performance.complex.poetry.PerformancePoemsBrowserWorkflow.State.Selected
import com.squareup.benchmarks.performance.complex.poetry.instrumentation.ActionHandlingTracingInterceptor
import com.squareup.benchmarks.performance.complex.poetry.instrumentation.SimulatedPerfConfig
import com.squareup.benchmarks.performance.complex.poetry.instrumentation.TraceableWorker
import com.squareup.benchmarks.performance.complex.poetry.instrumentation.asTraceableWorker
import com.squareup.benchmarks.performance.complex.poetry.views.BlankScreen
import com.squareup.sample.container.overviewdetail.OverviewDetailScreen
import com.squareup.sample.poetry.ConfigAndPoems
import com.squareup.sample.poetry.PoemListScreen.Companion.NO_POEM_SELECTED
import com.squareup.sample.poetry.PoemListWorkflow
import com.squareup.sample.poetry.PoemListWorkflow.Props
import com.squareup.sample.poetry.PoemWorkflow
import com.squareup.sample.poetry.PoemsBrowserWorkflow
import com.squareup.sample.poetry.model.Poem
import com.squareup.workflow1.Snapshot
import com.squareup.workflow1.StatefulWorkflow
import com.squareup.workflow1.WorkflowAction.Companion.noAction
Expand Down Expand Up @@ -50,9 +51,11 @@ class PerformancePoemsBrowserWorkflow(
private val isLoading: MutableStateFlow<Boolean>,
) :
PoemsBrowserWorkflow,
StatefulWorkflow<List<Poem>, State, Unit, OverviewDetailScreen>() {
StatefulWorkflow<ConfigAndPoems, State, Unit, OverviewDetailScreen>() {

sealed class State {
object Recurse : State()

// N.B. This state is a smell. We include it to be able to mimic smells
// we encounter in real life. Best practice would be to fold it
// into [NoSelection] at the very least.
Expand All @@ -66,36 +69,63 @@ class PerformancePoemsBrowserWorkflow(
}

override fun initialState(
props: List<Poem>,
props: ConfigAndPoems,
snapshot: Snapshot?
): State {
return if (simulatedPerfConfig.useInitializingState) Initializing else NoSelection
return if (props.first.first > 0 &&
props.first.second == simulatedPerfConfig.recursionGraph.second
) {
Recurse
} else if (simulatedPerfConfig.useInitializingState) {
Initializing
} else {
NoSelection
}
}

@OptIn(WorkflowUiExperimentalApi::class)
override fun render(
renderProps: List<Poem>,
renderProps: ConfigAndPoems,
renderState: State,
context: RenderContext
): OverviewDetailScreen {
if (simulatedPerfConfig.simultaneousActions > 0) {
repeat(simulatedPerfConfig.simultaneousActions) { index ->
context.runningWorker(
worker = isLoading.asTraceableWorker("SimultaneousSubscribeBrowser-$index"),
key = "Browser-$index"
when (renderState) {
is Recurse -> {
val recursiveChild = PerformancePoemsBrowserWorkflow(
simulatedPerfConfig,
poemWorkflow,
isLoading,
)
repeat(renderProps.first.second) { breadth ->
val nextProps = renderProps.copy(
first = renderProps.first.first - 1 to breadth
)
// When we repeat horizontally we ask the runtime to 'render' these Workflow nodes but
// we don't use their renderings in what is passed to the UI layer as this is just to
// fill out the Workflow tree to give the runtime more work to do. As such, we call
// renderChild here but we ignore the rendering returned and do no action on any Output.
// See SimulatedPerfConfig kdoc for more explanation.
context.renderChild(
child = recursiveChild,
props = nextProps,
key = "${nextProps.first},${nextProps.second}",
) {
noAction()
}
}
val nextProps = renderProps.copy(
first = renderProps.first.first - 1 to renderProps.first.second
)
return context.renderChild(
child = recursiveChild,
props = nextProps,
key = "${nextProps.first},${nextProps.second}",
) {
noAction()
action {
setOutput(it)
}
}
}
}
val poemListProps = Props(
poems = renderProps,
eventHandlerTag = ActionHandlingTracingInterceptor::keyForTrace
)
val poemListRendering = context.renderChild(PoemListWorkflow, poemListProps) { selected ->
choosePoem(selected)
}
when (renderState) {
// Again, then entire `Initializing` state is a smell, which is most obvious from the
// use of `Worker.from { Unit }`. A Worker doing no work and only shuttling the state
// along is usually the sign you have an extraneous state that can be collapsed!
Expand All @@ -110,58 +140,86 @@ class PerformancePoemsBrowserWorkflow(
}
return OverviewDetailScreen(overviewRendering = BackStackScreen(BlankScreen))
}
is NoSelection -> {
return OverviewDetailScreen(
overviewRendering = BackStackScreen(
poemListRendering.copy(selection = NO_POEM_SELECTED)
)

is ComplexCall, is NoSelection, is Selected -> {
if (simulatedPerfConfig.simultaneousActions > 0) {
repeat(simulatedPerfConfig.simultaneousActions) { index ->
context.runningWorker(
worker = isLoading.asTraceableWorker("SimultaneousSubscribeBrowser-$index"),
key = "Browser-$index"
) {
noAction()
}
}
}
val poemListProps = Props(
poems = renderProps.second,
eventHandlerTag = ActionHandlingTracingInterceptor::keyForTrace
)
}
is ComplexCall -> {
context.runningWorker(
TraceableWorker.from("ComplexCallBrowser(${renderState.payload})") {
isLoading.value = true
delay(simulatedPerfConfig.complexityDelay)
// No Output for Worker is necessary because the selected index
// is already in the state.
val poemListRendering = context.renderChild(PoemListWorkflow, poemListProps) { selected ->
choosePoem(selected)
}
when (renderState) {
is NoSelection -> {
return OverviewDetailScreen(
overviewRendering = BackStackScreen(
poemListRendering.copy(selection = NO_POEM_SELECTED)
)
)
}
) {
action {
isLoading.value = false
(state as? ComplexCall)?.let { currentState ->
state = if (currentState.payload != NO_POEM_SELECTED) {
Selected(currentState.payload)
} else {
NoSelection

is ComplexCall -> {
context.runningWorker(
TraceableWorker.from("ComplexCallBrowser(${renderState.payload})") {
isLoading.value = true
delay(simulatedPerfConfig.complexityDelay)
// No Output for Worker is necessary because the selected index
// is already in the state.
}
) {
action {
isLoading.value = false
(state as? ComplexCall)?.let { currentState ->
state = if (currentState.payload != NO_POEM_SELECTED) {
Selected(currentState.payload)
} else {
NoSelection
}
}
}
}
var poems = OverviewDetailScreen(
overviewRendering = BackStackScreen(
poemListRendering.copy(selection = renderState.payload)
)
)
if (renderState.payload != NO_POEM_SELECTED) {
val poem: OverviewDetailScreen = context.renderChild(
poemWorkflow,
renderProps.second[renderState.payload]
) { clearSelection }
poems += poem
}
return poems
}

is Selected -> {
val poems = OverviewDetailScreen(
overviewRendering = BackStackScreen(
poemListRendering.copy(selection = renderState.poemIndex)
)
)
val poem: OverviewDetailScreen = context.renderChild(
poemWorkflow,
renderProps.second[renderState.poemIndex]
) { clearSelection }
return poems + poem
}

else -> {
throw IllegalStateException("State can't change while rendering.")
}
}
var poems = OverviewDetailScreen(
overviewRendering = BackStackScreen(
poemListRendering.copy(selection = renderState.payload)
)
)
if (renderState.payload != NO_POEM_SELECTED) {
val poem: OverviewDetailScreen = context.renderChild(
poemWorkflow,
renderProps[renderState.payload]
) { clearSelection }
poems += poem
}
return poems
}
is Selected -> {
val poems = OverviewDetailScreen(
overviewRendering = BackStackScreen(
poemListRendering.copy(selection = renderState.poemIndex)
)
)
val poem: OverviewDetailScreen = context.renderChild(
poemWorkflow,
renderProps[renderState.poemIndex]
) { clearSelection }
return poems + poem
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,15 @@ class PerformancePoetryActivity : AppCompatActivity() {

setupMainLooperTracing()

val recursiveDepth = intent.getIntExtra(EXTRA_PERF_CONFIG_RECURSION_DEPTH, 0)
val recursiveBreadth = intent.getIntExtra(EXTRA_PERF_CONFIG_RECURSION_BREADTH, 0)

// Default is just to have the basic 'delay' complexity.
val simulatedPerfConfig = SimulatedPerfConfig(
isComplex = true,
complexityDelay = intent.getLongExtra(EXTRA_PERF_CONFIG_DELAY, 200L),
useInitializingState = intent.getBooleanExtra(EXTRA_SCENARIO_CONFIG_INITIALIZING, false),
recursionGraph = recursiveDepth to recursiveBreadth,
repeatOnNext = intent.getIntExtra(EXTRA_PERF_CONFIG_REPEAT, 0),
simultaneousActions = intent.getIntExtra(EXTRA_PERF_CONFIG_SIMULTANEOUS, 0),
traceFrameLatency = intent.getBooleanExtra(EXTRA_TRACE_FRAME_LATENCY, false),
Expand Down Expand Up @@ -242,6 +246,10 @@ class PerformancePoetryActivity : AppCompatActivity() {
const val EXTRA_PERF_CONFIG_REPEAT = "complex.poetry.performance.config.repeat.amount"
const val EXTRA_PERF_CONFIG_DELAY = "complex.poetry.performance.config.delay.length"
const val EXTRA_PERF_CONFIG_SIMULTANEOUS = "complex.poetry.performance.config.simultaneous"
const val EXTRA_PERF_CONFIG_RECURSION_DEPTH =
"complex.poetry.performance.config.recursion.depth"
const val EXTRA_PERF_CONFIG_RECURSION_BREADTH =
"complex.poetry.performance.config.recursion.breadth"

const val SELECT_ON_TIMEOUT_LOG_NAME =
"kotlinx.coroutines.selects.SelectBuilderImpl\$onTimeout\$\$inlined\$Runnable"
Expand All @@ -257,7 +265,7 @@ class PerformancePoetryActivity : AppCompatActivity() {

class PoetryModel(
savedState: SavedStateHandle,
workflow: MaybeLoadingGatekeeperWorkflow<List<Poem>>,
workflow: MaybeLoadingGatekeeperWorkflow<Pair<Pair<Int, Int>, List<Poem>>>,
interceptor: WorkflowInterceptor?,
runtimeConfig: RuntimeConfig
) : ViewModel() {
Expand All @@ -274,7 +282,7 @@ class PoetryModel(

class Factory(
owner: SavedStateRegistryOwner,
private val workflow: MaybeLoadingGatekeeperWorkflow<List<Poem>>,
private val workflow: MaybeLoadingGatekeeperWorkflow<Pair<Pair<Int, Int>, List<Poem>>>,
private val workflowInterceptor: WorkflowInterceptor?,
private val runtimeConfig: RuntimeConfig
) : AbstractSavedStateViewModelFactory(owner, null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class PerformancePoetryComponent(

private val loadingGatekeeperForPoems = MaybeLoadingGatekeeperWorkflow(
childWithLoading = poemsBrowserWorkflow,
childProps = Poem.allPoems,
childProps = Pair(simulatedPerfConfig.recursionGraph, Poem.allPoems),
browserIsLoading.combine(poemIsLoading) { one, two -> one || two }
)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,42 @@
package com.squareup.benchmarks.performance.complex.poetry.instrumentation

import android.os.Parcelable
import com.squareup.sample.poetry.RecursionGraphConfig
import kotlinx.parcelize.Parcelize

/**
* We use this to 'simulate' different performance scenarios that we have seen that we want to
* be able to benchmark and monitor. Firstly we have a complexity which is just used to add some
* delay to selection activities - only use with Poetry right now.
*
* [isComplex] Determines whether or not we start a Worker in between state transitions that
* roughly approximates doing I/O work or a network call.
* [complexityDelay] Is the length of the delay for the work in between the state changes if
* [isComplex] is true.
* [useInitializingState] is a smell we have observed whereby an 'initializing' state is used
* while waiting for the first values before doing the real Workflow work.
* while waiting for the first values before doing the real Workflow work.
* [recursionGraph] Determines the shape of the tree that is rendered by the Workflow runtime. The
* first number n is the recursive 'depth'. This will have the [PerformancePoemBrowserWorkflow]
* render itself recursively n times, before rendering its actual children - the poem list and
* the poem. The second number m is the 'breadth'. This means that for each recursive depth layer
* n, the [PerformancePoemBrowserWorkflow] will also be rendered as siblings m times. Only the
* (m-1)th rendering is returned from render() though which means that the other renders will be
* work for the runtime to render but they won't be passed to the UI layer.
* [repeatOnNext] Is the number of times x that an action should be created when 'next' is pressed
* in the poem before the action takes its effect - advances the state to the next stanza.
* [simultaneousActions] Determines the number of workers y that should be created for each
* 'complex' state transition. This will result in y actions that need to be handled
* simultaneously and tries to represent a scenario of multiple Workflows listening to the same
* action.
* [traceRenderingPasses], [traceFrameLatency], [traceEventLatency] are all flags to add
* instrumentation for different performance measurements.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rjrjr @RBusarow - I added a description for recursionGraph here and while I was added I described each of the other config levers.

*/
@Parcelize
data class SimulatedPerfConfig(
val isComplex: Boolean,
val complexityDelay: Long,
val useInitializingState: Boolean,
val recursionGraph: RecursionGraphConfig = 0 to 0,
val repeatOnNext: Int = 0,
val simultaneousActions: Int = 0,
val traceRenderingPasses: Boolean = false,
Expand All @@ -27,6 +48,7 @@ data class SimulatedPerfConfig(
isComplex = false,
complexityDelay = 0,
useInitializingState = false,
recursionGraph = 0 to 0,
repeatOnNext = 0,
simultaneousActions = 0,
traceRenderingPasses = false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class PoetryModel(savedState: SavedStateHandle) : ViewModel() {
renderWorkflowIn(
workflow = RealPoemsBrowserWorkflow(RealPoemWorkflow()),
scope = viewModelScope,
prop = Poem.allPoems,
prop = 0 to 0 to Poem.allPoems,
savedStateHandle = savedState,
runtimeConfig = AndroidRuntimeConfigTools.getAppWorkflowRuntimeConfig()
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,14 @@ import com.squareup.sample.container.overviewdetail.OverviewDetailScreen
import com.squareup.sample.poetry.model.Poem
import com.squareup.workflow1.Workflow

typealias RecursionGraphConfig = Pair<Int, Int>
typealias ConfigAndPoems = Pair<RecursionGraphConfig, List<Poem>>

/**
* Provides an overview / detail treatment of a list of [Poem]s.
*
* (Defining this as an interface allows us to use other implementations
* in other contexts -- check out our :benchmarks module!)
*/
interface PoemsBrowserWorkflow : Workflow<List<Poem>, Unit, OverviewDetailScreen>
interface PoemsBrowserWorkflow :
Workflow<ConfigAndPoems, Unit, OverviewDetailScreen>
Loading