Skip to content

Commit ee0c4ef

Browse files
steve-the-edwardssteve-the-edwards
committed
Re-enable render counting tests; More Deterministic
Closes #745
1 parent fd48bf4 commit ee0c4ef

File tree

8 files changed

+160
-67
lines changed

8 files changed

+160
-67
lines changed

‎benchmarks/performance-poetry/complex-poetry/src/androidTest/java/com/squareup/benchmarks/performance/complex/poetry/RenderPassTest.kt

Lines changed: 64 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import com.squareup.benchmarks.performance.complex.poetry.instrumentation.Render
1111
import com.squareup.benchmarks.performance.complex.poetry.instrumentation.SimulatedPerfConfig
1212
import com.squareup.benchmarks.performance.complex.poetry.robots.landscapeOrientation
1313
import com.squareup.benchmarks.performance.complex.poetry.robots.openRavenAndNavigate
14+
import com.squareup.benchmarks.performance.complex.poetry.robots.resetToRootPoetryList
1415
import com.squareup.benchmarks.performance.complex.poetry.robots.waitForPoetry
1516
import org.junit.Assert.fail
1617
import org.junit.Before
@@ -38,16 +39,21 @@ class RenderPassTest {
3839
@Before fun setup() {
3940
context = ApplicationProvider.getApplicationContext()
4041
PerformancePoetryActivity.installedInterceptor = renderPassCountingInterceptor
42+
43+
device.wakeUp()
44+
device.pressHome()
45+
device.waitForIdle()
46+
47+
// Do these in landscape so we have both the 'index' and the 'detail' showing.
48+
device.landscapeOrientation()
4149
}
4250

4351
@Test fun renderPassCounterComplexWithInitializingState() {
44-
// https://github.com/square/workflow-kotlin/issues/745
45-
// runRenderPassCounter(COMPLEX_INITIALIZING)
52+
runRenderPassCounter(COMPLEX_INITIALIZING)
4653
}
4754

4855
@Test fun renderPassCounterComplexNoInitializingState() {
49-
// https://github.com/square/workflow-kotlin/issues/745
50-
// runRenderPassCounter(COMPLEX_NO_INITIALIZING)
56+
runRenderPassCounter(COMPLEX_NO_INITIALIZING)
5157
}
5258

5359
private fun runRenderPassCounter(scenario: Scenario) {
@@ -56,16 +62,17 @@ class RenderPassTest {
5662
addFlags(Intent.FLAG_ACTIVITY_CLEAR_TASK)
5763
putExtra(PERF_CONFIG_EXTRA, scenario.config)
5864
}
59-
device.wakeUp()
60-
device.pressHome()
61-
// Do these in landscape so we have both the 'index' and the 'detail' showing.
62-
device.landscapeOrientation()
6365

6466
InstrumentationRegistry.getInstrumentation().context.startActivity(intent)
65-
6667
device.waitForPoetry()
6768
device.waitForIdle()
6869

70+
// Go back to root list so this is deterministic.
71+
device.resetToRootPoetryList()
72+
73+
// Now reset for the actual counting.
74+
renderPassCountingInterceptor.reset()
75+
6976
device.openRavenAndNavigate()
7077

7178
val totalRenderPasses = renderPassCountingInterceptor.renderEfficiencyTracking.totalRenderPasses
@@ -78,6 +85,7 @@ class RenderPassTest {
7885
subject = renderPassSubject,
7986
by = by,
8087
value = "$totalRenderPasses",
88+
oldValue = "${scenario.expectedPasses}",
8189
scenario = scenario.title
8290
)
8391
} else if (totalRenderPasses < scenario.expectedPasses) {
@@ -88,6 +96,7 @@ class RenderPassTest {
8896
subject = renderPassSubject,
8997
by = by,
9098
value = "$totalRenderPasses",
99+
oldValue = "${scenario.expectedPasses}",
91100
scenario = scenario.title
92101
)
93102
}
@@ -103,8 +112,12 @@ class RenderPassTest {
103112

104113
val ratioSubject = "the fresh rendering ratio (higher better)"
105114
val ratioString = "%.3f".format(freshRatio)
115+
val oldRatioString = "%.3f".format(expectedRatio)
106116
val valueString = "(ratio: $ratioString; fresh renderings: $freshRenderings;" +
107117
" stale renderings: $staleRenderings)"
118+
val oldValueString =
119+
"(ratio: $oldRatioString; fresh renderings: ${scenario.expectedFreshRenderings};" +
120+
" stale renderings: ${scenario.expectedStaleRenderings})"
108121

109122
if (freshRatio > expectedRatio) {
110123
// Something has 'improved' - let's see if we reduced stale nodes.
@@ -114,9 +127,20 @@ class RenderPassTest {
114127
"%.2f".format(100.0 * (diff.toDouble() / scenario.expectedStaleRenderings.toDouble()))
115128
val by = "rendering $diff fewer stale nodes" +
116129
" (reducing by $percentage% to $staleRenderings)"
117-
congrats(subject = ratioSubject, by = by, value = valueString, scenario = scenario.title)
130+
congrats(
131+
subject = ratioSubject,
132+
by = by,
133+
value = valueString,
134+
oldValue = oldValueString,
135+
scenario = scenario.title
136+
)
118137
} else {
119-
meh(ratioSubject, valueString, scenario.title)
138+
meh(
139+
subject = ratioSubject,
140+
value = valueString,
141+
oldValue = oldValueString,
142+
scenario = scenario.title
143+
)
120144
}
121145
} else if (freshRatio < expectedRatio) {
122146
// Something has 'worsened' - let's see if we increased stale nodes.
@@ -126,9 +150,20 @@ class RenderPassTest {
126150
"%.2f".format(100.0 * (diff.toDouble() / scenario.expectedStaleRenderings.toDouble()))
127151
val by = "rendering $diff more stale nodes" +
128152
" (increasing by $percentage% to $staleRenderings)"
129-
uhOh(subject = ratioSubject, by = by, value = valueString, scenario = scenario.title)
153+
uhOh(
154+
subject = ratioSubject,
155+
by = by,
156+
value = valueString,
157+
oldValue = oldValueString,
158+
scenario = scenario.title
159+
)
130160
} else {
131-
meh(ratioSubject, valueString, scenario.title)
161+
meh(
162+
subject = ratioSubject,
163+
value = valueString,
164+
oldValue = oldValueString,
165+
scenario = scenario.title
166+
)
132167
}
133168
}
134169
}
@@ -141,9 +176,9 @@ class RenderPassTest {
141176
useInitializingState = true,
142177
complexityDelay = 100L
143178
),
144-
expectedPasses = 57,
145-
expectedFreshRenderings = 83,
146-
expectedStaleRenderings = 615
179+
expectedPasses = 58,
180+
expectedFreshRenderings = 85,
181+
expectedStaleRenderings = 617
147182
)
148183

