Inlining heuristics for optimizing proto builders
This CL introduces a new option options.protoShrinking().generatedMessageLiteBuilderShrinking, and adds additional items into the alwaysInline and neverInline sets if the option is set.
Change-Id: Ibe11b2d314fc6ca6789af68280166a9a33f66be9
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/proto/GeneratedMessageLiteBuilderShrinker.java b/src/main/java/com/android/tools/r8/ir/analysis/proto/GeneratedMessageLiteBuilderShrinker.java
new file mode 100644
index 0000000..b60e3f1
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/ir/analysis/proto/GeneratedMessageLiteBuilderShrinker.java
@@ -0,0 +1,86 @@
+// 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.proto;
+
+import com.android.tools.r8.graph.AppInfoWithSubtyping;
+import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.DexItemFactory;
+import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.graph.DexType;
+import java.util.Set;
+
+public class GeneratedMessageLiteBuilderShrinker {
+
+ public static void addInliningHeuristicsForBuilderInlining(
+ AppView<? extends AppInfoWithSubtyping> appView,
+ Set<DexMethod> alwaysInline,
+ Set<DexMethod> neverInline) {
+ new RootSetExtension(appView, alwaysInline, neverInline).extend();
+ }
+
+ private static class RootSetExtension {
+
+ private final AppView<? extends AppInfoWithSubtyping> appView;
+ private final ProtoReferences references;
+
+ private final Set<DexMethod> alwaysInline;
+ private final Set<DexMethod> neverInline;
+
+ RootSetExtension(
+ AppView<? extends AppInfoWithSubtyping> appView,
+ Set<DexMethod> alwaysInline,
+ Set<DexMethod> neverInline) {
+ this.appView = appView;
+ this.references = appView.protoShrinker().references;
+ this.alwaysInline = alwaysInline;
+ this.neverInline = neverInline;
+ }
+
+ void extend() {
+ // GeneratedMessageLite heuristics.
+ alwaysInlineCreateBuilderFromGeneratedMessageLite();
+ neverInlineIsInitializedFromGeneratedMessageLite();
+
+ // * extends GeneratedMessageLite heuristics.
+ alwaysInlineDynamicMethodFromGeneratedMessageLiteImplementations();
+
+ // GeneratedMessageLite$Builder heuristics.
+ alwaysInlineBuildPartialFromGeneratedMessageLiteBuilder();
+ }
+
+ private void alwaysInlineBuildPartialFromGeneratedMessageLiteBuilder() {
+ alwaysInline.add(references.generatedMessageLiteBuilderMethods.buildPartialMethod);
+ }
+
+ private void alwaysInlineCreateBuilderFromGeneratedMessageLite() {
+ alwaysInline.add(references.generatedMessageLiteMethods.createBuilderMethod);
+ }
+
+ private void alwaysInlineDynamicMethodFromGeneratedMessageLiteImplementations() {
+ // TODO(b/132600418): We should be able to determine that dynamicMethod() becomes 'SIMPLE'
+ // when the MethodToInvoke argument is MethodToInvoke.NEW_BUILDER.
+ DexItemFactory dexItemFactory = appView.dexItemFactory();
+ for (DexType type : appView.appInfo().subtypes(references.generatedMessageLiteType)) {
+ alwaysInline.add(
+ dexItemFactory.createMethod(
+ type,
+ dexItemFactory.createProto(
+ dexItemFactory.objectType,
+ references.methodToInvokeType,
+ dexItemFactory.objectType,
+ dexItemFactory.objectType),
+ "dynamicMethod"));
+ }
+ }
+
+ /**
+ * Without this rule, GeneratedMessageLite$Builder.build() becomes too big for class inlining.
+ * TODO(b/112437944): Maybe introduce a -neverinlineinto rule instead?
+ */
+ private void neverInlineIsInitializedFromGeneratedMessageLite() {
+ neverInline.add(references.generatedMessageLiteMethods.isInitializedMethod);
+ }
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/proto/ProtoReferences.java b/src/main/java/com/android/tools/r8/ir/analysis/proto/ProtoReferences.java
index 0919db9..4b1cb0e 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/proto/ProtoReferences.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/proto/ProtoReferences.java
@@ -17,10 +17,14 @@
public final DexType extensionRegistryLiteType;
public final DexType generatedExtensionType;
public final DexType generatedMessageLiteType;
+ public final DexType generatedMessageLiteBuilderType;
public final DexType rawMessageInfoType;
public final DexType messageLiteType;
public final DexType methodToInvokeType;
+ public final GeneratedMessageLiteMethods generatedMessageLiteMethods;
+ public final GeneratedMessageLiteBuilderMethods generatedMessageLiteBuilderMethods;
+
public final DexString dynamicMethodName;
public final DexString findLiteExtensionByNumberName;
@@ -42,6 +46,9 @@
factory.createString("Lcom/google/protobuf/GeneratedMessageLite$GeneratedExtension;"));
generatedMessageLiteType =
factory.createType(factory.createString("Lcom/google/protobuf/GeneratedMessageLite;"));
+ generatedMessageLiteBuilderType =
+ factory.createType(
+ factory.createString("Lcom/google/protobuf/GeneratedMessageLite$Builder;"));
rawMessageInfoType =
factory.createType(factory.createString("Lcom/google/protobuf/RawMessageInfo;"));
messageLiteType = factory.createType(factory.createString("Lcom/google/protobuf/MessageLite;"));
@@ -73,6 +80,9 @@
factory.createProto(
factory.voidType, messageLiteType, factory.stringType, factory.objectArrayType),
factory.constructorMethodName);
+
+ generatedMessageLiteMethods = new GeneratedMessageLiteMethods(factory);
+ generatedMessageLiteBuilderMethods = new GeneratedMessageLiteBuilderMethods(factory);
}
public boolean isDynamicMethod(DexMethod method) {
@@ -91,4 +101,36 @@
public boolean isMessageInfoConstructionMethod(DexMethod method) {
return method.match(newMessageInfoMethod) || method == rawMessageInfoConstructor;
}
+
+ class GeneratedMessageLiteMethods {
+
+ public final DexMethod createBuilderMethod;
+ public final DexMethod isInitializedMethod;
+
+ private GeneratedMessageLiteMethods(DexItemFactory dexItemFactory) {
+ createBuilderMethod =
+ dexItemFactory.createMethod(
+ generatedMessageLiteType,
+ dexItemFactory.createProto(generatedMessageLiteBuilderType),
+ "createBuilder");
+ isInitializedMethod =
+ dexItemFactory.createMethod(
+ generatedMessageLiteType,
+ dexItemFactory.createProto(dexItemFactory.booleanType),
+ "isInitialized");
+ }
+ }
+
+ class GeneratedMessageLiteBuilderMethods {
+
+ public final DexMethod buildPartialMethod;
+
+ private GeneratedMessageLiteBuilderMethods(DexItemFactory dexItemFactory) {
+ buildPartialMethod =
+ dexItemFactory.createMethod(
+ generatedMessageLiteBuilderType,
+ dexItemFactory.createProto(generatedMessageLiteType),
+ "buildPartial");
+ }
+ }
}
diff --git a/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java b/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
index 55b880b..4cacde6 100644
--- a/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
+++ b/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
@@ -25,6 +25,7 @@
import com.android.tools.r8.graph.DexReference;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.DirectMappedDexApplication;
+import com.android.tools.r8.ir.analysis.proto.GeneratedMessageLiteBuilderShrinker;
import com.android.tools.r8.logging.Log;
import com.android.tools.r8.utils.Consumer3;
import com.android.tools.r8.utils.InternalOptions;
@@ -280,6 +281,10 @@
BottomUpClassHierarchyTraversal.forAllClasses(appView)
.visit(appView.appInfo().classes(), this::propagateAssumeRules);
}
+ if (appView.options().protoShrinking().enableGeneratedMessageLiteBuilderShrinking) {
+ GeneratedMessageLiteBuilderShrinker.addInliningHeuristicsForBuilderInlining(
+ appView, alwaysInline, neverInline);
+ }
assert Sets.intersection(neverInline, alwaysInline).isEmpty()
&& Sets.intersection(neverInline, forceInline).isEmpty()
: "A method cannot be marked as both -neverinline and -forceinline/-alwaysinline.";
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 27fbefe..8faa274 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -933,11 +933,16 @@
public boolean enableGeneratedMessageLiteShrinking =
System.getProperty("com.android.tools.r8.generatedMessageLiteShrinking") != null;
+ public boolean enableGeneratedMessageLiteBuilderShrinking =
+ System.getProperty("com.android.tools.r8.generatedMessageLiteBuilderShrinking") != null;
+
public boolean traverseOneOfAndRepeatedProtoFields =
System.getProperty("com.android.tools.r8.traverseOneOfAndRepeatedProtoFields") == null;
public boolean isProtoShrinkingEnabled() {
- return enableGeneratedExtensionRegistryShrinking || enableGeneratedMessageLiteShrinking;
+ return enableGeneratedExtensionRegistryShrinking
+ || enableGeneratedMessageLiteShrinking
+ || enableGeneratedMessageLiteBuilderShrinking;
}
}
diff --git a/src/test/java/com/android/tools/r8/internal/proto/Proto2BuilderShrinkingTest.java b/src/test/java/com/android/tools/r8/internal/proto/Proto2BuilderShrinkingTest.java
index 07ccabd..ff0d4bd 100644
--- a/src/test/java/com/android/tools/r8/internal/proto/Proto2BuilderShrinkingTest.java
+++ b/src/test/java/com/android/tools/r8/internal/proto/Proto2BuilderShrinkingTest.java
@@ -35,7 +35,8 @@
@Parameterized.Parameters(name = "{1}, enable minification: {0}")
public static List<Object[]> data() {
- return buildParameters(BooleanUtils.values(), getTestParameters().withAllRuntimes().build());
+ return buildParameters(
+ BooleanUtils.values(), getTestParameters().withAllRuntimesAndApiLevels().build());
}
public Proto2BuilderShrinkingTest(boolean enableMinification, TestParameters parameters) {
@@ -54,15 +55,16 @@
.addOptionsModification(
options -> {
options.enableFieldBitAccessAnalysis = true;
- options.protoShrinking().enableGeneratedMessageLiteShrinking = true;
options.protoShrinking().enableGeneratedExtensionRegistryShrinking = true;
+ options.protoShrinking().enableGeneratedMessageLiteShrinking = true;
+ options.protoShrinking().enableGeneratedMessageLiteBuilderShrinking = true;
options.enableStringSwitchConversion = true;
})
.allowAccessModification()
.allowUnusedProguardConfigurationRules()
.enableInliningAnnotations()
.minification(enableMinification)
- .setMinApi(parameters.getRuntime())
+ .setMinApi(parameters.getApiLevel())
.compile()
.inspect(this::inspect)
.run(parameters.getRuntime(), TEST_CLASS)
diff --git a/src/test/java/com/android/tools/r8/internal/proto/Proto2ShrinkingTest.java b/src/test/java/com/android/tools/r8/internal/proto/Proto2ShrinkingTest.java
index 2071b82..aed2c26 100644
--- a/src/test/java/com/android/tools/r8/internal/proto/Proto2ShrinkingTest.java
+++ b/src/test/java/com/android/tools/r8/internal/proto/Proto2ShrinkingTest.java
@@ -83,8 +83,9 @@
.addOptionsModification(
options -> {
options.enableFieldBitAccessAnalysis = true;
- options.protoShrinking().enableGeneratedMessageLiteShrinking = true;
options.protoShrinking().enableGeneratedExtensionRegistryShrinking = true;
+ options.protoShrinking().enableGeneratedMessageLiteShrinking = true;
+ options.protoShrinking().enableGeneratedMessageLiteBuilderShrinking = true;
options.enableStringSwitchConversion = true;
})
.allowAccessModification(allowAccessModification)
diff --git a/src/test/java/com/android/tools/r8/internal/proto/Proto3ShrinkingTest.java b/src/test/java/com/android/tools/r8/internal/proto/Proto3ShrinkingTest.java
index 7a67427..faa91e5 100644
--- a/src/test/java/com/android/tools/r8/internal/proto/Proto3ShrinkingTest.java
+++ b/src/test/java/com/android/tools/r8/internal/proto/Proto3ShrinkingTest.java
@@ -57,8 +57,10 @@
.addKeepRuleFiles(PROTOBUF_LITE_PROGUARD_RULES)
.addOptionsModification(
options -> {
- options.protoShrinking().enableGeneratedMessageLiteShrinking = true;
+ options.enableFieldBitAccessAnalysis = true;
options.protoShrinking().enableGeneratedExtensionRegistryShrinking = true;
+ options.protoShrinking().enableGeneratedMessageLiteShrinking = true;
+ options.protoShrinking().enableGeneratedMessageLiteBuilderShrinking = true;
options.enableStringSwitchConversion = true;
})
.allowAccessModification(allowAccessModification)
diff --git a/tools/run_on_app.py b/tools/run_on_app.py
index add4b77..0941623 100755
--- a/tools/run_on_app.py
+++ b/tools/run_on_app.py
@@ -512,6 +512,7 @@
extra_args.append('-Dcom.android.tools.r8.fieldBitAccessAnalysis=1')
extra_args.append('-Dcom.android.tools.r8.generatedExtensionRegistryShrinking=1')
extra_args.append('-Dcom.android.tools.r8.generatedMessageLiteShrinking=1')
+ extra_args.append('-Dcom.android.tools.r8.generatedMessageLiteBuilderShrinking=1')
extra_args.append('-Dcom.android.tools.r8.stringSwitchConversion=1')
extra_args.append('-Dcom.android.tools.r8.traverseOneOfAndRepeatedProtoFields=0')
