OkHttp lazy initialization #1190
Conversation
|
100ms 🤯 can you measure this on release builds as well? Debug builds are notoriously slow because they are not optimized and contain debugging features that slows down everything (especially Compose related code) |
app/src/main/kotlin/com/google/samples/apps/nowinandroid/NiaApplication.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/google/samples/apps/nowinandroid/util/ImageLoaderAsyncFactory.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/google/samples/apps/nowinandroid/util/ImageLoaderAsyncFactory.kt
Outdated
Show resolved
Hide resolved
I'm always measuring on release (benchmark) build 😁 . I think on the new versions of Android might not be as problematic, but on Pixel 6 it takes ~15ms. |
app/src/main/java/com/google/samples/apps/nowinandroid/util/ImageLoaderAsyncFactory.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/google/samples/apps/nowinandroid/util/ImageLoaderAsyncFactory.kt
Outdated
Show resolved
Hide resolved
...etwork/src/main/kotlin/com/google/samples/apps/nowinandroid/core/network/di/NetworkModule.kt
Show resolved
Hide resolved
|
@hoc081098 could you pls sign the CLA so that we can merge this PR? Thank you 😊 |
app/src/main/java/com/google/samples/apps/nowinandroid/util/ImageLoaderAsyncFactory.kt
Outdated
Show resolved
Hide resolved
I have signed the CLA for "[email protected]". Don't know why the check failed 😂? |
...etwork/src/main/kotlin/com/google/samples/apps/nowinandroid/core/network/di/NetworkModule.kt
Show resolved
Hide resolved
Pls check the CLA failure for more info. Maybe you used a different email? Not sure what went wrong then. |
This way, we can load Coil's backend on a background thread and not block the MainThread with it. Previously, the Coil image loader was initialized with the first composed image, which caused ~10ms duration and most likely skipped frames. Change-Id: Iaa583b6adc1df7d7a51dbae1473e539f2c0b0b62
Co-authored-by: Yuri Schimke <[email protected]>
mlykotom
force-pushed
the
mlykotom/coil-async-init
branch
2 times, most recently
from
29fa6c9
to
863bfd3
Compare
Change-Id: Icbbdbcbcac1a6275857ebe998509f1e09109db7a
Change-Id: I641187ce277f434c6fca49a11b3cfccd50ecf5da
Change-Id: I0a77eb6457cac27c1a4d604c8efdcbbdce95bc48
Change-Id: I859babab7278137a4a2e49e5a085c65632888dd0
Change-Id: I347f1080ab5adf774a0cdd3c659cbf25c4820f9a
Change-Id: Ic7a6887b76caf26f00b58b0753271d426b67e75b
As discussed in coil-kt/coil#2097 the problem is caused by regitering system services, which will be fixed in 2.6.0 Change-Id: I9085309780508137f10b25ff82deed3c62e5d159
mlykotom
force-pushed
the
mlykotom/coil-async-init
branch
from
863bfd3
to
58483b6
Compare
|
Rebased on |
Change-Id: I847827efc08f5cbc00f0c809ad992c267f826e50
There was a problem hiding this comment.
Awesome! If I'm understanding it right - we are now lazily creating the okHttpCallFactory, both for our own normal network calls, and once the Coil changes are released, for Coil as well.
Code looks good, maybe the title of the PR should be "Lazily initialize okHttpCallFactory" or something similar if we aren't affecting Coil too directly in this PR?
|
We're initializing OkHttp lazily async both for our Retrofit and Coil implementation. So with this PR we have half of the improvement. The remaining work for Coil library is to lazily initialize system callbacks on a worker thread. Also, renamed the PR. Thanks for all reviews! ❤️ |
What I have done and why
Move Coil's initialization with OkHttp to a background thread.
Without this change, Coil and OkHttp is initialized with the first composed image, which causes skipped frames, because the initialization can take up to 100ms.
Do tests pass?
DemoDebugvariant:./gradlew testDemoDebug./gradlew --init-script gradle/init.gradle.kts spotlessApply