Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Investigate the possibility of using D8/R8/L8 as shirnking & desugaring tool #171

Open
Noisyfox opened this issue Apr 3, 2022 · 13 comments · May be fixed by multi-os-engine/moe-plugin-gradle#11

Comments

@Noisyfox
Copy link
Member

Noisyfox commented Apr 3, 2022

No description provided.

@Berstanio
Copy link

Berstanio commented Apr 3, 2022

I have already looked into this a while ago.
One problem seems to be, that d8 can't properly apply desugared lib configuration if they are java7 java.* files in the moe-core jar. One way would be to exclude the java.* classes from the moe-core and dex them independently. But I also don't know whether moe even needs L8/desugare config.
I tried it without it and got to the point where the application was "running", but ended directly in a infinite call recursion (StackOverflow).

grafik
Haven't tested yet whether this is because of the dexer or the backporting. It seems to be because of the processing with d8.

@Berstanio
Copy link

Berstanio commented Apr 3, 2022

Seems to be the d8 processing.
Result of d8:
grafik
Result of proguard:
grafik
Bytecode before processing:
grafik

So can it be, that because moe uses now the new Buffer API's the d8 version isn't able to call the super method and falls back to the method in MappedByteBuffer again -> infinte recursion?

Reverting the backporting of the Java 9+ Buffer API's solves it. After reverting this a sample app worked perfectly fine!
d8 adds the relevant casts etc. by itself so there seems to be no need to have the overriden Buffer methods with the different return type. Other Buffer API's should work fine.

@PokeMMO
Copy link

PokeMMO commented Apr 4, 2022

I wonder what moe is doing differently that would cause the recursion than a normal modern android device with a native java 8 api.

@Berstanio
Copy link

I wonder what moe is doing differently that would cause the recursion than a normal modern android device with a native java 8 api.

My guess is newer libcore does some kind of magic to support both:
grafik
https://cs.android.com/android/platform/superproject/+/master:libcore/dalvik/src/main/java/dalvik/annotation/codegen/CovariantReturnType.java

@Noisyfox
Copy link
Member Author

Noisyfox commented Apr 5, 2022

I wonder what moe is doing differently that would cause the recursion than a normal modern android device with a native java 8 api.

Because MOE 1.x is based on an ART version that does not support Java 8 language & JVM features.

@Berstanio
Copy link

I got a POC for this working. Atm it just uses d8, because r8 was throwing a NPE somewhere, but I haven't really looked into it yet.
I would now open the pr's for the poc.
I was also wondering whether
"-if interface ...
-keep class <1>$Impl"
would do the job of "-keepcompanionclasses", the docs were relatively vague about what "present" means. But I would guess not?

@Noisyfox
Copy link
Member Author

I was also wondering whether
"-if interface ...
-keep class <1>$Impl"
would do the job of "-keepcompanionclasses", the docs were relatively vague about what "present" means. But I would guess not?

It doesn't work the same way. Present literally means if the class A exist then keep class B, but it doesn't care if class A will be kept or not.

@Berstanio
Copy link

Berstanio commented Apr 24, 2022

So I made some tests with R8. Nothing intensiv but I think the results are still interesting!

  1. There is a massive size reduction and I don't know how this can be. Proguard + dx leads to a oat size of 98mb and r8 leads to a oat size of 24mb.
  2. R8 is waay faster! With a clean build to launch of App I got 00:57 min with R8 and 01:51 min with proguard/dx. The R8 build can also be optimized with not letting ClassValidate process the moe core and handle the jaring better.

Also R8 adds back the possibility of predexing the core. There is a "desugared-lib" parameter that specifies rewrite rules. Sadly I couldn't implement this yet, because dex2oat segfaults when doing it. Haven't looked into it yet deeper.

// EDIT
About the desugared-lib. The problem is the following: When using L8 the dex only contains classes that got rewritten with the config file. All other classes are not dexed. They would need to be dexed with standard d8/r8. The problem is, we can't simply run d8 over the moe core, because it contains the classes that got rewritten. And d8 complains if it tries to process a class that is specified by a rewrite rule.
The only solution, if there isn't a option to tell L8 to dex all, not just the rewritten classes, would be to seperate the moe-core in two parts. The one that contains the pure java 7 libs and one that contains the additions that needs to be backported.

@Berstanio
Copy link

Berstanio commented Apr 28, 2022

Instead of making a R8 "-keepcompanionclasses" extension, I would rather use the ClassValidator to output some keep configs.
Is probably way easier and some code can already be based of the ReflectionConfigCollector.
Anything speaking against this?

@Berstanio
Copy link

Berstanio commented May 4, 2022

Is there a specific reason the dex2oat task uses the Quick backend by default?
While testing a libGDX App against r8 I encountered a weird bug. It seems the gc wants to free a object that is still hold?
This only happens on the simulator (only x86_64) in combination with r8 and until now I got it only reproduced on a libGDX App (but haven't tried to minimize the reproducer yet).
Haven't debugged the gc yet, but here is the error: https://gist.github.com/Berstanio/69744566b0c8a8b2a0d5a5eaef5c33ba
But during testing I changed the compiler backend to "optimizing" and this fixed the error.
So do you know any downside of using the Optimizing backend for dex2oat? As far as I am aware this is also usally the default backend for the dex2oat.

@Berstanio
Copy link

Hmm thinking through the issue, that the whole core needs to be evaluated for correct backporting, I might got a idea how to fix this.
R8 provides the capability of providing a backport configuration. In my last attempts I tried to apply a backport configuration to the core and and the app, but failed because of the reasons explained in #171 (comment)
But when we would run just d8 on the core d8 will probably backport the classes in a predictable way. If we know just apply than a crafted backport configuration to the app r8 run, we might be able to backport this stuff also correctly. At a first look it seems promising!

@Berstanio
Copy link

Berstanio commented May 11, 2022

POC was successful 🥳
So we can get back to predexing and probably way faster build times!!
Also, the solution is soo simple! I don't know why I didn't realised it earlier...

How it works:
We keep the core as is, with all the java 8+ apis. Than we run L8 with the desugared-lib config on the desugared jdk libs with the moe-core as "--lib" parameter.
Now we have a desugared_jdk_lib dex, that contains all desugared classes with a j$ prefix. Than we just dex the core with d8.
Now when we want to build a app we apply the desugared_lib config used earlier also to r8. r8 will rewrite than all calls that needs to be desugared to the j$ package (the classes inside the desugared_jdk dex).
Now when we run dex2oat we just apply all dexes and we are done (app, core, desugared_lib)!

@Berstanio
Copy link

I was also wondering whether
"-if interface ...
-keep class <1>$Impl"
would do the job of "-keepcompanionclasses", the docs were relatively vague about what "present" means. But I would guess not?

It doesn't work the same way. Present literally means if the class A exist then keep class B, but it doesn't care if class A will be kept or not.

Actually, it looks like r8 is than inconsistent with proguard. I made a few tests and it looks like it in r8, "-if" means, that the condition class needs to be also kept.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants