Skip to content

Merge main into ray/ui-update #724

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

Merged
merged 22 commits into from
Apr 14, 2022
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
e466fac
Allow repeatOnLifecycle state to be configurable.
c-soon Mar 15, 2022
8677f21
Merge pull request #701 from c-soon/csoon/addNewParamToStartWorkflowL…
rjrjr Mar 18, 2022
5763982
699: Add simulated performance config to Poetry
steve-the-edwards Mar 17, 2022
6e511e8
Merge pull request #704 from square/sedwards/699-benchmark-sample
steve-the-edwards Mar 29, 2022
3a865d2
Refactor Perf Poetry to add Related Benchmarks
steve-the-edwards Mar 29, 2022
64e3073
Merge pull request #711 from square/sedwards/update-benchmarks-readme
steve-the-edwards Mar 29, 2022
d59fe8c
Update AGP versions and OptIn Notation
steve-the-edwards Apr 8, 2022
80119ba
Merge pull request #717 from square/sedwards/update-kotlin-agp
steve-the-edwards Apr 8, 2022
c515971
Fixes memory leak in ViewStateCache.
rjrjr Apr 8, 2022
9ccf42a
Exclude Benchmarks for connectedCheck verification instructions
steve-the-edwards Apr 12, 2022
597ed49
Merge pull request #720 from square/sedwards/update-connected-check
rjrjr Apr 12, 2022
cd7bd1b
Merge pull request #718 from square/ray/view-state-cache-leak-oops
rjrjr Apr 12, 2022
02524d7
Fixes a typo in step 1 of RELEASING.md
rjrjr Apr 12, 2022
b5fc065
Releasing v1.7.0
rjrjr Apr 12, 2022
244d58e
Finish releasing v1.7.0
rjrjr Apr 12, 2022
d5b600a
Bump okio to 3.0.0
bnvinay92 Apr 13, 2022
677abbd
Merge pull request #721 from bnvinay92/vn/bump-okio
rjrjr Apr 13, 2022
7f80c33
Bump tutorials to 1.7.0
rjrjr Apr 13, 2022
15bbeea
Merge pull request #722 from square/ray/update-tuts-1.7.0
rjrjr Apr 13, 2022
d664df4
Merge remote-tracking branch 'origin/main' into ray/catchup-1.7.0
rjrjr Apr 14, 2022
2bc8690
Fixes unused imports on LoaderSpinner
rjrjr Apr 14, 2022
5e72bed
Deletes extra copy of AndroidOverlay
rjrjr Apr 14, 2022
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
2 changes: 2 additions & 0 deletions .buildscript/binary-validation.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,6 @@ apiValidation {
// Only leaf project name is valid configuration, and every project must be individually ignored.
// See https://github.com/Kotlin/binary-compatibility-validator/issues/3
ignoredProjects += project('samples').subprojects.collect { it.name }
ignoredProjects += project('benchmarks').subprojects.collect { it.name }
}

3 changes: 2 additions & 1 deletion RELEASING.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@
1. Make sure you're on the `main` branch (or fix branch, e.g. `v0.1-fixes`).

1. Confirm that the kotlin build is green before committing any changes
(Note we exclude benchmarks, but you can check those too!)
```bash
./gradlew build connectedCheck
./gradlew build && ./gradlew connectedCheck -x :benchmarks:dungeon-benchmark:connectedCheck
```

1. Update your tags.
Expand Down
32 changes: 24 additions & 8 deletions benchmarks/README.md
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
# benchmark
# benchmarks

This module contains benchmarks. Used to measure and help improve Workflow performance. These tests
should be run on physical devices.

## dungeon-benchmark

