Skip to content

Commit bacaba0

Browse files
mlykotommlykotom
authored
Improve RealImageLoader.execute to properly call async request (#2205)
* Improve RealImageLoader.execute to properly call async request * Add experiments to get information about performance * Revert fix * Add comment what's blocking * Fix spotless * Tweak benchmarks + add traces Update benchmarks Experiment removing mutableState Before: AsyncImagePainter.onRememberedAverageMs min 0.1, median 0.1, max 0.1 AsyncImagePainter.onRememberedCount min 130.0, median 139.0, max 146.0 AsyncImagePainter.onRememberedMs min 9.3, median 12.5, max 16.3 frameDurationCpuMs P50 4.1, P90 9.1, P95 20.0, P99 99.3 After: AsyncImagePainter.onRememberedAverageMs min 0.1, median 0.1, max 0.1 AsyncImagePainter.onRememberedCount min 130.0, median 141.0, max 148.0 AsyncImagePainter.onRememberedMs min 8.3, median 10.5, max 15.8 frameDurationCpuMs P50 4.4, P90 8.6, P95 12.0, P99 95.1 Don't launch job for compose before AsyncImagePainter.onRememberedAverage_µs min 77.3, median 101.5, max 116.7 AsyncImagePainter.onRememberedCount min 130.0, median 130.0, max 146.0 AsyncImagePainter.onRemembered_µs min 10,048.4, median 13,880.2, max 16,900.0 timeToInitialDisplayMs min 256.8, median 281.7, max 358.9 frameDurationCpuMs P50 5.1, P90 9.2, P95 13.3, P99 117.0 after AsyncImagePainter.onRememberedAverage_µs min 68.9, median 78.7, max 91.1 AsyncImagePainter.onRememberedCount min 128.0, median 141.0, max 146.0 AsyncImagePainter.onRemembered_µs min 8,956.1, median 11,163.1, max 13,111.9 timeToInitialDisplayMs min 234.1, median 276.5, max 325.7 frameDurationCpuMs P50 4.5, P90 8.9, P95 15.5, P99 120.3 * Experiment removing Compose State Before: AsyncImagePainter.onRememberedAverageMs min 0.1, median 0.1, max 0.1 AsyncImagePainter.onRememberedCount min 130.0, median 139.0, max 146.0 AsyncImagePainter.onRememberedMs min 9.3, median 12.5, max 16.3 frameDurationCpuMs P50 4.1, P90 9.1, P95 20.0, P99 99.3 After: AsyncImagePainter.onRememberedAverageMs min 0.1, median 0.1, max 0.1 AsyncImagePainter.onRememberedCount min 130.0, median 141.0, max 148.0 AsyncImagePainter.onRememberedMs min 8.3, median 10.5, max 15.8 frameDurationCpuMs P50 4.4, P90 8.6, P95 12.0, P99 95.1 * Don't launch job for compose before AsyncImagePainter.onRememberedAverage_µs min 77.3, median 101.5, max 116.7 AsyncImagePainter.onRememberedCount min 130.0, median 130.0, max 146.0 AsyncImagePainter.onRemembered_µs min 10,048.4, median 13,880.2, max 16,900.0 timeToInitialDisplayMs min 256.8, median 281.7, max 358.9 frameDurationCpuMs P50 5.1, P90 9.2, P95 13.3, P99 117.0 after AsyncImagePainter.onRememberedAverage_µs min 68.9, median 78.7, max 91.1 AsyncImagePainter.onRememberedCount min 128.0, median 141.0, max 146.0 AsyncImagePainter.onRemembered_µs min 8,956.1, median 11,163.1, max 13,111.9 timeToInitialDisplayMs min 234.1, median 276.5, max 325.7 frameDurationCpuMs P50 4.5, P90 8.9, P95 15.5, P99 120.3 * Cleanup * Fix dependency with version catalog * Cleanup * Revert API changes
1 parent 95edaae commit bacaba0

7 files changed

Lines changed: 162 additions & 16 deletions

File tree

‎coil-base/src/main/java/coil/RealImageLoader.kt‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -123,18 +123,25 @@ internal class RealImageLoader(
123123
}
124124
}
125125

