From bacaba0628579cd02908fca5eff851ed42749e04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Mlynari=C4=8D?= Date: Fri, 10 May 2024 07:40:46 +0200 Subject: [PATCH] Improve RealImageLoader.execute to properly call async request (#2205) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 --- .../src/main/java/coil/RealImageLoader.kt | 27 +++++--- coil-benchmark/build.gradle.kts | 8 +++ .../benchmark/MicrosTraceSectionMetric.kt | 58 ++++++++++++++++ .../java/coil/benchmark/ScrollBenchmark.kt | 66 +++++++++++++++++++ .../java/coil/compose/AsyncImagePainter.kt | 11 ++-- coil-sample-compose/build.gradle.kts | 1 + gradle/libs.versions.toml | 7 +- 7 files changed, 162 insertions(+), 16 deletions(-) create mode 100644 coil-benchmark/src/main/java/coil/benchmark/MicrosTraceSectionMetric.kt create mode 100644 coil-benchmark/src/main/java/coil/benchmark/ScrollBenchmark.kt diff --git a/coil-base/src/main/java/coil/RealImageLoader.kt b/coil-base/src/main/java/coil/RealImageLoader.kt index 42cda5bb33..e7d05b4edd 100644 --- a/coil-base/src/main/java/coil/RealImageLoader.kt +++ b/coil-base/src/main/java/coil/RealImageLoader.kt @@ -123,18 +123,25 @@ internal class RealImageLoader( } } - override suspend fun execute(request: ImageRequest) = coroutineScope { - // Start executing the request on the main thread. - val job = async(Dispatchers.Main.immediate) { - executeMain(request, REQUEST_TYPE_EXECUTE) - } - - // Update the current request attached to the view and await the result. + override suspend fun execute(request: ImageRequest) = if (request.target is ViewTarget<*>) { - request.target.view.requestManager.getDisposable(job) + // We don't use the async call that returns the job for Compose to micro-optimize the performance. + // The job is only needed in case of the Views implementation. + coroutineScope { // Start executing the request on the main thread. + val job = async(Dispatchers.Main.immediate) { + executeMain(request, REQUEST_TYPE_EXECUTE) + } + // Update the current request attached to the view and await the result. + request.target.view.requestManager.getDisposable(job) + + job.await() + } + } else { + // Start executing the request on the main thread. + withContext(Dispatchers.Main.immediate) { + executeMain(request, REQUEST_TYPE_EXECUTE) + } } - return@coroutineScope job.await() - } @MainThread private suspend fun executeMain(initialRequest: ImageRequest, type: Int): ImageResult { diff --git a/coil-benchmark/build.gradle.kts b/coil-benchmark/build.gradle.kts index 136f850c76..d4ca588313 100644 --- a/coil-benchmark/build.gradle.kts +++ b/coil-benchmark/build.gradle.kts @@ -11,6 +11,12 @@ setupTestModule(name = "coil.benchmark", config = true) { defaultConfig { minSdk = 23 buildConfigField("String", "PROJECT", "\"$targetProject\"") + + // Enables Composition Tracing for benchmarks + // testInstrumentationRunnerArguments["androidx.benchmark.fullTracing.enable"] = "true" + // Enables Method tracing for benchmarks. Be aware this skews the performance results, + // so don't use it for measuring exact timinig + // testInstrumentationRunnerArguments["androidx.benchmark.profiling.mode"] = "MethodTracing" } buildTypes { create("benchmark") { @@ -39,6 +45,8 @@ dependencies { implementation(libs.androidx.test.espresso) implementation(libs.androidx.test.junit) implementation(libs.androidx.test.uiautomator) + implementation(libs.androidx.tracing.perfetto) + implementation(libs.androidx.tracing.perfetto.binary) } androidComponents { diff --git a/coil-benchmark/src/main/java/coil/benchmark/MicrosTraceSectionMetric.kt b/coil-benchmark/src/main/java/coil/benchmark/MicrosTraceSectionMetric.kt new file mode 100644 index 0000000000..001954f0e4 --- /dev/null +++ b/coil-benchmark/src/main/java/coil/benchmark/MicrosTraceSectionMetric.kt @@ -0,0 +1,58 @@ +package coil.benchmark + +import androidx.benchmark.macro.ExperimentalMetricApi +import androidx.benchmark.macro.TraceMetric +import androidx.benchmark.perfetto.ExperimentalPerfettoTraceProcessorApi +import androidx.benchmark.perfetto.PerfettoTraceProcessor + +/** + * TraceSectionMetric to give average/sum in microseconds measurements. + */ +@OptIn(ExperimentalMetricApi::class) +class MicrosTraceSectionMetric( + private val sectionName: String, + private vararg val mode: Mode, + private val label: String = sectionName, + private val targetPackageOnly: Boolean = true, +) : TraceMetric() { + + enum class Mode { + Sum, + Average + } + + @ExperimentalPerfettoTraceProcessorApi + @Suppress("RestrictedApi") + override fun getResult( + captureInfo: CaptureInfo, + traceSession: PerfettoTraceProcessor.Session, + ): List { + val slices = traceSession.querySlices( + sectionName, + packageName = if (targetPackageOnly) captureInfo.targetPackageName else null, + ) + + return mode.flatMap { m -> + when (m) { + Mode.Sum -> listOf( + Measurement( + name = sectionName + "_µs", + // note, this duration assumes non-reentrant slices + data = slices.sumOf { it.dur } / 1_000.0, + ), + Measurement( + name = sectionName + "Count", + data = slices.size.toDouble(), + ), + ) + + Mode.Average -> listOf( + Measurement( + name = label + "Average_µs", + data = slices.sumOf { it.dur } / 1_000.0 / slices.size, + ), + ) + } + } + } +} diff --git a/coil-benchmark/src/main/java/coil/benchmark/ScrollBenchmark.kt b/coil-benchmark/src/main/java/coil/benchmark/ScrollBenchmark.kt new file mode 100644 index 0000000000..14f9a8c83b --- /dev/null +++ b/coil-benchmark/src/main/java/coil/benchmark/ScrollBenchmark.kt @@ -0,0 +1,66 @@ +package coil.benchmark + +import android.graphics.Point +import android.os.Build +import androidx.annotation.RequiresApi +import androidx.benchmark.macro.BaselineProfileMode +import androidx.benchmark.macro.CompilationMode +import androidx.benchmark.macro.FrameTimingMetric +import androidx.benchmark.macro.StartupMode +import androidx.benchmark.macro.StartupTimingMetric +import androidx.benchmark.macro.junit4.MacrobenchmarkRule +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.uiautomator.By +import coil.benchmark.BuildConfig.PROJECT +import coil.benchmark.MicrosTraceSectionMetric.Mode.Average +import coil.benchmark.MicrosTraceSectionMetric.Mode.Sum +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith + +@RunWith(AndroidJUnit4::class) +class ScrollBenchmark { + + @get:Rule + val benchmarkRule = MacrobenchmarkRule() + + @RequiresApi(Build.VERSION_CODES.N) + @Test + fun baselineProfile() { + benchmark(CompilationMode.Partial(BaselineProfileMode.Require)) + } + + @Test + fun fullCompilation() { + benchmark(CompilationMode.Full()) + } + + private fun benchmark(compilationMode: CompilationMode) { + benchmarkRule.measureRepeated( + packageName = "sample.$PROJECT", + metrics = listOf( + FrameTimingMetric(), + StartupTimingMetric(), + MicrosTraceSectionMetric( + "rememberAsyncImagePainter", + Sum, Average, + ), + MicrosTraceSectionMetric( + "AsyncImagePainter.onRemembered", + Sum, Average, + ), + ), + iterations = 20, + startupMode = StartupMode.COLD, + compilationMode = compilationMode, + measureBlock = { + startActivityAndWait() + Thread.sleep(3_000) + val list = device.findObject(By.res("list")) + list.setGestureMargin(device.displayWidth / 5) + list.drag(Point(list.visibleBounds.centerX(), list.visibleBounds.top)) + Thread.sleep(300) + }, + ) + } +} diff --git a/coil-compose-base/src/main/java/coil/compose/AsyncImagePainter.kt b/coil-compose-base/src/main/java/coil/compose/AsyncImagePainter.kt index 567be8290f..c2ac419786 100644 --- a/coil-compose-base/src/main/java/coil/compose/AsyncImagePainter.kt +++ b/coil-compose-base/src/main/java/coil/compose/AsyncImagePainter.kt @@ -30,6 +30,7 @@ import androidx.compose.ui.graphics.vector.ImageVector import androidx.compose.ui.layout.ContentScale import androidx.compose.ui.platform.LocalInspectionMode import androidx.compose.ui.unit.Constraints +import androidx.compose.ui.util.trace import coil.ImageLoader import coil.compose.AsyncImagePainter.Companion.DefaultTransform import coil.compose.AsyncImagePainter.State @@ -197,7 +198,7 @@ private fun rememberAsyncImagePainter( onState: ((State) -> Unit)?, contentScale: ContentScale, filterQuality: FilterQuality, -): AsyncImagePainter { +): AsyncImagePainter = trace("rememberAsyncImagePainter") { val request = requestOf(state.model) validateRequest(request) @@ -253,7 +254,7 @@ class AsyncImagePainter internal constructor( private set /** The current [ImageRequest]. */ - var request: ImageRequest by mutableStateOf(request) + var request by mutableStateOf(request) internal set /** The current [ImageLoader]. */ @@ -282,9 +283,9 @@ class AsyncImagePainter internal constructor( } @OptIn(ExperimentalCoroutinesApi::class) - override fun onRemembered() { + override fun onRemembered() = trace("AsyncImagePainter.onRemembered") { // Short circuit if we're already remembered. - if (rememberScope != null) return + if (rememberScope != null) return@trace // Create a new scope to observe state and execute requests while we're remembered. val scope = CoroutineScope(SupervisorJob() + Dispatchers.Main.immediate) @@ -297,7 +298,7 @@ class AsyncImagePainter internal constructor( if (isPreview) { val request = request.newBuilder().defaults(imageLoader.defaults).build() updateState(State.Loading(request.placeholder?.toPainter())) - return + return@trace } // Observe the current request and execute any emissions. diff --git a/coil-sample-compose/build.gradle.kts b/coil-sample-compose/build.gradle.kts index c8d2350ab5..43263ecfa5 100644 --- a/coil-sample-compose/build.gradle.kts +++ b/coil-sample-compose/build.gradle.kts @@ -42,4 +42,5 @@ dependencies { implementation(libs.androidx.activity.compose) implementation(libs.androidx.lifecycle.viewmodel.compose) implementation(libs.compose.material) + implementation(libs.androidx.compose.runtime.tracing) } diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 0f58afaf70..7b43ed91b2 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -8,6 +8,8 @@ ktlint = "1.0.1" okhttp = "4.12.0" okio = "3.8.0" roborazzi = "1.7.0" +perfetto = "1.0.0" +runtimeTracing = "1.0.0-beta01" [plugins] binaryCompatibility = "org.jetbrains.kotlinx.binary-compatibility-validator:0.14.0" @@ -25,9 +27,10 @@ accompanist-drawablepainter = "com.google.accompanist:accompanist-drawablepainte androidx-activity = { module = "androidx.activity:activity-ktx", version.ref = "androidx-activity" } androidx-activity-compose = { module = "androidx.activity:activity-compose", version.ref = "androidx-activity" } +androidx-compose-runtime-tracing = { module = "androidx.compose.runtime:runtime-tracing", version.ref = "runtimeTracing" } androidx-appcompat-resources = "androidx.appcompat:appcompat-resources:1.6.1" androidx-annotation = "androidx.annotation:annotation:1.7.1" -androidx-benchmark-macro = "androidx.benchmark:benchmark-macro-junit4:1.2.3" +androidx-benchmark-macro = "androidx.benchmark:benchmark-macro-junit4:1.2.4" androidx-collection = "androidx.collection:collection:1.4.0" androidx-constraintlayout = "androidx.constraintlayout:constraintlayout:2.1.4" androidx-core = "androidx.core:core-ktx:1.12.0" @@ -43,6 +46,8 @@ androidx-test-junit = "androidx.test.ext:junit-ktx:1.1.5" androidx-test-rules = "androidx.test:rules:1.5.0" androidx-test-runner = "androidx.test:runner:1.5.2" androidx-test-uiautomator = "androidx.test.uiautomator:uiautomator:2.2.0" +androidx-tracing-perfetto = { module = "androidx.tracing:tracing-perfetto", version.ref = "perfetto" } +androidx-tracing-perfetto-binary = { module = "androidx.tracing:tracing-perfetto-binary", version.ref = "perfetto" } androidx-vectordrawable-animated = "androidx.vectordrawable:vectordrawable-animated:1.1.0" compose-foundation = { module = "androidx.compose.foundation:foundation", version.ref = "compose" }