Fixed
Status Update
Comments
ja...@gmail.com <ja...@gmail.com> #2
Another non-contrived example is Build.VERSION.SDK_INT. I can remove entire conditionals with a value range but cannot remove the static field read:
000130: |[000130] AssumeValues.main:([Ljava/lang/String;)V
000140: 6001 0000 |0000: sget v1, Landroid/os/Build$VERSION;.SDK_INT:I // field@0000
000144: 6201 0100 |0002: sget-object v1, Ljava/lang/System;.out:Ljava/io/PrintStream; // field@0001
000148: 1a00 0100 |0004: const-string v0, "21+" // string@0001
00014c: 6e20 0100 0100 |0006: invoke-virtual {v1, v0}, Ljava/io/PrintStream;.println:(Ljava/lang/String;)V // method@0001
000152: 0e00 |0009: return-void
000130: |[000130] AssumeValues.main:([Ljava/lang/String;)V
000140: 6001 0000 |0000: sget v1, Landroid/os/Build$VERSION;.SDK_INT:I // field@0000
000144: 6201 0100 |0002: sget-object v1, Ljava/lang/System;.out:Ljava/io/PrintStream; // field@0001
000148: 1a00 0100 |0004: const-string v0, "21+" // string@0001
00014c: 6e20 0100 0100 |0006: invoke-virtual {v1, v0}, Ljava/io/PrintStream;.println:(Ljava/lang/String;)V // method@0001
000152: 0e00 |0009: return-void
ja...@gmail.com <ja...@gmail.com> #3
So I think the call remains because of classloading and the side-effects it might cause? I hoped that by using -assumenosideeffects it would indicate that I don't care about classloading side-effects allowing the method invoke / field lookup to be completely eliminated.
sg...@google.com <sg...@google.com> #4
The short story here is that we don't apply -assumexxx on methods on library classes. So if you change your code to:
import java.util.Random;
class MyRandom {
Random random = new Random();
boolean nextBoolean() {
return random.nextBoolean();
}
}
public class Test {
public static void main(String... args) {
MyRandom random = new MyRandom();
if (random.nextBoolean()) {
System.out.println("True");
} else {
System.out.println("False");
}
}
}
and the rules to
-keep public class * {
public static void main(...);
}
-assumenosideeffects class MyRandom {
boolean nextBoolean() return false;
}
the output is:
000134: |[000134] Test.main:([Ljava/lang/String;)V
000144: 2201 0500 |0000: new-instance v1, Ljava/util/Random; // type@0005
000148: 7010 0200 0100 |0002: invoke-direct {v1}, Ljava/util/Random;.<init>:()V // method@0002
00014e: 6201 0000 |0005: sget-object v1, Ljava/lang/System;.out:Ljava/io/PrintStream; // field@0000
000152: 1a00 0200 |0007: const-string v0, "False" // string@0002
000156: 6e20 0100 0100 |0009: invoke-virtual {v1, v0}, Ljava/io/PrintStream;.println:(Ljava/lang/String;)V // method@0001
00015c: 0e00 |000c: return-void
With -asumevalues the call to nextRandom stays:
000154: |[000154] Test.main:([Ljava/lang/String;)V
000164: 2201 0500 |0000: new-instance v1, Ljava/util/Random; // type@0005
000168: 7010 0200 0100 |0002: invoke-direct {v1}, Ljava/util/Random;.<init>:()V // method@0002
00016e: 6e10 0300 0100 |0005: invoke-virtual {v1}, Ljava/util/Random;.nextBoolean:()Z // method@0003
000174: 6201 0000 |0008: sget-object v1, Ljava/lang/System;.out:Ljava/io/PrintStream; // field@0000
000178: 1a00 0200 |000a: const-string v0, "False" // string@0002
00017c: 6e20 0100 0100 |000c: invoke-virtual {v1, v0}, Ljava/io/PrintStream;.println:(Ljava/lang/String;)V // method@0001
000182: 0e00 |000f: return-void
The way these rules are processed is that R8 will only apply a -asumexxx rule if there is a single definition of the target for the call. If the example is extended to
import java.util.Random;
class MyRandom {
Random random = new Random();
boolean nextBoolean() {
return random.nextBoolean();
}
}
class MyRandomSub extends MyRandom {
boolean nextBoolean() {
return !random.nextBoolean();
}
}
public class Test {
public static void main(String... args) {
MyRandom random = new MyRandom();
MyRandomSub randomSub = new MyRandomSub();
System.out.println(randomSub.nextBoolean());
if (random.nextBoolean()) {
System.out.println("True");
} else {
System.out.println("False");
}
}
}
then the rule would no longer apply, as random.nextBoolean() might hit either MyRandom.nextBoolean() or MyRandomSub.nextBoolean() (with proper type analysis in this concrete example we could figure out that there is only one target). For library provided classes we can not know if there is just a single target, as the library can have a number of implementation sub-classes.
This might be too conservative, but that is the current implementation.
import java.util.Random;
class MyRandom {
Random random = new Random();
boolean nextBoolean() {
return random.nextBoolean();
}
}
public class Test {
public static void main(String... args) {
MyRandom random = new MyRandom();
if (random.nextBoolean()) {
System.out.println("True");
} else {
System.out.println("False");
}
}
}
and the rules to
-keep public class * {
public static void main(...);
}
-assumenosideeffects class MyRandom {
boolean nextBoolean() return false;
}
the output is:
000134: |[000134] Test.main:([Ljava/lang/String;)V
000144: 2201 0500 |0000: new-instance v1, Ljava/util/Random; // type@0005
000148: 7010 0200 0100 |0002: invoke-direct {v1}, Ljava/util/Random;.<init>:()V // method@0002
00014e: 6201 0000 |0005: sget-object v1, Ljava/lang/System;.out:Ljava/io/PrintStream; // field@0000
000152: 1a00 0200 |0007: const-string v0, "False" // string@0002
000156: 6e20 0100 0100 |0009: invoke-virtual {v1, v0}, Ljava/io/PrintStream;.println:(Ljava/lang/String;)V // method@0001
00015c: 0e00 |000c: return-void
With -asumevalues the call to nextRandom stays:
000154: |[000154] Test.main:([Ljava/lang/String;)V
000164: 2201 0500 |0000: new-instance v1, Ljava/util/Random; // type@0005
000168: 7010 0200 0100 |0002: invoke-direct {v1}, Ljava/util/Random;.<init>:()V // method@0002
00016e: 6e10 0300 0100 |0005: invoke-virtual {v1}, Ljava/util/Random;.nextBoolean:()Z // method@0003
000174: 6201 0000 |0008: sget-object v1, Ljava/lang/System;.out:Ljava/io/PrintStream; // field@0000
000178: 1a00 0200 |000a: const-string v0, "False" // string@0002
00017c: 6e20 0100 0100 |000c: invoke-virtual {v1, v0}, Ljava/io/PrintStream;.println:(Ljava/lang/String;)V // method@0001
000182: 0e00 |000f: return-void
The way these rules are processed is that R8 will only apply a -asumexxx rule if there is a single definition of the target for the call. If the example is extended to
import java.util.Random;
class MyRandom {
Random random = new Random();
boolean nextBoolean() {
return random.nextBoolean();
}
}
class MyRandomSub extends MyRandom {
boolean nextBoolean() {
return !random.nextBoolean();
}
}
public class Test {
public static void main(String... args) {
MyRandom random = new MyRandom();
MyRandomSub randomSub = new MyRandomSub();
System.out.println(randomSub.nextBoolean());
if (random.nextBoolean()) {
System.out.println("True");
} else {
System.out.println("False");
}
}
}
then the rule would no longer apply, as random.nextBoolean() might hit either MyRandom.nextBoolean() or MyRandomSub.nextBoolean() (with proper type analysis in this concrete example we could figure out that there is only one target). For library provided classes we can not know if there is just a single target, as the library can have a number of implementation sub-classes.
This might be too conservative, but that is the current implementation.
ja...@gmail.com <ja...@gmail.com> #5
Ok so the fact that type hierarchy analysis isn't done makes sense.
The actual target for this bug was View.isInEditMode(). I was writing the rule like this:
-assumevalues class android.view.View {
boolean isInEditMode() return false;
}
-assumevalues class * extends android.view.View {
boolean isInEditMode() return false;
}
So that it would cover both
if (new View(null).isInEditMode()) ...
and
if (new LinearLayout(null).isInEditMode()) ...
But if type hierarchy analysis isn't done for library types I understand why this won't work. Maybe a feature request for the future.
Are -assumevalues for field reads in library classes also not supported? The essential target here being android.os.Build.VERSION.SDK_INT which, when combined with -assumevalues, eliminate *massive* amounts of compatibility abstraction from the support libraries. When I do an -assumevalues on SDK_INT, branches are correctly eliminated but the bytecode still contains a sget for the field. Can that field read be eliminated?
The actual target for this bug was View.isInEditMode(). I was writing the rule like this:
-assumevalues class android.view.View {
boolean isInEditMode() return false;
}
-assumevalues class * extends android.view.View {
boolean isInEditMode() return false;
}
So that it would cover both
if (new View(null).isInEditMode()) ...
and
if (new LinearLayout(null).isInEditMode()) ...
But if type hierarchy analysis isn't done for library types I understand why this won't work. Maybe a feature request for the future.
Are -assumevalues for field reads in library classes also not supported? The essential target here being android.os.Build.VERSION.SDK_INT which, when combined with -assumevalues, eliminate *massive* amounts of compatibility abstraction from the support libraries. When I do an -assumevalues on SDK_INT, branches are correctly eliminated but the bytecode still contains a sget for the field. Can that field read be eliminated?
ag...@google.com <ag...@google.com>
ja...@gmail.com <ja...@gmail.com> #6
With issue 110178672 landing to automatically remove impossible SDK_INT conditions, the SDK_INT field read still remains. Even if this feature won't be supported in the general case for library fields ( issue 123080377 ), SDK_INT should be special-cased because it's known to not cause side-effects.
ag...@google.com <ag...@google.com>
sg...@google.com <sg...@google.com> #7
The field read of SDK_INT can cause the static initializer of android.os.Build.VERSION to be called. The synthesized rule added is of type -assumevalues not -assumenosideeffect. I could try to change that, as postponing <clinit> of android.os.Build.VERSION should be benign.
ch...@google.com <ch...@google.com>
ch...@google.com <ch...@google.com> #8
Regarding comment #6 , we now synthesize the rule `-assumenosideeffects android.os.Build$VERSION { public static final SDK_INT return <MIN_API_LEVEL>..<MAX_INT>; }` instead of an `-assumevalues` rule. This way R8 allows removing the static-get instruction despite the fact that the class initialization of android.os.Build$VERSION could have side effects. Naturally such -assumenosideeffects rules should be used with caution because they may change the behavior of the program.
Note also that it is possible to use -assumevalues and -assumenosideeffects rules for library fields.
Note also that it is possible to use -assumevalues and -assumenosideeffects rules for library fields.
ch...@google.com <ch...@google.com> #9
Marking this as fixed. I filed feature request b/133208961 to support the example in comment #5 (although there is a possibility that this will only be supported for program classes).
Description
$ java -version
java version "1.8.0_144"
Java(TM) SE Runtime Environment (build 1.8.0_144-b01)
Java HotSpot(TM) 64-Bit Server VM (build 25.144-b01, mixed mode)
$ java -jar ~/dev/android/r8/build/libs/r8.jar --version
R8 1.3.11-dev
build engineering
$ cat Test.java
import java.util.Random;
public class Test {
public static void main(String... args) {
Random random = new Random();
if (random.nextBoolean()) {
System.out.println("True");
} else {
System.out.println("False");
}
}
}
$ cat keep.txt
-keep public class * {
public static void main(...);
}
-assumevalues class java.util.Random {
boolean nextBoolean() return false;
}
$ javac Test.java
$ java -jar ~/dev/android/r8/build/libs/r8.jar --lib $ANDROID_HOME/platforms/android-28/android.jar --release --output . --pg-conf keep.txt Test.class
$ $ANDROID_HOME/build-tools/27.0.3/dexdump -d classes.dex
(snipped)
000158: |[000158] Test.main:([Ljava/lang/String;)V
000168: 2201 0500 |0000: new-instance v1, Ljava/util/Random; // type@0005
00016c: 7010 0200 0100 |0002: invoke-direct {v1}, Ljava/util/Random;.<init>:()V // method@0002
000172: 6e10 0300 0100 |0005: invoke-virtual {v1}, Ljava/util/Random;.nextBoolean:()Z // method@0003
000178: 0a01 |0008: move-result v1
00017a: 3801 0a00 |0009: if-eqz v1, 0013 // +000a
00017e: 6201 0000 |000b: sget-object v1, Ljava/lang/System;.out:Ljava/io/PrintStream; // field@0000
000182: 1a00 0900 |000d: const-string v0, "True" // string@0009
000186: 6e20 0100 0100 |000f: invoke-virtual {v1, v0}, Ljava/io/PrintStream;.println:(Ljava/lang/String;)V // method@0001
00018c: 2808 |0012: goto 001a // +0008
00018e: 6201 0000 |0013: sget-object v1, Ljava/lang/System;.out:Ljava/io/PrintStream; // field@0000
000192: 1a00 0200 |0015: const-string v0, "False" // string@0002
000196: 6e20 0100 0100 |0017: invoke-virtual {v1, v0}, Ljava/io/PrintStream;.println:(Ljava/lang/String;)V // method@0001
00019c: 0e00 |001a: return-void
Using only -assumevalues has no effect on the bytecode as the call to nextBoolean() and both branches are still present.
I also tried:
-keep public class * {
public static void main(...);
}
-assumevalues class java.util.Random {
boolean nextBoolean() return false;
}
-assumenosideeffects class java.util.Random {
boolean nextBoolean();
}
for the keep.txt file. No change.
The -assumenosideeffects syntax also allows the 'return' clause so I tried that. I also tried removing -assumevalues. All 4 combinations do not result in the elimination of this method call.
This example is contrived, obviously, but there exists at least one valid use case for this: View.isInEditMode() always returns false at runtime and never needs invoked and any code inside a conditional eliminated.