Currently this is the only benchmark included here. It is for the /samples/dungeon/app. This
includes a macrobenchmark [test](benchmarks/dungeon-benchmark/src/main/java/com/squareup/sample/dungeon/benchmark/WorkflowBaselineProfiles.kt)
that exercises this sample app (with UiAutomator) to collect a [baseline profile](https://developer.android.com/studio/profile/baselineprofiles).
After running this the `profile.txt` file can be taken off of the device and used directly in the
/src/main directory of the application. This will include the profile into the APK for guided
optimization at install time.
This is for the /samples/dungeon/app. This includes a macrobenchmark
[test](dungeon-benchmark/src/main/java/com/squareup/sample/dungeon/benchmark/WorkflowBaselineProfiles.kt)
that exercises this sample app (with UiAutomator) to collect
a [baseline profile](https://developer.android.com/studio/profile/baselineprofiles). After running
this the `profile.txt` file can be taken off of the device and used directly in the /src/main
directory of the application. This will include the profile into the APK for guided optimization at
install time.

Better yet, the profile can be split up and added into each Workflow module's src directory so that
it will be included with all APKs built using Workflow (including 3rd party). To do this a java
Expand All @@ -28,8 +29,23 @@ This will create an output file separated by module and then also by package as
profile for each module can be added into its /src/main directory as `baseline-prof.txt`. Then on a
release build this will be included with the resulting APK/binary.

The other [test](benchmarks/dungeon-benchmark/src/main/java/com/squareup/sample/dungeon/benchmark/WorkflowBaselineBenchmark.kt)
The other [test](dungeon-benchmark/src/main/java/com/squareup/sample/dungeon/benchmark/WorkflowBaselineBenchmark.kt)
is used to verify the results of including the baseline profiles on the startup time. This runs the
same scenario with and without forcing the use of the profiles. To force the use of profiles, the
`libs.androidx.profileinstaller` dependency is included into the app under profile (dungeon in this
case) for side-loading the profiles.

## performance-poetry

Module of code for performance testing related to poetry applications.

### complex-poetry

This application is a modification of the samples/containers/app-poetry app which uses also the
common components in samples/containers/common and samples/containers/poetry. It modifies this
application to pass the Workflow
a [SimulatedPerfConfig](performance-poetry/complex-poetry/src/main/java/com/squareup/benchmarks/performance/poetry/SimulatedPerfConfig.kt).

In this case we specify that the app should be more 'complex' which adds delays into each of the
selections that are run by Worker's which then trigger a loading state that is handled by the
[MaybeLoadingGatekeeperWorkflow](performance-poetry/complex-poetry/src/main/java/com/squareup/benchmarks/performance/poetry/MaybeLoadingGatekeeperWorkflow.kt).
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ apply(from = rootProject.file(".buildscript/android-ui-tests.gradle"))

android {
defaultConfig {
applicationId = "com.squareup.benchmarks.performance.poetry"
applicationId = "com.squareup.benchmarks.performance.complex.poetry"
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
<?xml version="1.0" encoding="utf-8"?>
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:tools="http://schemas.android.com/tools"
package="com.squareup.benchmarks.performance.poetry">
package="com.squareup.benchmarks.performance.complex.poetry">

<application
android:allowBackup="false"
android:label="@string/app_name"
android:theme="@style/AppTheme"
tools:ignore="AllowBackup,GoogleAppIndexingWarning,MissingApplicationIcon">
<activity
android:name=".PerformancePoetryActivity"
android:name="com.squareup.benchmarks.performance.complex.poetry.PerformancePoetryActivity"
android:exported="true">
<intent-filter>
<action android:name="android.intent.action.MAIN" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package com.squareup.benchmarks.performance.complex.poetry

import com.squareup.benchmarks.performance.complex.poetry.views.LoaderSpinner
import com.squareup.benchmarks.performance.complex.poetry.views.MayBeLoadingScreen
import com.squareup.sample.container.overviewdetail.OverviewDetailScreen
import com.squareup.workflow1.Snapshot
import com.squareup.workflow1.StatefulWorkflow
import com.squareup.workflow1.Workflow
import com.squareup.workflow1.action
import com.squareup.workflow1.asWorker
import com.squareup.workflow1.runningWorker
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
import kotlinx.coroutines.flow.Flow

typealias IsLoading = Boolean

@OptIn(WorkflowUiExperimentalApi::class)
class MaybeLoadingGatekeeperWorkflow<T : Any>(
private val childWithLoading: Workflow<T, Any, OverviewDetailScreen>,
private val childProps: T,
private val isLoading: Flow<Boolean>
) : StatefulWorkflow<Unit, IsLoading, Unit, MayBeLoadingScreen>() {
override fun initialState(
props: Unit,
snapshot: Snapshot?
): IsLoading = false

override fun render(
renderProps: Unit,
renderState: IsLoading,
context: RenderContext
): MayBeLoadingScreen {
context.runningWorker(isLoading.asWorker()) {
action {
state = it
}
}
return MayBeLoadingScreen(
baseScreen = context.renderChild(childWithLoading, childProps) {
action { setOutput(Unit) }
},
loaders = if (renderState) listOf(LoaderSpinner) else emptyList()
)
}

override fun snapshotState(state: IsLoading): Snapshot? = null
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,241 @@
package com.squareup.benchmarks.performance.complex.poetry

import com.squareup.benchmarks.performance.complex.poetry.PerformancePoemWorkflow.Action.ClearSelection
import com.squareup.benchmarks.performance.complex.poetry.PerformancePoemWorkflow.Action.HandleStanzaListOutput
import com.squareup.benchmarks.performance.complex.poetry.PerformancePoemWorkflow.Action.SelectNext
import com.squareup.benchmarks.performance.complex.poetry.PerformancePoemWorkflow.Action.SelectPrevious
import com.squareup.benchmarks.performance.complex.poetry.PerformancePoemWorkflow.State
import com.squareup.benchmarks.performance.complex.poetry.PerformancePoemWorkflow.State.ComplexCall
import com.squareup.benchmarks.performance.complex.poetry.PerformancePoemWorkflow.State.Initializing
import com.squareup.benchmarks.performance.complex.poetry.PerformancePoemWorkflow.State.Selected
import com.squareup.benchmarks.performance.complex.poetry.views.BlankScreen
import com.squareup.sample.container.overviewdetail.OverviewDetailScreen
import com.squareup.sample.poetry.PoemWorkflow
import com.squareup.sample.poetry.PoemWorkflow.ClosePoem
import com.squareup.sample.poetry.StanzaListWorkflow
import com.squareup.sample.poetry.StanzaListWorkflow.NO_SELECTED_STANZA
import com.squareup.sample.poetry.StanzaScreen
import com.squareup.sample.poetry.StanzaWorkflow
import com.squareup.sample.poetry.StanzaWorkflow.Output.CloseStanzas
import com.squareup.sample.poetry.StanzaWorkflow.Output.ShowNextStanza
import com.squareup.sample.poetry.StanzaWorkflow.Output.ShowPreviousStanza
import com.squareup.sample.poetry.StanzaWorkflow.Props
import com.squareup.sample.poetry.model.Poem
import com.squareup.workflow1.Snapshot
import com.squareup.workflow1.StatefulWorkflow
import com.squareup.workflow1.Worker
import com.squareup.workflow1.WorkflowAction
import com.squareup.workflow1.WorkflowAction.Companion.noAction
import com.squareup.workflow1.action
import com.squareup.workflow1.runningWorker
import com.squareup.workflow1.ui.Screen
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
import com.squareup.workflow1.ui.container.BackStackScreen
import com.squareup.workflow1.ui.container.toBackStackScreen
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.MutableStateFlow

/**
* Version of [PoemWorkflow] that takes in a [SimulatedPerfConfig] to control the performance
* behavior of the Workflow.
*
* @param [simulatedPerfConfig] specifies whether to make the Workflow more 'complex' by
* introducing some asynchronous delays. See [SimulatedPerfConfig] for more details.
*
* @param [isLoading] will be set to true while this workflow is 'loading'. This is so that another
* component, such as a [MaybeLoadingGatekeeperWorkflow] can overlay the screen with a visual
* loading state. N.B. that whether or not this is loading could be included in the
* RenderingT if the interface [PoemWorkflow] had been left more flexible.
*
* ** Also note that raw mutable state sharing like this will almost always be a smell. It would
* be better to inject an interface of a 'Loading' service that could trigger this and likely
* break ties/conflicts with a token in the start/stop requests. We leave that complexity out
* here. **
*/
class PerformancePoemWorkflow(
private val simulatedPerfConfig: SimulatedPerfConfig = SimulatedPerfConfig.NO_SIMULATED_PERF,
private val isLoading: MutableStateFlow<Boolean>
) : PoemWorkflow, StatefulWorkflow<Poem, State, ClosePoem, OverviewDetailScreen>() {

sealed class 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 [Selected(NO_SELECTED_STANZA)] at the very least.
object Initializing : State()
data class ComplexCall(
val payload: Int
) : State()

data class Selected(val stanzaIndex: Int) : State()
}

override fun initialState(
props: Poem,
snapshot: Snapshot?
): State {
return if (simulatedPerfConfig.useInitializingState) Initializing else Selected(
NO_SELECTED_STANZA
)
}

@OptIn(WorkflowUiExperimentalApi::class)
override fun render(
renderProps: Poem,
renderState: State,
context: RenderContext
): OverviewDetailScreen {
return when (renderState) {
Initializing -> {
// 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!
// Don't try this at home.
context.runningWorker(Worker.from { Unit }, "initializing") {
isLoading.value = true
action {
isLoading.value = false
state = Selected(NO_SELECTED_STANZA)
}
}
OverviewDetailScreen(overviewRendering = BackStackScreen(BlankScreen))
}
else -> {
val (stanzaIndex, currentStateIsLoading) = when (renderState) {
is ComplexCall -> Pair(renderState.payload, true)
is Selected -> Pair(renderState.stanzaIndex, false)
Initializing -> throw IllegalStateException("No longer initializing.")
}

if (currentStateIsLoading) {
context.runningWorker(
Worker.from {
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 = Selected(currentState.payload)
}
}
}
}

val previousStanzas: List<StanzaScreen> =
if (stanzaIndex == NO_SELECTED_STANZA) emptyList()
else renderProps.stanzas.subList(0, stanzaIndex)
.mapIndexed { index, _ ->
context.renderChild(StanzaWorkflow, Props(renderProps, index), "$index") {
noAction()
}
}

val visibleStanza =
if (stanzaIndex == NO_SELECTED_STANZA) {
null
} else {
context.renderChild(
StanzaWorkflow, Props(renderProps, stanzaIndex), "$stanzaIndex"
) {
when (it) {
CloseStanzas -> ClearSelection(simulatedPerfConfig)
ShowPreviousStanza -> SelectPrevious(simulatedPerfConfig)
ShowNextStanza -> SelectNext(simulatedPerfConfig)
}
}
}

val stackedStanzas = visibleStanza?.let {
(previousStanzas + visibleStanza).toBackStackScreen<Screen>()
}

val stanzaListOverview =
context.renderChild(
StanzaListWorkflow,
renderProps
) { selected ->
HandleStanzaListOutput(simulatedPerfConfig, selected)
}
.copy(selection = stanzaIndex)

stackedStanzas
?.let {
OverviewDetailScreen(
overviewRendering = BackStackScreen(stanzaListOverview),
detailRendering = it
)
} ?: OverviewDetailScreen(
overviewRendering = BackStackScreen(stanzaListOverview),
selectDefault = {
context.actionSink.send(HandleStanzaListOutput(simulatedPerfConfig, 0))
}
)
}
}
}

override fun snapshotState(state: State): Snapshot? = null

internal sealed class Action : WorkflowAction<Poem, State, ClosePoem>() {
abstract val simulatedPerfConfig: SimulatedPerfConfig

class ClearSelection(override val simulatedPerfConfig: SimulatedPerfConfig) : Action()
class SelectPrevious(override val simulatedPerfConfig: SimulatedPerfConfig) : Action()
class SelectNext(override val simulatedPerfConfig: SimulatedPerfConfig) : Action()
class HandleStanzaListOutput(
override val simulatedPerfConfig: SimulatedPerfConfig,
val selection: Int
) : Action()

class ExitPoem(override val simulatedPerfConfig: SimulatedPerfConfig) : Action()

override fun Updater.apply() {
val currentIndex: Int = when (val solidState = state) {
is ComplexCall -> solidState.payload
Initializing -> NO_SELECTED_STANZA
is Selected -> solidState.stanzaIndex
}
when (this@Action) {
is ClearSelection ->
state =
if (simulatedPerfConfig.isComplex) {
ComplexCall(NO_SELECTED_STANZA)
} else {
Selected(
NO_SELECTED_STANZA
)
}
is SelectPrevious ->
state =
if (simulatedPerfConfig.isComplex) {
ComplexCall(currentIndex - 1)
} else {
Selected(
currentIndex - 1
)
}
is SelectNext ->
state =
if (simulatedPerfConfig.isComplex) {
ComplexCall(currentIndex + 1)
} else {
Selected(
currentIndex + 1
)
}
is HandleStanzaListOutput -> {
if (selection == NO_SELECTED_STANZA) setOutput(ClosePoem)
state = if (simulatedPerfConfig.isComplex) {
ComplexCall(selection)
} else {
Selected(selection)
}
}
is ExitPoem -> setOutput(ClosePoem)
}
}
}
}
Loading