149184
val COMPLEX_NO_INITIALIZING = Scenario(
@@ -153,41 +188,45 @@ class RenderPassTest {
153188
useInitializingState = false,
154189
complexityDelay = 100L
155190
),
156-
expectedPasses = 58,
157-
expectedFreshRenderings = 84,
158-
expectedStaleRenderings = 636
191+
expectedPasses = 56,
192+
expectedFreshRenderings = 83,
193+
expectedStaleRenderings = 605
159194
)
160195

161196
fun congrats(
162197
subject: String,
163198
value: String,
199+
oldValue: String,
164200
scenario: String,
165201
by: String? = null
166202
) = fail(
167-
"Congrats! You have improved the $subject ${by?.let { "by $by " }}in $scenario!" +
168-
" Please update the expected value for your config. The value is now $value."
203+
"Congrats! You have improved the $subject ${by?.let { "by $by " } ?: ""}in $scenario!" +
204+
" Please update the expected value for your config. The value is now $value" +
205+
" (was $oldValue)."
169206
)
170207

171208
fun uhOh(
172209
subject: String,
173210
value: String,
211+
oldValue: String,
174212
scenario: String,
175213
by: String? = null
176214
) = fail(
177-
"Uh Oh! You have worsened the $subject ${by?.let { "by $by " }}in $scenario!" +
178-
" The value is now $value. This likely results in worse performance." +
215+
"Uh Oh! You have worsened the $subject ${by?.let { "by $by " } ?: ""}in $scenario!" +
216+
" The value is now $value (was $oldValue). This likely results in worse performance." +
179217
" You can check with the timing benchmarks if you disagree."
180218
)
181219

