Handle the StringConcatFactory invocation with 200+ arguments

#1164
eclipse-jdt · Jul 3, 2023 · 04ce81f · 04ce81f
1 parent 32961cf
commit 04ce81f
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 46 deletions.
diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ClassFile.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ClassFile.java
@@ -3659,8 +3659,8 @@ private int generateBootstrapMethods(List<Object> bootStrapMethodsList) {
  } else {
  localContentsOffset = addBootStrapTypeSwitchEntry(localContentsOffset, stmt, fPtr);
  }
- } else if (o instanceof StringBuilder) {
- localContentsOffset = addBootStrapStringConcatEntry(localContentsOffset, (StringBuilder) o, fPtr);
+ } else if (o instanceof String) {
+ localContentsOffset = addBootStrapStringConcatEntry(localContentsOffset, (String) o, fPtr);
  }
  }
 
@@ -3934,7 +3934,7 @@ private int addBootStrapEnumSwitchEntry(int localContentsOffset, SwitchStatement
 
  return localContentsOffset;
  }
- private int addBootStrapStringConcatEntry(int localContentsOffset, StringBuilder recipe, Map<String, Integer> fPtr) {
+ private int addBootStrapStringConcatEntry(int localContentsOffset, String recipe, Map<String, Integer> fPtr) {
  final int contentsEntries = 10;
  int indexForStringConcat = fPtr.get(ClassFile.CONCAT_CONSTANTS);
  if (contentsEntries + localContentsOffset >= this.contents.length) {
@@ -3954,7 +3954,7 @@ private int addBootStrapStringConcatEntry(int localContentsOffset, StringBuilder
  this.contents[localContentsOffset++] = (byte) 1;
 
  int intValIdx =
- this.constantPool.literalIndex(recipe.toString());
+ this.constantPool.literalIndex(recipe);
  this.contents[localContentsOffset++] = (byte) (intValIdx >> 8);
  this.contents[localContentsOffset++] = (byte) intValIdx;
 
@@ -6262,7 +6262,7 @@ public int recordBootstrapMethod(SwitchStatement switchStatement) {
  this.bootstrapMethods.add(switchStatement);
  return this.bootstrapMethods.size() - 1;
  }
- public int recordBootstrapMethod(StringBuilder expression) {
+ public int recordBootstrapMethod(String expression) {
  if (this.bootstrapMethods == null) {
  this.bootstrapMethods = new ArrayList<>();
  }

diff --git a/...eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/Expression.java b/...eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/Expression.java
@@ -931,6 +931,21 @@ public void generateOptimizedStringConcatenationCreation(BlockScope blockScope,
  }
  codeStream.invokeStringConcatenationStringConstructor();
 }
+private void addArgumentToRecipe(CodeStream codeStream, StringBuilder recipe, TypeBinding argType, List<TypeBinding> args) {
+ recipe.append(STRING_CONCAT_MARKER_1);
+ args.add(argType);
+ // Any number below 200 will do. But 100 is safer since
+ // further nested concatenations can push the stack further
+ if (args.size() > 100) {
+ // Commit whatever we have accumulated so far
+ // Get the result pushed to the stack and to be used as an operand
+ // for the subsequent concat operation
+ codeStream.invokeDynamicForStringConcat(recipe, args);
+ // Clear the arguments for the next batch
+ args.clear();
+ recipe.delete(0, recipe.length());
+ }
+}
 public void buildStringForConcatation(BlockScope blockScope, CodeStream codeStream, int typeID, StringBuilder recipe, List<TypeBinding> argTypes) {
  if (this.constant == Constant.NotAConstant) {
  switch (typeID) {
@@ -944,8 +959,7 @@ public void buildStringForConcatation(BlockScope blockScope, CodeStream codeStre
  case TypeIds.T_char :
  case TypeIds.T_boolean :
  generateCode(blockScope, codeStream, true);
- argTypes.add(this.resolvedType);
- recipe.append(STRING_CONCAT_MARKER_1);
+ addArgumentToRecipe(codeStream, recipe, this.resolvedType, argTypes);
  break;
  default :
  if (this.resolvedType.id == TypeIds.T_null) {
@@ -954,17 +968,15 @@ public void buildStringForConcatation(BlockScope blockScope, CodeStream codeStre
  } else {
  generateCode(blockScope, codeStream, true);
  codeStream.invokeStringValueOf(typeID);
- argTypes.add(blockScope.getJavaLangString());
- recipe.append(STRING_CONCAT_MARKER_1);
+ addArgumentToRecipe(codeStream, recipe, blockScope.getJavaLangString(), argTypes);
  }
  break;
  }
  } else {
  // StringLiteral and CharLiteral may contain special characters
  if (this.constant.stringValue().indexOf('\u0001') != -1 || this.constant.stringValue().indexOf('\u0002') != -1) {
  codeStream.ldc(this.constant.stringValue());
- recipe.append(STRING_CONCAT_MARKER_1);
- argTypes.add(blockScope.getJavaLangString());
+ addArgumentToRecipe(codeStream, recipe, blockScope.getJavaLangString(), argTypes);
  } else {
  recipe.append(this.constant.stringValue());
  }

diff --git a/...pse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/codegen/CodeStream.java b/...pse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/codegen/CodeStream.java
@@ -2606,7 +2606,21 @@ public void generateReturnBytecode(Expression expression) {
  }
  }
 }
-
+public void invokeDynamicForStringConcat(StringBuilder recipe, List<TypeBinding> arguments) {
+ int invokeDynamicNumber = this.classFile.recordBootstrapMethod(recipe.toString());
+ StringBuilder signature = new StringBuilder("("); //$NON-NLS-1$
+ for(int i = 0; i < arguments.size(); i++) {
+ signature.append(arguments.get(i).signature());
+ }
+ signature.append(")Ljava/lang/String;"); //$NON-NLS-1$
+ this.invokeDynamic(invokeDynamicNumber,
+ 2,
+ 1, // int
+ ConstantPool.ConcatWithConstants,
+ signature.toString().toCharArray(),
+ TypeIds.T_JavaLangObject,
+ getPopularBinding(ConstantPool.JavaLangStringConstantPoolName));
+}
 /**
  * The equivalent code performs a string conversion:
  *
@@ -2632,19 +2646,7 @@ public void generateStringConcatenationAppend(BlockScope blockScope, Expression
  oper1.buildStringForConcatation(blockScope, this, oper1.implicitConversion & TypeIds.COMPILE_TYPE_MASK, recipe, arguments);
  }
  oper2.buildStringForConcatation(blockScope, this, oper2.implicitConversion & TypeIds.COMPILE_TYPE_MASK, recipe, arguments);
- int invokeDynamicNumber = this.classFile.recordBootstrapMethod(recipe);
- StringBuilder signature = new StringBuilder("("); //$NON-NLS-1$
- for(int i = 0; i < arguments.size(); i++) {
- signature.append(arguments.get(i).signature());
- }
- signature.append(")Ljava/lang/String;"); //$NON-NLS-1$
- this.invokeDynamic(invokeDynamicNumber,
- 2,
- 1, // int
- ConstantPool.ConcatWithConstants,
- signature.toString().toCharArray(),
- TypeIds.T_JavaLangObject,
- getPopularBinding(ConstantPool.JavaLangStringConstantPoolName));
+ invokeDynamicForStringConcat(recipe, arguments);
  } else {
  int pc;
  if (oper1 == null) {

diff --git a/...dt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/XLargeTest.java b/...dt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/XLargeTest.java
@@ -3736,7 +3736,9 @@ public void test010() {
 // need to use a computed string (else this source file will get blown away
 // as well)
 public void test011() {
- int length = 3 * 54 * 500;
+ if (this.complianceLevel >= ClassFileConstants.JDK9)
+ return;
+ int length = 3 * 54 * 1000;
  // the longer the slower, but still needs to reach the limit...
  StringBuilder veryLongString = new StringBuilder(length + 20);
  veryLongString.append('"');
@@ -3746,20 +3748,6 @@ public void test011() {
  veryLongString.append(random.nextLong());
  }
  veryLongString.append('"');
- String expectedError = this.complianceLevel >= ClassFileConstants.JDK9 ?
- "----------\n" +
- "1. ERROR in X.java (at line 1)\n" +
- " public class X {\n" +
- " ^\n" +
- "The type generates a string that requires more than 65535 bytes to encode in Utf8 format in the constant pool\n" +
- "----------\n" :
- "----------\n" +
- "1. ERROR in X.java (at line 2)\n" +
- " void foo(String a, String b, String c, String d, String e) {\n" +
- " ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" +
- "The code of method foo(String, String, String, String, String) is " +
- "exceeding the 65535 bytes limit\n" +
- "----------\n";
  this.runNegativeTest(
  new String[] {
  "X.java",
@@ -3768,14 +3756,16 @@ public void test011() {
  " String s = \n" +
  veryLongString.toString() +
  " + \"abcdef\" + a + b + c + d + e + \" ghiABCDEFGHIJKLMNOPQRSTUVWXYZjklmnopqrstuvwxyzabcdefghiABCDEFGHIJKLMNOPQRSTUVWXYZjklmnopqrstuvwxyzabcdefghiABCDEFGHIJKLMNOPQRSTUVWXYZjklmnopqrstuvwxyzabcdefghiABCDEFGHIJKLMNOPQRSTUVWXYZjklmnopqrstuvwxy12\";\n" +
- " for(int i = 0; i < 100; i++) {\n" +
- " s += " + veryLongString.toString() + ";\n" +
  " }\n" +
- " }\n" +
  "}"
  },
- expectedError
- );
+ "----------\n" +
+ "1. ERROR in X.java (at line 2)\n" +
+ " void foo(String a, String b, String c, String d, String e) {\n" +
+ " ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" +
+ "The code of method foo(String, String, String, String, String) is " +
+ "exceeding the 65535 bytes limit\n" +
+ "----------\n");
 }
 
 // https://bugs.eclipse.org/bugs/show_bug.cgi?id=102728
@@ -3785,7 +3775,7 @@ public void test012() {
  "public class X {\n" +
  " void foo(String a, String b, String c, String d, String e) {\n" +
  " String s = a + (\n");
- for (int i = 0; i < 500; i++) {
+ for (int i = 0; i < 1000; i++) {
  sourceCode.append(
  " \"abcdef\" + a + b + c + d + e + " +
  "\" ghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmno" +