Asynchronous initialization of ImageLoader

[mlykotom]

Is your feature request related to a problem? Please describe.
In Now in Android we had the Application implement ImageLoaderFactory and initialize the ImageLoader with the newImageLoader() method.
We realised that the initialization happens on the main thread with the composition of the first image on screen, which causes some frames to be skipped (~15ms on Pixel 6 in benchmark build).

What we did was to start initialization of Coil on a background thread and in case it'd take too long, block the first image to wait for the initialization (android/nowinandroid#1190).

Describe the solution you'd like

I think it's not 100% obvious that Coil will initialize on the main thread and given it's initializing OkHttp, it may take significant time to do so.

a) Could there be a similar way baked-in to Coil to do initialization in a non-blocking way?
b) Should this be documented in the documentation so that people are aware this might happen?
c) ?

Additional context

[colinrtwhite]

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!

[colinrtwhite]

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()}")

[mlykotom]

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.

[mlykotom]

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

[colinrtwhite]

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.

[colinrtwhite]

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.

[mlykotom]

Awesome! Thanks! 🎉

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

[yschimke]

PR for a cautionary fix on OkHttp 5.x

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