182220
fun meh(
183221
subject: String,
184222
value: String,
223+
oldValue: String,
185224
scenario: String,
186225
by: String? = null
187226
) = fail(
188-
"Hmmm. The $subject has improved ${by?.let { "by $by " }}in $scenario," +
227+
"Hmmm. The $subject has improved ${by?.let { "by $by " } ?: ""}in $scenario," +
189228
" but only because the scenario has changed, impacting expectation" +
190-
" but not for the better. The value is now $value. Please update the test."
229+
" but not for the better. The value is now $value (was $oldValue). Please update the test."
191230
)
192231
}
193232
}

‎benchmarks/performance-poetry/complex-poetry/src/main/java/com/squareup/benchmarks/performance/complex/poetry/instrumentation/PerformanceTracking.kt

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@ data class RenderStats(
1111
nodesRenderedFresh += renderStats.nodesRenderedFresh
1212
nodesRenderedStale += renderStats.nodesRenderedStale
1313
}
14+
15+
fun reset() {
16+
nodesRenderedFresh = 0
17+
nodesRenderedStale = 0
18+
}
1419
}
1520

1621
/**
@@ -23,4 +28,9 @@ data class RenderEfficiency(
2328
val freshRenderingRatio: Double
2429
get() = totalNodeStats.nodesRenderedFresh.toDouble() /
2530
(totalNodeStats.nodesRenderedStale + totalNodeStats.nodesRenderedFresh).toDouble()
31+
32+
fun reset() {
33+
totalRenderPasses = 0
34+
totalNodeStats.reset()
35+
}
2636
}

‎benchmarks/performance-poetry/complex-poetry/src/main/java/com/squareup/benchmarks/performance/complex/poetry/instrumentation/RenderPassCountingInterceptor.kt

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,4 +54,12 @@ class RenderPassCountingInterceptor : WorkflowInterceptor {
5454
}
5555
}
5656
}
57+
58+
/**
59+
* Reset all the counters.
60+
*/
61+
fun reset() {
62+
renderEfficiencyTracking.reset()
63+
nodeStates.clear()
64+
}
5765
}

‎benchmarks/performance-poetry/complex-poetry/src/main/java/com/squareup/benchmarks/performance/complex/poetry/robots/DeviceRobots.kt renamed to ‎benchmarks/performance-poetry/complex-poetry/src/main/java/com/squareup/benchmarks/performance/complex/poetry/robots/DeviceCyborgs.kt

File renamed without changes.

