Asynchronous initialization of ImageLoader about coil HOT 8 CLOSED

There's a couple of problems measuring the timing this way:

1. In general, you shouldn't measure performance on emulator as this way you effectively measure how powerful hosting machine you have
2. This way, you are can be hitting all the system caches, which doesn't show in the real app.

What I did to benchmak this, was using Macrobenchmark in the real app with StartupMode.COLD to see the worst-case scenario.

For 10 iterations, I see these results

NiaImageLoaderMs                        min     2.0,   median     4.2,   max     6.9
Traces: Iteration 0 1 2 3 4 5 6 7 8 9

I further started digging into what's happening and I think I know now.

I followed you advice to only have OkHttp asynchronously, but keep the ImageLoader on-demand and this is what I see in the traces.

When the first image is shown on screen (in NIA it's the SingleTopicButton), it starts to initialize Coil, which takes ~7ms.
If this was during startup, it wouldn't be much of an issue, but with a first composition, it might be problematic (trace here).

The cause are (my hypothesis) the binder transaction sections, which comes from the SystemCallbacks.

I think Coil should be able to register the system services on a Default thread to prevent the initialization hit and then it won't matter when the ImageLoader is actually initialized.

from coil.

Fixed here in the 3.x release. I'll also backport the change to the next release, 2.6.0.

I tried running my own macrobenchmark test after the changes, but the initialization method didn't appear in the measured methods.

from coil.

Awesome! Thanks! 🎉

If you'd like to help diagnose the macrobenchmark tests, maybe ping me on ASG?

from coil.

It looks like you were initializing your OkHttpClient eagerly here, which slowed down the ImageLoader creation. Coil already supports lazy initialization of the OkHttpClient using the ImageLoader.Builder.callFactory { } function that accepts a callback that'll be invoked on a background thread. By default it'll create the OkHttpClient using that method.

newImageLoader is invoked the first time Coil.imageLoader() is called, which is generally on the main thread, but doesn't have to be. I don't think we should eagerly asynchronously initialize the singleton ImageLoader for a couple reasons:

• ImageLoaders are cheap to initialize (see benchmark below).
• Eagerly initializing the ImageLoader would require importing androidx.startup.
• We wouldn't be able to eagerly initialize on non-Android platforms so the behaviour wouldn't be consistent.
• Eagerly initializing would create a race condition if the user uses Coil.setImageLoader before the first Coil.imageLoader call.

For docs, I'd be open to a PR that improves this!

from coil.

I ran a quick benchmark on a Pixel 7 emulator and an ImageLoader with default arguments takes 0.1 milliseconds to initialize. The result is similar when using ImageLoader.Builder.callFactory { }. Here's the code:

val iterations = 1_000
var sum = 0L
var imageLoader: ImageLoader?
repeat(iterations) {
    sum += measureNanoTime {
        imageLoader = ImageLoader(context)
    }
    imageLoader?.shutdown()
}
println("BENCHMARK OUTPUT: ${sum / iterations.toDouble()}")

from coil.

When I set .networkObserverEnabled(false), then the initialization drops to ~1.5ms. (trace)

from coil.

Ah thanks for digging in! Agreed - SystemCallbacks should be registered lazily on a background thread. We should be able to delay checking if the device has a suitable network connection until we're on a background thread and move the network callbacks + memory callbacks registration to a background thread.

from coil.

PR for a cautionary fix on OkHttp 5.x

https://github.com/square/okhttp/pull/8248/files

from coil.