Skip to content

Commit

Permalink
Improve RealImageLoader.execute to properly call async request (#2205)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
mlykotom committed May 10, 2024
1 parent 95edaae commit bacaba0
Show file tree
Hide file tree
Showing 7 changed files with 162 additions and 16 deletions.
27 changes: 17 additions & 10 deletions coil-base/src/main/java/coil/RealImageLoader.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 8 additions & 0 deletions coil-benchmark/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -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<Measurement> {
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,
),
)
}
}
}
}
66 changes: 66 additions & 0 deletions coil-benchmark/src/main/java/coil/benchmark/ScrollBenchmark.kt
Original file line number Diff line number Diff line change
@@ -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)
},
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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]. */
Expand Down Expand Up @@ -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)
Expand All @@ -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.
Expand Down
1 change: 1 addition & 0 deletions coil-sample-compose/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
7 changes: 6 additions & 1 deletion gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand All @@ -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" }
Expand Down

0 comments on commit bacaba0

Please sign in to comment.