Extend unused StringBuilder removal to Objects.toString() and String.valueOf()
Bug: 174285670
Change-Id: Ia7d6df07abd04fdfaa5e23f44d19574dbed36280
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/library/ObjectsMethodOptimizer.java b/src/main/java/com/android/tools/r8/ir/optimize/library/ObjectsMethodOptimizer.java
index 9d0c8c8..5fc8d62 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/library/ObjectsMethodOptimizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/library/ObjectsMethodOptimizer.java
@@ -13,6 +13,7 @@
import com.android.tools.r8.ir.code.InstructionListIterator;
import com.android.tools.r8.ir.code.InvokeMethod;
import com.android.tools.r8.ir.code.Value;
+import com.android.tools.r8.utils.ValueUtils;
import java.util.Set;
public class ObjectsMethodOptimizer extends StatelessLibraryMethodModelCollection {
@@ -66,12 +67,21 @@
InstructionListIterator instructionIterator,
InvokeMethod invoke,
Set<Value> affectedValues) {
+ // Optimize Objects.toString(null) into "null".
Value object = invoke.getFirstArgument();
if (object.getType().isDefinitelyNull()) {
instructionIterator.replaceCurrentInstructionWithConstString(appView, code, "null");
if (invoke.hasOutValue()) {
affectedValues.addAll(invoke.outValue().affectedValues());
}
+ return;
+ }
+
+ // Remove Objects.toString(stringBuilder) if unused.
+ if (ValueUtils.isStringBuilder(invoke.getFirstArgument(), dexItemFactory)) {
+ if (!invoke.hasOutValue() || !invoke.outValue().hasNonDebugUsers()) {
+ instructionIterator.removeOrReplaceByDebugLocalRead();
+ }
}
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/library/StringBuilderMethodOptimizer.java b/src/main/java/com/android/tools/r8/ir/optimize/library/StringBuilderMethodOptimizer.java
index 9cccb5d..99b1bda 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/library/StringBuilderMethodOptimizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/library/StringBuilderMethodOptimizer.java
@@ -4,7 +4,10 @@
package com.android.tools.r8.ir.optimize.library;
+import static com.android.tools.r8.ir.code.Opcodes.ASSUME;
+import static com.android.tools.r8.ir.code.Opcodes.IF;
import static com.android.tools.r8.ir.code.Opcodes.INVOKE_DIRECT;
+import static com.android.tools.r8.ir.code.Opcodes.INVOKE_STATIC;
import static com.android.tools.r8.ir.code.Opcodes.INVOKE_VIRTUAL;
import static com.android.tools.r8.ir.code.Opcodes.NEW_INSTANCE;
@@ -29,6 +32,7 @@
import com.android.tools.r8.ir.optimize.UtilityMethodsForCodeOptimizations.UtilityMethodForCodeOptimizations;
import com.android.tools.r8.ir.optimize.library.StringBuilderMethodOptimizer.State;
import com.android.tools.r8.utils.InternalOptions;
+import com.android.tools.r8.utils.ValueUtils;
import com.android.tools.r8.utils.WorkList;
import com.google.common.collect.Sets;
import it.unimi.dsi.fastutil.objects.Reference2BooleanMap;
@@ -73,6 +77,8 @@
InvokeMethodWithReceiver invokeWithReceiver = invoke.asInvokeMethodWithReceiver();
if (stringBuilderMethods.isAppendMethod(singleTarget.getReference())) {
optimizeAppend(code, instructionIterator, invokeWithReceiver, singleTarget, state);
+ } else if (singleTarget.getReference() == dexItemFactory.stringBuilderMethods.toString) {
+ optimizeToString(instructionIterator, invokeWithReceiver);
}
}
}
@@ -130,6 +136,16 @@
}
}
+ private void optimizeToString(
+ InstructionListIterator instructionIterator, InvokeMethodWithReceiver invoke) {
+ // Optimize StringBuilder.toString() if unused.
+ if (ValueUtils.isNonNullStringBuilder(invoke.getReceiver(), dexItemFactory)) {
+ if (!invoke.hasOutValue() || !invoke.outValue().hasNonDebugUsers()) {
+ instructionIterator.removeOrReplaceByDebugLocalRead();
+ }
+ }
+ }
+
class State implements LibraryMethodModelCollection.State {
final MethodProcessor methodProcessor;
@@ -185,6 +201,10 @@
Instruction definition = alias.definition;
switch (definition.opcode()) {
+ case ASSUME:
+ worklist.addIfNotSeen(definition.inValues());
+ break;
+
case NEW_INSTANCE:
assert definition.asNewInstance().clazz == dexItemFactory.stringBuilderType;
break;
@@ -208,6 +228,14 @@
// Analyze all users.
for (Instruction user : alias.uniqueUsers()) {
switch (user.opcode()) {
+ case ASSUME:
+ worklist.addIfNotSeen(user.outValue());
+ break;
+
+ case IF:
+ // StringBuilder null check.
+ break;
+
case INVOKE_DIRECT:
{
InvokeDirect invoke = user.asInvokeDirect();
@@ -224,6 +252,25 @@
}
break;
+ case INVOKE_STATIC:
+ {
+ InvokeStatic invoke = user.asInvokeStatic();
+ DexMethod invokedMethod = invoke.getInvokedMethod();
+
+ // Allow calls to Objects.toString(Object) and String.valueOf(Object).
+ if (invokedMethod == dexItemFactory.objectsMethods.toStringWithObject
+ || invokedMethod == dexItemFactory.stringMembers.valueOf) {
+ // Only allow unused StringBuilders.
+ if (invoke.hasOutValue() && invoke.outValue().hasNonDebugUsers()) {
+ return false;
+ }
+ break;
+ }
+
+ // Invoke to unhandled method, give up.
+ return false;
+ }
+
case INVOKE_VIRTUAL:
{
InvokeVirtual invoke = user.asInvokeVirtual();
@@ -245,7 +292,8 @@
}
// Allow calls to toString().
- if (invokedMethod == stringBuilderMethods.toString) {
+ if (invokedMethod == dexItemFactory.objectMembers.toString
+ || invokedMethod == stringBuilderMethods.toString) {
// Only allow unused StringBuilders.
if (invoke.hasOutValue() && invoke.outValue().hasNonDebugUsers()) {
return false;
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/library/StringMethodOptimizer.java b/src/main/java/com/android/tools/r8/ir/optimize/library/StringMethodOptimizer.java
index c8d04ae..a8fabe3 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/library/StringMethodOptimizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/library/StringMethodOptimizer.java
@@ -7,6 +7,7 @@
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexClassAndMethod;
import com.android.tools.r8.graph.DexItemFactory;
+import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexReference;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.ProgramMethod;
@@ -15,7 +16,10 @@
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.InstructionListIterator;
import com.android.tools.r8.ir.code.InvokeMethod;
+import com.android.tools.r8.ir.code.InvokeStatic;
+import com.android.tools.r8.ir.code.InvokeVirtual;
import com.android.tools.r8.ir.code.Value;
+import com.android.tools.r8.utils.ValueUtils;
import java.util.Set;
public class StringMethodOptimizer extends StatelessLibraryMethodModelCollection {
@@ -40,17 +44,20 @@
InvokeMethod invoke,
DexClassAndMethod singleTarget,
Set<Value> affectedValues) {
- if (singleTarget.getReference() == dexItemFactory.stringMembers.equals) {
- optimizeEquals(code, instructionIterator, invoke);
+ DexMethod singleTargetReference = singleTarget.getReference();
+ if (singleTargetReference == dexItemFactory.stringMembers.equals) {
+ optimizeEquals(code, instructionIterator, invoke.asInvokeVirtual());
+ } else if (singleTargetReference == dexItemFactory.stringMembers.valueOf) {
+ optimizeValueOf(instructionIterator, invoke.asInvokeStatic());
}
}
private void optimizeEquals(
- IRCode code, InstructionListIterator instructionIterator, InvokeMethod invoke) {
+ IRCode code, InstructionListIterator instructionIterator, InvokeVirtual invoke) {
if (appView.appInfo().hasLiveness()) {
ProgramMethod context = code.context();
- Value first = invoke.arguments().get(0).getAliasedValue();
- Value second = invoke.arguments().get(1).getAliasedValue();
+ Value first = invoke.getReceiver().getAliasedValue();
+ Value second = invoke.getArgument(1).getAliasedValue();
if (isPrunedClassNameComparison(first, second, context)
|| isPrunedClassNameComparison(second, first, context)) {
instructionIterator.replaceCurrentInstructionWithConstInt(code, 0);
@@ -58,6 +65,15 @@
}
}
+ private void optimizeValueOf(InstructionListIterator instructionIterator, InvokeStatic invoke) {
+ // Optimize String.valueOf(stringBuilder) if unused.
+ if (ValueUtils.isNonNullStringBuilder(invoke.getFirstArgument(), dexItemFactory)) {
+ if (!invoke.hasOutValue() || !invoke.outValue().hasNonDebugUsers()) {
+ instructionIterator.removeOrReplaceByDebugLocalRead();
+ }
+ }
+ }
+
/**
* Returns true if {@param classNameValue} is defined by calling {@link Class#getName()} and
* {@param constStringValue} is a constant string that is identical to the name of a class that
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderOptimizer.java b/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderOptimizer.java
index a61662c..4dc7926 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderOptimizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderOptimizer.java
@@ -852,7 +852,8 @@
@Override
public boolean isToStringMethod(DexMethod method) {
- return method == factory.stringBuilderMethods.toString
+ return method == factory.objectMembers.toString
+ || method == factory.stringBuilderMethods.toString
|| method == factory.stringBufferMethods.toString
|| method == factory.stringMembers.valueOf;
}
diff --git a/src/main/java/com/android/tools/r8/utils/ValueUtils.java b/src/main/java/com/android/tools/r8/utils/ValueUtils.java
new file mode 100644
index 0000000..8121493
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/utils/ValueUtils.java
@@ -0,0 +1,46 @@
+// Copyright (c) 2021, 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.utils;
+
+import com.android.tools.r8.graph.DexItemFactory;
+import com.android.tools.r8.ir.analysis.type.TypeElement;
+import com.android.tools.r8.ir.code.Instruction;
+import com.android.tools.r8.ir.code.InvokeVirtual;
+import com.android.tools.r8.ir.code.NewInstance;
+import com.android.tools.r8.ir.code.Value;
+
+public class ValueUtils {
+
+ public static boolean isStringBuilder(Value value, DexItemFactory dexItemFactory) {
+ TypeElement type = value.getType();
+ return type.isClassType()
+ && type.asClassType().getClassType() == dexItemFactory.stringBuilderType;
+ }
+
+ public static boolean isNonNullStringBuilder(Value value, DexItemFactory dexItemFactory) {
+ while (true) {
+ if (value.isPhi()) {
+ return false;
+ }
+
+ Instruction definition = value.getDefinition();
+ if (definition.isNewInstance()) {
+ NewInstance newInstance = definition.asNewInstance();
+ return newInstance.clazz == dexItemFactory.stringBuilderType;
+ }
+
+ if (definition.isInvokeVirtual()) {
+ InvokeVirtual invoke = definition.asInvokeVirtual();
+ if (dexItemFactory.stringBuilderMethods.isAppendMethod(invoke.getInvokedMethod())) {
+ value = invoke.getReceiver();
+ continue;
+ }
+ }
+
+ // Unhandled definition.
+ return false;
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/string/UnusedStringBuilderWithAppendObjectTest.java b/src/test/java/com/android/tools/r8/ir/optimize/string/UnusedStringBuilderWithAppendObjectTest.java
index d8ed11c..b8c0799 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/string/UnusedStringBuilderWithAppendObjectTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/string/UnusedStringBuilderWithAppendObjectTest.java
@@ -5,13 +5,14 @@
package com.android.tools.r8.ir.optimize.string;
import static com.android.tools.r8.utils.codeinspector.CodeMatchers.instantiatesClass;
-import static com.android.tools.r8.utils.codeinspector.Matchers.notIf;
+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.MethodSubject;
+import java.util.Objects;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -40,11 +41,7 @@
.inspect(
inspector -> {
MethodSubject mainMethod = inspector.clazz(Main.class).mainMethod();
- assertThat(
- mainMethod,
- notIf(
- instantiatesClass(StringBuilder.class),
- canUseJavaUtilObjects(parameters) || parameters.isDexRuntime()));
+ assertThat(mainMethod, not(instantiatesClass(StringBuilder.class)));
})
.run(parameters.getRuntime(), Main.class)
.assertSuccessWithEmptyOutput();
@@ -55,6 +52,8 @@
public static void main(String[] args) {
A a = System.currentTimeMillis() > 0 ? new A() : null;
new StringBuilder().append(a).toString();
+ Objects.toString(new StringBuilder().append(a));
+ String.valueOf(new StringBuilder().append(a));
}
}