‎benchmarks/performance-poetry/complex-poetry/src/main/java/com/squareup/benchmarks/performance/complex/poetry/robots/PoetryCyborgs.kt

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
package com.squareup.benchmarks.performance.complex.poetry.robots
2+
3+
import android.widget.ImageButton
4+
import android.widget.ProgressBar
5+
import androidx.test.uiautomator.By
6+
import androidx.test.uiautomator.BySelector
7+
import androidx.test.uiautomator.UiDevice
8+
import androidx.test.uiautomator.Until
9+
10+
const val POETRY_PACKAGE = "com.squareup.benchmarks.performance.complex.poetry"
11+
val PoetryPackageSelector: BySelector =
12+
By.pkg(POETRY_PACKAGE).depth(0)
13+
val LoadingDialogSelector: BySelector =
14+
By.clazz(ProgressBar::class.java).res(POETRY_PACKAGE, "loading_progress_bar")
15+
val RavenPoemSelector: BySelector = By.text("The Raven").clickable(true).focusable(true)
16+
val NextSelector: BySelector = By.textStartsWith("next")
17+
val PreviousSelector: BySelector = By.textEndsWith("previous")
18+
val UpButtonSelector: BySelector =
19+
By.clazz(ImageButton::class.java).descContains("Up").clickable(true)
20+
21+
fun UiDevice.waitForLoadingInterstitial(timeout: Long = DEFAULT_UI_AUTOMATOR_TIMEOUT) {
22+
wait(Until.hasObject(LoadingDialogSelector), timeout)
23+
}
24+
25+
fun UiDevice.waitForPoetry(timeout: Long = DEFAULT_UI_AUTOMATOR_TIMEOUT) {
26+
wait(Until.hasObject(PoetryPackageSelector), timeout)
27+
}
28+
29+
fun UiDevice.next(timeout: Long = DEFAULT_UI_AUTOMATOR_TIMEOUT) {
30+
waitForAndClick(NextSelector, timeout)
31+
}
32+
33+
fun UiDevice.previous(timeout: Long = DEFAULT_UI_AUTOMATOR_TIMEOUT) {
34+
waitForAndClick(PreviousSelector, timeout)
35+
}
36+
37+
fun UiDevice.resetToRootPoetryList() {
38+
var uiObject = wait(Until.findObject(UpButtonSelector), DEFAULT_UI_AUTOMATOR_TIMEOUT)
39+
while (uiObject != null) {
40+
uiObject.click()
41+
waitForLoadingInterstitial()
42+
uiObject = wait(Until.findObject(UpButtonSelector), DEFAULT_UI_AUTOMATOR_TIMEOUT)
43+
}
44+
}
45+
46+
/**
47+
* Note this only works in landscape mode.
48+
*/
49+
fun UiDevice.openRavenAndNavigate() {
50+
waitForIdle()
51+
waitForAndClick(RavenPoemSelector)
52+
waitForLoadingInterstitial()
53+
waitForAndClick(By.textStartsWith("Deep into that darkness peering"))
54+
waitForLoadingInterstitial()
55+
56+
repeat(5) {
57+
next()
58+
waitForLoadingInterstitial()
59+
}
60+
61+
repeat(5) {
62+
previous()
63+
waitForLoadingInterstitial()
64+
}
65+
66+
waitForAndClick(UpButtonSelector)
67+
waitForLoadingInterstitial()
68+
69+
// Back to the start.
70+
waitFor(RavenPoemSelector)
71+
}

‎benchmarks/performance-poetry/complex-poetry/src/main/java/com/squareup/benchmarks/performance/complex/poetry/robots/PoetryRobots.kt

Lines changed: 0 additions & 42 deletions
This file was deleted.

‎benchmarks/performance-poetry/complex-poetry/src/main/java/com/squareup/benchmarks/performance/complex/poetry/views/LoaderSpinner.kt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import android.view.ViewGroup
55
import android.view.ViewGroup.LayoutParams.WRAP_CONTENT
66
import android.widget.FrameLayout
77
import android.widget.ProgressBar
8+
import com.squareup.benchmarks.performance.complex.poetry.R
89
import com.squareup.workflow1.ui.AndroidScreen
910
import com.squareup.workflow1.ui.ScreenViewFactory
1011
import com.squareup.workflow1.ui.ScreenViewHolder
@@ -15,6 +16,7 @@ object LoaderSpinner : AndroidScreen<LoaderSpinner> {
1516
override val viewFactory =
1617
ScreenViewFactory.fromCode<LoaderSpinner> { _, initialEnvironment, context, _ ->
1718
val progressBar = ProgressBar(context).apply {
19+
id = R.id.loading_progress_bar
1820
layoutParams = FrameLayout.LayoutParams(
1921
ViewGroup.LayoutParams(WRAP_CONTENT, WRAP_CONTENT)
2022
).apply {

‎benchmarks/performance-poetry/complex-poetry/src/main/res/values/ids.xml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<resources>
3+
<!-- For the loading bar in between poetry screens -->
4+
<item name="loading_progress_bar" type="id"/>
5+
</resources>

0 commit comments

Comments
 (0)