Assume that class initialization of library interfaces do not have side effects
Bug: 132683206
Change-Id: I3c8d7c113c9e6bb26755847de9076e8a1fa00f72
diff --git a/src/main/java/com/android/tools/r8/graph/DexClass.java b/src/main/java/com/android/tools/r8/graph/DexClass.java
index fb96f76..3f3577a 100644
--- a/src/main/java/com/android/tools/r8/graph/DexClass.java
+++ b/src/main/java/com/android/tools/r8/graph/DexClass.java
@@ -718,23 +718,30 @@
return appView.appInfo().isExternalizable(type);
}
- public boolean classInitializationMayHaveSideEffects(DexDefinitionSupplier definitions) {
- return classInitializationMayHaveSideEffects(definitions, Predicates.alwaysFalse());
+ public boolean classInitializationMayHaveSideEffects(AppView<?> appView) {
+ return classInitializationMayHaveSideEffects(appView, Predicates.alwaysFalse());
}
public boolean classInitializationMayHaveSideEffects(
- DexDefinitionSupplier definitions, Predicate<DexType> ignore) {
- if (ignore.test(type)
- || definitions.dexItemFactory().libraryTypesWithoutStaticInitialization.contains(type)) {
+ AppView<?> appView, Predicate<DexType> ignore) {
+ if (ignore.test(type)) {
return false;
}
+ if (isLibraryClass()) {
+ if (isInterface()) {
+ return appView.options().libraryInterfacesMayHaveStaticInitialization;
+ }
+ if (appView.dexItemFactory().libraryClassesWithoutStaticInitialization.contains(type)) {
+ return false;
+ }
+ }
if (hasNonTrivialClassInitializer()) {
return true;
}
if (defaultValuesForStaticFieldsMayTriggerAllocation()) {
return true;
}
- return initializationOfParentTypesMayHaveSideEffects(definitions, ignore);
+ return initializationOfParentTypesMayHaveSideEffects(appView, ignore);
}
public Iterable<DexType> allImmediateSupertypes() {
@@ -746,18 +753,18 @@
return () -> iterator;
}
- public boolean initializationOfParentTypesMayHaveSideEffects(DexDefinitionSupplier definitions) {
- return initializationOfParentTypesMayHaveSideEffects(definitions, Predicates.alwaysFalse());
+ public boolean initializationOfParentTypesMayHaveSideEffects(AppView<?> appView) {
+ return initializationOfParentTypesMayHaveSideEffects(appView, Predicates.alwaysFalse());
}
public boolean initializationOfParentTypesMayHaveSideEffects(
- DexDefinitionSupplier definitions, Predicate<DexType> ignore) {
+ AppView<?> appView, Predicate<DexType> ignore) {
for (DexType iface : interfaces.values) {
- if (iface.classInitializationMayHaveSideEffects(definitions, ignore)) {
+ if (iface.classInitializationMayHaveSideEffects(appView, ignore)) {
return true;
}
}
- if (superType != null && superType.classInitializationMayHaveSideEffects(definitions, ignore)) {
+ if (superType != null && superType.classInitializationMayHaveSideEffects(appView, ignore)) {
return true;
}
return false;
diff --git a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
index c1c8cc6..cb9d725 100644
--- a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
+++ b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
@@ -419,9 +419,8 @@
public Set<DexType> libraryTypesAssumedToBePresent =
ImmutableSet.of(objectType, stringBufferType, stringBuilderType);
- public final Set<DexType> libraryTypesWithoutStaticInitialization =
- ImmutableSet.of(
- iteratorType, objectType, serializableType, stringBufferType, stringBuilderType);
+ public final Set<DexType> libraryClassesWithoutStaticInitialization =
+ ImmutableSet.of(objectType, stringBufferType, stringBuilderType);
private boolean skipNameValidationForTesting = false;
diff --git a/src/main/java/com/android/tools/r8/graph/DexType.java b/src/main/java/com/android/tools/r8/graph/DexType.java
index 801d6f8..51c6825 100644
--- a/src/main/java/com/android/tools/r8/graph/DexType.java
+++ b/src/main/java/com/android/tools/r8/graph/DexType.java
@@ -45,24 +45,24 @@
return false;
}
- public boolean classInitializationMayHaveSideEffects(DexDefinitionSupplier definitions) {
- return classInitializationMayHaveSideEffects(definitions, Predicates.alwaysFalse());
+ public boolean classInitializationMayHaveSideEffects(AppView<?> appView) {
+ return classInitializationMayHaveSideEffects(appView, Predicates.alwaysFalse());
}
public boolean classInitializationMayHaveSideEffects(
- DexDefinitionSupplier definitions, Predicate<DexType> ignore) {
- DexClass clazz = definitions.definitionFor(this);
- return clazz == null || clazz.classInitializationMayHaveSideEffects(definitions, ignore);
+ AppView<?> appView, Predicate<DexType> ignore) {
+ DexClass clazz = appView.definitionFor(this);
+ return clazz == null || clazz.classInitializationMayHaveSideEffects(appView, ignore);
}
- public boolean initializationOfParentTypesMayHaveSideEffects(AppInfo appInfo) {
- return initializationOfParentTypesMayHaveSideEffects(appInfo, Predicates.alwaysFalse());
+ public boolean initializationOfParentTypesMayHaveSideEffects(AppView<?> appView) {
+ return initializationOfParentTypesMayHaveSideEffects(appView, Predicates.alwaysFalse());
}
public boolean initializationOfParentTypesMayHaveSideEffects(
- AppInfo appInfo, Predicate<DexType> ignore) {
- DexClass clazz = appInfo.definitionFor(this);
- return clazz == null || clazz.initializationOfParentTypesMayHaveSideEffects(appInfo, ignore);
+ AppView<?> appView, Predicate<DexType> ignore) {
+ DexClass clazz = appView.definitionFor(this);
+ return clazz == null || clazz.initializationOfParentTypesMayHaveSideEffects(appView, ignore);
}
public boolean isAlwaysNull(AppView<AppInfoWithLiveness> appView) {
diff --git a/src/main/java/com/android/tools/r8/ir/code/InvokeStatic.java b/src/main/java/com/android/tools/r8/ir/code/InvokeStatic.java
index 93e98db..56f852b 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InvokeStatic.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InvokeStatic.java
@@ -177,7 +177,7 @@
target.getOptimizationInfo().mayHaveSideEffects()
// Verify that calling the target method won't lead to class initialization.
|| target.method.holder.classInitializationMayHaveSideEffects(
- appView.appInfo(),
+ appView,
// Types that are a super type of `context` are guaranteed to be initialized
// already.
type -> appView.isSubtype(context, type).isTrue());
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
index d255669..4cf7120 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
@@ -153,7 +153,7 @@
//
// For simplicity, we are conservative and consider all interfaces, not only the ones with
// default methods.
- return !clazz.classInitializationMayHaveSideEffects(appView.appInfo());
+ return !clazz.classInitializationMayHaveSideEffects(appView);
}
private synchronized boolean isDoubleInliningTarget(DexEncodedMethod candidate) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java b/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
index 615b4c1..dd1064c 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
@@ -330,7 +330,7 @@
DexClass holderDefinition = appView.definitionFor(field.holder);
if (holderDefinition != null
&& holderDefinition.accessFlags.isFinal()
- && !field.holder.initializationOfParentTypesMayHaveSideEffects(appView.appInfo())) {
+ && !field.holder.initializationOfParentTypesMayHaveSideEffects(appView)) {
Value outValue = current.dest();
DexEncodedMethod classInitializer = holderDefinition.getClassInitializer();
if (classInitializer != null && !isProcessedConcurrently.test(classInitializer)) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java b/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java
index ec95f89..650d737 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java
@@ -493,7 +493,7 @@
if (field.field.holder != code.method.method.holder) {
DexClass enclosingClass = appView.definitionFor(code.method.method.holder);
if (enclosingClass == null
- || enclosingClass.classInitializationMayHaveSideEffects(appView.appInfo())) {
+ || enclosingClass.classInitializationMayHaveSideEffects(appView)) {
return;
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
index 5140cfd..1e85790 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
@@ -233,6 +233,6 @@
}
// Check for static initializers in this class or any of interfaces it implements.
- return !clazz.initializationOfParentTypesMayHaveSideEffects(appView.appInfo());
+ return !clazz.initializationOfParentTypesMayHaveSideEffects(appView);
}
}
diff --git a/src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java b/src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java
index 8b4bedb..b526695 100644
--- a/src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java
@@ -284,7 +284,7 @@
|| appView.appInfo().noSideEffects.keySet().contains(method.method))) {
return MergeGroup.DONT_MERGE;
}
- if (clazz.classInitializationMayHaveSideEffects(appView.appInfo())) {
+ if (clazz.classInitializationMayHaveSideEffects(appView)) {
// This could have a negative impact on inlining.
//
// See {@link com.android.tools.r8.ir.optimize.DefaultInliningOracle#canInlineStaticInvoke}
diff --git a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
index 8383f5c..085acee 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -429,8 +429,8 @@
// We basically can't move the clinit, since it is not called when implementing classes have
// their clinit called - except when the interface has a default method.
if ((clazz.hasClassInitializer() && targetClass.hasClassInitializer())
- || targetClass.classInitializationMayHaveSideEffects(appInfo, type -> type == clazz.type)
- || (clazz.isInterface() && clazz.classInitializationMayHaveSideEffects(appInfo))) {
+ || targetClass.classInitializationMayHaveSideEffects(appView, type -> type == clazz.type)
+ || (clazz.isInterface() && clazz.classInitializationMayHaveSideEffects(appView))) {
// TODO(herhut): Handle class initializers.
if (Log.ENABLED) {
AbortReason.STATIC_INITIALIZERS.printLogMessageForClass(clazz);
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index ef1fb01..4d0de15 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -141,6 +141,8 @@
// Flag to toggle if DEX code objects should pass-through without IR processing.
public boolean passthroughDexCode = false;
+ public boolean libraryInterfacesMayHaveStaticInitialization = false;
+
// Optimization-related flags. These should conform to -dontoptimize and disableAllOptimizations.
public boolean enableDynamicTypeOptimization = true;
public boolean enableHorizontalClassMerging = true;
diff --git a/src/test/java/com/android/tools/r8/classmerging/InliningAfterStaticClassMergerTest.java b/src/test/java/com/android/tools/r8/classmerging/InliningAfterStaticClassMergerTest.java
index fd5e09a..d59f954 100644
--- a/src/test/java/com/android/tools/r8/classmerging/InliningAfterStaticClassMergerTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/InliningAfterStaticClassMergerTest.java
@@ -78,11 +78,12 @@
CodeInspector inspector =
testForR8(parameters.getBackend())
.addProgramClasses(
- TestClass.class,
- StaticMergeCandidateA.class,
- StaticMergeCandidateB.class)
+ TestClass.class, StaticMergeCandidateA.class, StaticMergeCandidateB.class)
.addKeepMainRule(TestClass.class)
+ .addOptionsModification(
+ options -> options.libraryInterfacesMayHaveStaticInitialization = true)
.noMinification()
+ .setMinApi(parameters.getRuntime())
.run(parameters.getRuntime(), TestClass.class)
.assertSuccessWithOutput(expected)
.inspector();
diff --git a/src/test/java/com/android/tools/r8/ir/analysis/initializedclasses/LibraryClassInitializationTest.java b/src/test/java/com/android/tools/r8/ir/analysis/initializedclasses/LibraryClassInitializationTest.java
new file mode 100644
index 0000000..b2ddc66
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/analysis/initializedclasses/LibraryClassInitializationTest.java
@@ -0,0 +1,56 @@
+// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.ir.analysis.initializedclasses;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import java.io.Serializable;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class LibraryClassInitializationTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimes().build();
+ }
+
+ public LibraryClassInitializationTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ CodeInspector inspector =
+ testForR8(parameters.getBackend())
+ .addInnerClasses(LibraryClassInitializationTest.class)
+ .addKeepMainRule(TestClass.class)
+ .setMinApi(parameters.getRuntime())
+ .compile()
+ .inspector();
+ assertThat(inspector.clazz(ClassWithoutClassInitialization.class), not(isPresent()));
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ // The instantiation can only be removed if we assume that the class initialization of
+ // Serializable may not have side effects.
+ new ClassWithoutClassInitialization();
+ }
+ }
+
+ static class ClassWithoutClassInitialization implements Serializable {}
+}
diff --git a/src/test/java/com/android/tools/r8/naming/NonMemberClassTest.java b/src/test/java/com/android/tools/r8/naming/NonMemberClassTest.java
index 9ef1a4a..8bb936d 100644
--- a/src/test/java/com/android/tools/r8/naming/NonMemberClassTest.java
+++ b/src/test/java/com/android/tools/r8/naming/NonMemberClassTest.java
@@ -113,6 +113,8 @@
.addKeepAttributes("InnerClasses", "EnclosingMethod")
.enableInliningAnnotations()
.setMinApi(parameters.getRuntime())
+ .addOptionsModification(options -> options.enableClassInlining = false)
+ .compile()
.run(parameters.getRuntime(), MAIN)
.assertSuccessWithOutput(EXPECTED_OUTPUT)
.inspect(this::inspect);
