Enable R8 optimization of Dispatchers.Main loading #1232
Conversation
The developer still has to disable the FastServiceLoader manually if they're using R8. R8 is then able to optimize away the ServiceLoader and reflection entirely, resulting in direct class instantiation and no extra I/O on calling thread. Fixes Kotlin#1231
|
@wojtek-kalicinski Would it possible for the R8 team to disable |
|
I'm looking into ways we can do it, I agree that having a manual step that developers have to remember about does not solve the issue fully, but I just want to unblock the R8 optimization first. |
|
@wojtek-kalicinski Not sure if out of scope for this PR but |
Couldn't the default value just be |
|
I'm working on a test for this which actually invokes R8 and verifies that all ServiceLoader calls are removed from the final dex to prevent regression or new occurrences in the wrong form. Feel free to merge without it though, as it might be a few days. |
|
I agree, turning it off by default would make sense. Then, we might get AGP
turn it on by default when R8 is not enabled (i.e. minifyEnabled = false),
or if not, we might still do it ourselves, which is less of an issue.
…On Thu, May 30, 2019, 8:15 PM Nicklas Ansman Giertz < ***@***.***> wrote:
I'm looking into ways we can do it, I agree that having a manual step that
developers have to remember about does not solve the issue fully, but I
just want to unblock the R8 optimization first.
I'll follow up on this if I have a solution.
Couldn't the default value just be false since the FastServiceLoader is
only really needed on Android? JAR verification seems like a useful thing
on the server side so it would make sense to hade it use the regular
service loader by default.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#1232?email_source=notifications&email_token=ABVG6BO7NWKJKHFADYLVU3TPYAKTPA5CNFSM4HRETU42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWTCXMA#issuecomment-497429424>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABVG6BKWHBW5STGJBNJMDALPYAKTPANCNFSM4HRETU4Q>
.
|
There was a problem hiding this comment.
LGTM.
I'm going to disable FastServiceLoader by default if optimization works on the latest R8 versions.
The rationale is quite simple: if users use the latest version of kx.coroutines, they'll probably use the latest version of Android toolchain, then it makes sense to simplify the development process for then.
For the rest of the users, we can recommend either to enable FSL or to update R8 version.
Could you please specify since which R8 version is this optimization present?
|
I would argue that we should not disable FastServiceLoader by default just yet, and discuss this further.
It's difficult to come up with a solution that works for both groups, but I have some ideas that I'd like to further discuss. I'll follow up in a bit. |
Also to add to my previous comment, if I understand the R8 releases correctly, this optimization will be present in future R8 releases from the 1.5.X branch, so this further reinforces why we should not disable the fast loader for everyone just yet. |
|
Out of curiosity is there a proper way to set system properties on Android? Or it's to be done in code as early as possible in application.OnCreate? |
|
@wojtek-kalicinski good points, I'll merge it as is in that case. Please let us know when R8 is released so we could update our documentation |
The developer still has to disable the FastServiceLoader manually
if they're using R8.
R8 is then able to optimize away the ServiceLoader and reflection
entirely, resulting in direct class instantiation and no extra I/O
on calling thread.
Fixes #1231