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.

Sign up for GitHub

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

Concat Transform Optimizations #149

Closed
wants to merge 8 commits into from
+124 −34
Closed

Concat Transform Optimizations #149

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f136df5
Updates reducing redundant StringBuilders
igraves Jun 18, 2024
e578123
Cleanup
igraves Jun 18, 2024
2cfe4de
Reducing checks for lookahead
igraves Jun 18, 2024
0ae8001
Updates to cover additional tests and types
igraves Jun 20, 2024
7d966a4
Integrating Paul's tree-based approach with argument handling
igraves Jun 26, 2024
b38af67
Renaming some tests and adding additional coverage for side-effects i…
igraves Jun 27, 2024
775c9ea
Updates and revisions
igraves Jun 28, 2024
dfec1f5
Refactoring append
igraves Jun 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 58 additions & 30 deletions src/java.base/share/classes/java/lang/reflect/code/analysis/StringConcatTransformer.java
View file
Original file line number Diff line number Diff line change
@@ -30,6 +30,8 @@
import java.lang.reflect.code.type.JavaType;
import java.lang.reflect.code.type.FunctionType;
import java.lang.reflect.code.type.MethodRef;
import java.lang.reflect.code.type.PrimitiveType;
import java.util.*;

/**
* StringConcatTransformer is an {@link java.lang.reflect.code.OpTransformer} that removes concatenation operations
@@ -38,48 +40,74 @@
*/
public class StringConcatTransformer implements OpTransformer {

private static final JavaType SBC_TYPE = JavaType.type(StringBuilder.class);
private static final MethodRef SB_TO_STRING = MethodRef.method(SBC_TYPE, "toString", JavaType.J_L_STRING);

record StringAndBuilder(Value string, Value stringBuilder) { }
private static final JavaType J_L_STRING_BUILDER = JavaType.type(StringBuilder.class);
private static final MethodRef SB_TO_STRING_REF = MethodRef.method(
J_L_STRING_BUILDER, "toString", JavaType.J_L_STRING);

public StringConcatTransformer() {}

@Override
public Block.Builder apply(Block.Builder builder, Op op) {
if (op instanceof CoreOp.ConcatOp cz) {
public Block.Builder apply(Block.Builder block, Op op) {
switch (op) {
case CoreOp.ConcatOp cz when isRootConcat(cz) -> {
// Create a string builder and build by traversing tree of operands
Op.Result builder = block.apply(CoreOp._new(FunctionType.functionType(J_L_STRING_BUILDER)));
buildFromTree(block, builder, cz);
// Convert to string
Value s = block.op(CoreOp.invoke(SB_TO_STRING_REF, builder));
block.context().mapValue(cz.result(), s);
}
case CoreOp.ConcatOp _ -> {
// Process later when building from root concat
}
default -> block.op(op);
}
return block;
}

Value left = builder.context().getValue(cz.operands().get(0));
Value right = builder.context().getValue(cz.operands().get(1));
static boolean isRootConcat(CoreOp.ConcatOp cz) {
// Root of concat tree, zero uses, two or more uses,
// or one use that is not a subsequent concat op
Set<Op.Result> uses = cz.result().uses();
return uses.size() != 1 || !(uses.iterator().next().op() instanceof CoreOp.ConcatOp);
}

Value result = cz.result();
static void buildFromTree(Block.Builder block, Op.Result builder, CoreOp.ConcatOp cz) {
// Process concat op's operands from left to right
buildFromTree(block, builder, cz.operands().get(0));
buildFromTree(block, builder, cz.operands().get(1));
}

StringAndBuilder newRes = stringBuild(builder, left, right);
builder.context().mapValue(result, newRes.string);
static void buildFromTree(Block.Builder block, Op.Result builder, Value v) {
if (v instanceof Op.Result r &&
r.op() instanceof CoreOp.ConcatOp cz &&
r.uses().size() == 1) {
// Node of tree, recursively traverse the operands
buildFromTree(block, builder, cz);
} else {
builder.op(op);
// Leaf of tree, append value to builder
// Note leaf can be the result of a ConcatOp with multiple uses
block.op(append(block, builder, block.context().getValue(v)));
}
return builder;
}

//Uses StringBuilder and Immediate String Value
private static StringAndBuilder stringBuild(Block.Builder builder, Value left, Value right) {
var newB = stringBuilder(builder, left, right);
var toStringInvoke = CoreOp.invoke(SB_TO_STRING, newB);
Value newString = builder.apply(toStringInvoke);
return new StringAndBuilder(newString, newB);
}
private static Op append(Block.Builder block, Value builder, Value arg) {
// Check if we need to widen unsupported integer types in the StringBuilder API
// Strings are fed in as-is, everything else given as an Object.
TypeElement type = arg.type();
if (type instanceof PrimitiveType) {
//Widen Short and Byte to Int.
if (type.equals(JavaType.BYTE) || type.equals(JavaType.SHORT)) {
arg = block.op(CoreOp.conv(JavaType.INT, arg));
type = JavaType.INT;
}
} else if (!type.equals(JavaType.J_L_STRING)){
type = JavaType.J_L_OBJECT;
}

private static Value stringBuilder(Block.Builder builder, Value left, Value right) {
CoreOp.NewOp newBuilder = CoreOp._new(FunctionType.functionType(SBC_TYPE));
Value sb = builder.apply(newBuilder);
builder.op(append(sb, left));
builder.op(append(sb, right));
return sb;
MethodRef methodDesc = MethodRef.method(J_L_STRING_BUILDER, "append", J_L_STRING_BUILDER, type);
return CoreOp.invoke(methodDesc, builder, arg);
}

private static Op append(Value stringBuilder, Value arg) {
MethodRef leftMethodDesc = MethodRef.method(SBC_TYPE, "append", SBC_TYPE, arg.type());
return CoreOp.invoke(leftMethodDesc, stringBuilder, arg);
}

}
70 changes: 66 additions & 4 deletions test/jdk/java/lang/reflect/code/TestStringConcatTransform.java
View file
Original file line number Diff line number Diff line change
@@ -42,7 +42,6 @@
import java.lang.runtime.CodeReflection;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

public class TestStringConcatTransform {
@@ -79,6 +78,7 @@ public String toString() {
});
valMap.put(TestObject.class, new TestObject());
valMap.put(String.class, TESTSTR);
valMap.put(StringBuilder.class, new StringBuilder("test"));
}

public static final class TestObject {
@@ -97,6 +97,7 @@ public void testModelTransform(@NoInjection Method method) {
CoreOp.FuncOp f_transformed = model.transform(new StringConcatTransformer());
Object[] args = prepArgs(method);

model.writeTo(System.out);
f_transformed.writeTo(System.out);

var interpreted = Interpreter.invoke(model, args);
@@ -108,11 +109,31 @@ public void testModelTransform(@NoInjection Method method) {

@Test(dataProvider = "getClassMethods")
public void testSSAModelTransform(@NoInjection Method method) {
Object[] args = prepArgs(method);
testStringConcat(method, args);
}

//Testing to make sure StringBuilders aren't caught up in the concat transformation
@Test
public void testStringBuilderUnchanged() {
Method method;

try {
method = TestStringConcatTransform.class.getMethod("stringBuilderArgCheck", String.class, String.class, StringBuilder.class);
} catch (NoSuchMethodException e) {
throw new RuntimeException(e);
}
Object[] args = {"Foo", "Bar", new StringBuilder("test")};
testStringConcat(method, args);

Assert.assertEquals("test", args[2].toString());
}

private void testStringConcat(Method method, Object[] args) {
CoreOp.FuncOp model = method.getCodeModel().orElseThrow();
CoreOp.FuncOp transformed_model = model.transform(new StringConcatTransformer());
CoreOp.FuncOp ssa_model = generateSSA(model);
CoreOp.FuncOp ssa_transformed_model = ssa_model.transform(new StringConcatTransformer());
Object[] args = prepArgs(method);

var model_interpreted = Interpreter.invoke(model, args);
var transformed_model_interpreted = Interpreter.invoke(transformed_model, args);
@@ -128,6 +149,7 @@ public void testSSAModelTransform(@NoInjection Method method) {
Assert.assertEquals(transformed_model_interpreted, ssa_interpreted);
Assert.assertEquals(ssa_interpreted, ssa_transformed_interpreted);
Assert.assertEquals(ssa_transformed_interpreted, jvm_interpreted);

}

public static Object[] prepArgs(Method m) {
@@ -193,10 +215,50 @@ public static String intConcatDoubVar(int i, String s) {
}

@CodeReflection
public static String intConcatNestedSplit(int i, String s){
public static String intConcatNestedSplit(int i, String s) {
String q, r;
String inter = i + (q = r = s + "hello") + 52;
return q + r + inter;
}

}
@CodeReflection
public static String nonLeftAssociativeTree(String a, String b, String c, String d) {
String s = (a + b) + (c + d);
return s;
}

@CodeReflection
public static String stringBuilderCheck(String a, String d) {
StringBuilder sb = new StringBuilder("test");
String s = sb + a;
String t = s + d;
return t;
}

@CodeReflection
public static String stringBuilderArgCheck(String a, String d, StringBuilder c) {
StringBuilder sb = c;
String s = sb + a;
String t = s + d;
return t;
}

@CodeReflection
public static String leftAssociativeTree(String a, String b, String c, String d) {
String s = ((a + b) + c) + d;
return s;
}

@CodeReflection
public static String rightAssociativeTree(String a, String b, String c, String d) {
String s = (a + (b + (c + d)));
return s;
}

@CodeReflection
public static String widenPrimitives(short a, byte b, int c, int d) {
String s = (a + (b + (c + d + "hi")));
return s;
}
}