126-
override suspend fun execute(request: ImageRequest) = coroutineScope {
127-
// Start executing the request on the main thread.
128-
val job = async(Dispatchers.Main.immediate) {
129-
executeMain(request, REQUEST_TYPE_EXECUTE)
130-
}
131-
132-
// Update the current request attached to the view and await the result.
126+
override suspend fun execute(request: ImageRequest) =
133127
if (request.target is ViewTarget<*>) {
134-
request.target.view.requestManager.getDisposable(job)
128+
// We don't use the async call that returns the job for Compose to micro-optimize the performance.
129+
// The job is only needed in case of the Views implementation.
130+
coroutineScope { // Start executing the request on the main thread.
131+
val job = async(Dispatchers.Main.immediate) {
132+
executeMain(request, REQUEST_TYPE_EXECUTE)
133+
}
134+
// Update the current request attached to the view and await the result.
135+
request.target.view.requestManager.getDisposable(job)
136+
137+
job.await()
138+
}
139+
} else {
140+
// Start executing the request on the main thread.
141+
withContext(Dispatchers.Main.immediate) {
142+
executeMain(request, REQUEST_TYPE_EXECUTE)
143+
}
135144
}
136-
return@coroutineScope job.await()
137-
}
138145

139146
@MainThread
140147
private suspend fun executeMain(initialRequest: ImageRequest, type: Int): ImageResult {

‎coil-benchmark/build.gradle.kts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,12 @@ setupTestModule(name = "coil.benchmark", config = true) {
1111
defaultConfig {
1212
minSdk = 23
1313
buildConfigField("String", "PROJECT", "\"$targetProject\"")
14+
15+
// Enables Composition Tracing for benchmarks
16+
// testInstrumentationRunnerArguments["androidx.benchmark.fullTracing.enable"] = "true"
17+
// Enables Method tracing for benchmarks. Be aware this skews the performance results,
18+
// so don't use it for measuring exact timinig
19+
// testInstrumentationRunnerArguments["androidx.benchmark.profiling.mode"] = "MethodTracing"
1420
}
1521
buildTypes {
1622
create("benchmark") {
@@ -39,6 +45,8 @@ dependencies {
3945
implementation(libs.androidx.test.espresso)
4046
implementation(libs.androidx.test.junit)
4147
implementation(libs.androidx.test.uiautomator)
48+
implementation(libs.androidx.tracing.perfetto)
49+
implementation(libs.androidx.tracing.perfetto.binary)
4250
}
4351

4452
androidComponents {

‎coil-benchmark/src/main/java/coil/benchmark/MicrosTraceSectionMetric.kt‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
package coil.benchmark
2+
3+
import androidx.benchmark.macro.ExperimentalMetricApi
4+
import androidx.benchmark.macro.TraceMetric
5+
import androidx.benchmark.perfetto.ExperimentalPerfettoTraceProcessorApi
6+
import androidx.benchmark.perfetto.PerfettoTraceProcessor
7+
8+
/**
9+
* TraceSectionMetric to give average/sum in microseconds measurements.
10+
*/
11+
@OptIn(ExperimentalMetricApi::class)
12+
class MicrosTraceSectionMetric(
13+
private val sectionName: String,
14+
private vararg val mode: Mode,
15+
private val label: String = sectionName,
16+
private val targetPackageOnly: Boolean = true,
17+
) : TraceMetric() {
18+
19+
enum class Mode {
20+
Sum,
21+
Average
22+
}
23+
24+
@ExperimentalPerfettoTraceProcessorApi
25+
@Suppress("RestrictedApi")
26+
override fun getResult(
27+
captureInfo: CaptureInfo,
28+
traceSession: PerfettoTraceProcessor.Session,
29+
): List<Measurement> {
30+
val slices = traceSession.querySlices(
31+
sectionName,
32+
packageName = if (targetPackageOnly) captureInfo.targetPackageName else null,
33+
)
34+
35+
return mode.flatMap { m ->
36+
when (m) {
37+
Mode.Sum -> listOf(
38+
Measurement(
39+
name = sectionName + "_µs",
40+
// note, this duration assumes non-reentrant slices
41+
data = slices.sumOf { it.dur } / 1_000.0,
42+
),
43+
Measurement(
44+
name = sectionName + "Count",
45+
data = slices.size.toDouble(),
46+
),
47+
)
48+
49+
Mode.Average -> listOf(
50+
Measurement(
51+
name = label + "Average_µs",
52+
data = slices.sumOf { it.dur } / 1_000.0 / slices.size,
53+
),
54+
)
55+
}
56+
}
57+
}
58+
}

‎coil-benchmark/src/main/java/coil/benchmark/ScrollBenchmark.kt‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
package coil.benchmark
2+
3+
import android.graphics.Point
4+
import android.os.Build
5+
import androidx.annotation.RequiresApi
6+
import androidx.benchmark.macro.BaselineProfileMode
7+
import androidx.benchmark.macro.CompilationMode
8+
import androidx.benchmark.macro.FrameTimingMetric
9+
import androidx.benchmark.macro.StartupMode
10+
import androidx.benchmark.macro.StartupTimingMetric
11+
import androidx.benchmark.macro.junit4.MacrobenchmarkRule
12+
import androidx.test.ext.junit.runners.AndroidJUnit4
13+
import androidx.test.uiautomator.By
14+
import coil.benchmark.BuildConfig.PROJECT
15+
import coil.benchmark.MicrosTraceSectionMetric.Mode.Average
16+
import coil.benchmark.MicrosTraceSectionMetric.Mode.Sum
17+
import org.junit.Rule
18+
import org.junit.Test
19+
import org.junit.runner.RunWith
20+
21+
@RunWith(AndroidJUnit4::class)
22+
class ScrollBenchmark {
23+
24+
@get:Rule
25+
val benchmarkRule = MacrobenchmarkRule()
26+
27+
@RequiresApi(Build.VERSION_CODES.N)
28+
@Test
29+
fun baselineProfile() {
30+
benchmark(CompilationMode.Partial(BaselineProfileMode.Require))
31+
}
32+
33+
@Test
34+
fun fullCompilation() {
35+
benchmark(CompilationMode.Full())
36+
}
37+
38+
private fun benchmark(compilationMode: CompilationMode) {
39+
benchmarkRule.measureRepeated(
40+
packageName = "sample.$PROJECT",
41+
metrics = listOf(
42+
FrameTimingMetric(),
43+
StartupTimingMetric(),
44+
MicrosTraceSectionMetric(
45+
"rememberAsyncImagePainter",
46+
Sum, Average,
47+
),
48+
MicrosTraceSectionMetric(
49+
"AsyncImagePainter.onRemembered",
50+
Sum, Average,
51+
),
52+
),
53+
iterations = 20,
54+
startupMode = StartupMode.COLD,
55+
compilationMode = compilationMode,
56+
measureBlock = {
57+
startActivityAndWait()
58+
Thread.sleep(3_000)
59+
val list = device.findObject(By.res("list"))
60+
list.setGestureMargin(device.displayWidth / 5)
61+
list.drag(Point(list.visibleBounds.centerX(), list.visibleBounds.top))
62+
Thread.sleep(300)
63+
},
64+
)
65+
}
66+
}

‎coil-compose-base/src/main/java/coil/compose/AsyncImagePainter.kt‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import androidx.compose.ui.graphics.vector.ImageVector
3030
import androidx.compose.ui.layout.ContentScale
3131
import androidx.compose.ui.platform.LocalInspectionMode
3232
import androidx.compose.ui.unit.Constraints
33+
import androidx.compose.ui.util.trace
3334
import coil.ImageLoader
3435
import coil.compose.AsyncImagePainter.Companion.DefaultTransform
3536
import coil.compose.AsyncImagePainter.State
@@ -197,7 +198,7 @@ private fun rememberAsyncImagePainter(
197198
onState: ((State) -> Unit)?,
198199
contentScale: ContentScale,
199200
filterQuality: FilterQuality,
200-
): AsyncImagePainter {
201+
): AsyncImagePainter = trace("rememberAsyncImagePainter") {
201202
val request = requestOf(state.model)
202203
validateRequest(request)
203204

@@ -253,7 +254,7 @@ class AsyncImagePainter internal constructor(
253254
private set
254255

255256
/** The current [ImageRequest]. */
256-
var request: ImageRequest by mutableStateOf(request)
257+
var request by mutableStateOf(request)
257258
internal set
258259

259260
/** The current [ImageLoader]. */
@@ -282,9 +283,9 @@ class AsyncImagePainter internal constructor(
282283
}
283284

284285
@OptIn(ExperimentalCoroutinesApi::class)
285-
override fun onRemembered() {
286+
override fun onRemembered() = trace("AsyncImagePainter.onRemembered") {
286287
// Short circuit if we're already remembered.
287-
if (rememberScope != null) return
288+
if (rememberScope != null) return@trace
288289

289290
// Create a new scope to observe state and execute requests while we're remembered.
290291
val scope = CoroutineScope(SupervisorJob() + Dispatchers.Main.immediate)
@@ -297,7 +298,7 @@ class AsyncImagePainter internal constructor(
297298
if (isPreview) {
298299
val request = request.newBuilder().defaults(imageLoader.defaults).build()
299300
updateState(State.Loading(request.placeholder?.toPainter()))
300-
return
301+
return@trace
301302
}
302303

303304
// Observe the current request and execute any emissions.

‎coil-sample-compose/build.gradle.kts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,4 +42,5 @@ dependencies {
4242
implementation(libs.androidx.activity.compose)
4343
implementation(libs.androidx.lifecycle.viewmodel.compose)
4444
implementation(libs.compose.material)
45+
implementation(libs.androidx.compose.runtime.tracing)
4546
}

‎gradle/libs.versions.toml‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ ktlint = "1.0.1"
88
okhttp = "4.12.0"
99
okio = "3.8.0"
1010
roborazzi = "1.7.0"
11+
perfetto = "1.0.0"
12+
runtimeTracing = "1.0.0-beta01"
1113

1214
[plugins]
1315
binaryCompatibility = "org.jetbrains.kotlinx.binary-compatibility-validator:0.14.0"
@@ -25,9 +27,10 @@ accompanist-drawablepainter = "com.google.accompanist:accompanist-drawablepainte
2527

2628
androidx-activity = { module = "androidx.activity:activity-ktx", version.ref = "androidx-activity" }
2729
androidx-activity-compose = { module = "androidx.activity:activity-compose", version.ref = "androidx-activity" }
30+
androidx-compose-runtime-tracing = { module = "androidx.compose.runtime:runtime-tracing", version.ref = "runtimeTracing" }
2831
androidx-appcompat-resources = "androidx.appcompat:appcompat-resources:1.6.1"
2932
androidx-annotation = "androidx.annotation:annotation:1.7.1"
30-
androidx-benchmark-macro = "androidx.benchmark:benchmark-macro-junit4:1.2.3"
33+
androidx-benchmark-macro = "androidx.benchmark:benchmark-macro-junit4:1.2.4"
3134
androidx-collection = "androidx.collection:collection:1.4.0"
3235
androidx-constraintlayout = "androidx.constraintlayout:constraintlayout:2.1.4"
3336
androidx-core = "androidx.core:core-ktx:1.12.0"
@@ -43,6 +46,8 @@ androidx-test-junit = "androidx.test.ext:junit-ktx:1.1.5"
4346
androidx-test-rules = "androidx.test:rules:1.5.0"
4447
androidx-test-runner = "androidx.test:runner:1.5.2"
4548
androidx-test-uiautomator = "androidx.test.uiautomator:uiautomator:2.2.0"
49+
androidx-tracing-perfetto = { module = "androidx.tracing:tracing-perfetto", version.ref = "perfetto" }
50+
androidx-tracing-perfetto-binary = { module = "androidx.tracing:tracing-perfetto-binary", version.ref = "perfetto" }
4651
androidx-vectordrawable-animated = "androidx.vectordrawable:vectordrawable-animated:1.1.0"
4752

4853
compose-foundation = { module = "androidx.compose.foundation:foundation", version.ref = "compose" }

0 commit comments

Comments
 (0)