This is an automated email from the ASF dual-hosted git repository. emilles pushed a commit to branch GROOVY-8699 in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 493c70912224b476391f790683ca56fe9227f305 Author: Eric Milles <[email protected]> AuthorDate: Tue Mar 10 17:05:02 2026 -0500 GROOVY-8699: SC: emit direct bytecode for simple list --- .../groovy/classgen/AsmClassGenerator.java | 4 +- .../org/codehaus/groovy/classgen/Verifier.java | 2 + .../sc/transformers/ListExpressionTransformer.java | 68 +++++++++++++++++++++- src/test/groovy/bugs/Groovy10034.groovy | 9 +-- .../ArraysAndCollectionsStaticCompileTest.groovy | 54 +++++++++-------- 5 files changed, 106 insertions(+), 31 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java index a90ae3e9d0..fdfff84e8c 100644 --- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java +++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java @@ -1981,8 +1981,8 @@ public class AsmClassGenerator extends ClassGenerator { MethodVisitor mv = controller.getMethodVisitor(); BytecodeHelper.pushConstant(mv, size); mv.visitTypeInsn(ANEWARRAY, "java/lang/Object"); - int maxInit = 1000; - if (size<maxInit || !containsOnlyConstants) { + final int maxInit = 1000; // top end for unroll + if (size < maxInit || !containsOnlyConstants) { for (int i = 0; i < size; i += 1) { mv.visitInsn(DUP); BytecodeHelper.pushConstant(mv, i); diff --git a/src/main/java/org/codehaus/groovy/classgen/Verifier.java b/src/main/java/org/codehaus/groovy/classgen/Verifier.java index 9f1d13c74e..331f3f2932 100644 --- a/src/main/java/org/codehaus/groovy/classgen/Verifier.java +++ b/src/main/java/org/codehaus/groovy/classgen/Verifier.java @@ -124,6 +124,7 @@ import static org.codehaus.groovy.ast.tools.GenericsUtils.correctToGenericsSpec; import static org.codehaus.groovy.ast.tools.GenericsUtils.createGenericsSpec; import static org.codehaus.groovy.ast.tools.GenericsUtils.parameterizeType; import static org.codehaus.groovy.ast.tools.PropertyNodeUtils.adjustPropertyModifiersForMethod; +import static org.codehaus.groovy.transform.sc.StaticCompilationMetadataKeys.STATIC_COMPILE_NODE; /** * Verifies the AST node and adds any default AST code before bytecode generation occurs. @@ -1020,6 +1021,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes { addPropertyMethod(newMethod); newMethod.putNodeMetaData(DEFAULT_PARAMETER_GENERATED, Boolean.TRUE); + newMethod.putNodeMetaData(STATIC_COMPILE_NODE, method.getNodeMetaData(STATIC_COMPILE_NODE)); }); } diff --git a/src/main/java/org/codehaus/groovy/transform/sc/transformers/ListExpressionTransformer.java b/src/main/java/org/codehaus/groovy/transform/sc/transformers/ListExpressionTransformer.java index 090a5e817c..5986dbddc0 100644 --- a/src/main/java/org/codehaus/groovy/transform/sc/transformers/ListExpressionTransformer.java +++ b/src/main/java/org/codehaus/groovy/transform/sc/transformers/ListExpressionTransformer.java @@ -18,18 +18,28 @@ */ package org.codehaus.groovy.transform.sc.transformers; +import org.codehaus.groovy.ast.ClassHelper; +import org.codehaus.groovy.ast.ClassNode; import org.codehaus.groovy.ast.ConstructorNode; +import org.codehaus.groovy.ast.GroovyCodeVisitor; import org.codehaus.groovy.ast.MethodNode; +import org.codehaus.groovy.ast.Parameter; import org.codehaus.groovy.ast.expr.ArgumentListExpression; import org.codehaus.groovy.ast.expr.ArrayExpression; +import org.codehaus.groovy.ast.expr.ConstantExpression; import org.codehaus.groovy.ast.expr.ConstructorCallExpression; import org.codehaus.groovy.ast.expr.Expression; +import org.codehaus.groovy.ast.expr.ExpressionTransformer; import org.codehaus.groovy.ast.expr.ListExpression; +import org.codehaus.groovy.classgen.AsmClassGenerator; import org.codehaus.groovy.transform.stc.StaticTypesMarker; +import java.util.ArrayList; import java.util.List; -import static java.util.stream.Collectors.toList; +import static org.codehaus.groovy.classgen.AsmClassGenerator.containsSpreadExpression; +import static org.objectweb.asm.Opcodes.DUP; +import static org.objectweb.asm.Opcodes.INVOKEVIRTUAL; class ListExpressionTransformer { @@ -42,7 +52,7 @@ class ListExpressionTransformer { Expression transformListExpression(final ListExpression le) { MethodNode mn = le.getNodeMetaData(StaticTypesMarker.DIRECT_METHOD_CALL_TARGET); if (mn instanceof ConstructorNode) { - List<Expression> elements = le.getExpressions().stream().map(scTransformer::transform).collect(toList()); + List<Expression> elements = le.getExpressions().stream().map(scTransformer::transform).toList(); if (mn.getDeclaringClass().isArray()) { var ae = new ArrayExpression(mn.getDeclaringClass().getComponentType(), elements); @@ -56,6 +66,60 @@ class ListExpressionTransformer { return cce; } + // GROOVY-8699: emit direct bytecode for simple list + if (mn == null && le.getExpressions().size() < 25 && !containsSpreadExpression(le)) { + ClassNode type = scTransformer.getTypeChooser().resolveType(le, scTransformer.getClassNode()); + if (ArrayList_TYPE.equals(type)) { // annotation attributes cannot be transformed + var list = new NewListExpression(le.getExpressions().stream().map(scTransformer::transform).toList()); + list.setSourcePosition(le); + list.copyNodeMetaData(le); + return list; + } + } + return scTransformer.superTransform(le); } + + //-------------------------------------------------------------------------- + + private static final ClassNode ArrayList_TYPE = ClassHelper.makeWithoutCaching(ArrayList.class); + + private static final MethodNode ArrayList_NEW = ArrayList_TYPE.getDeclaredConstructor(new Parameter[] {new Parameter(ClassHelper.int_TYPE, "capacity")}); + + private static class NewListExpression extends ListExpression { + + NewListExpression(final List<Expression> values) { + super(values); + } + + @Override + public Expression transformExpression(final ExpressionTransformer transformer) { + var list = new NewListExpression(transformExpressions(getExpressions(), transformer)); + list.setSourcePosition(this); + list.copyNodeMetaData(this); + return list; + } + + @Override + public void visit(final GroovyCodeVisitor visitor) { + if (!(visitor instanceof AsmClassGenerator g)) { + super.visit(visitor); + } else { + var mv = g.getController().getMethodVisitor(); + var os = g.getController().getOperandStack (); + + var list = new ConstructorCallExpression(ArrayList_TYPE, new ConstantExpression(getExpressions().size(), true)); + list.putNodeMetaData(StaticTypesMarker.DIRECT_METHOD_CALL_TARGET, ArrayList_NEW); + list.visit(visitor); + + for (Expression li : getExpressions()) { + mv.visitInsn(DUP); + li.visit(visitor); + os.box(); + mv.visitMethodInsn(INVOKEVIRTUAL, "java/util/ArrayList", "add", "(Ljava/lang/Object;)Z", false); + os.pop(); // boolean return value + } + } + } + } } diff --git a/src/test/groovy/bugs/Groovy10034.groovy b/src/test/groovy/bugs/Groovy10034.groovy index 7dc6baefa5..e5c3cb5f5d 100644 --- a/src/test/groovy/bugs/Groovy10034.groovy +++ b/src/test/groovy/bugs/Groovy10034.groovy @@ -31,14 +31,15 @@ final class Groovy10034 extends AbstractBytecodeTestCase { ["x"].toArray(new String[0]) } ''' - int offset = result.indexOf('INVOKESTATIC', result.indexOf('--BEGIN--')) + int offset = result.indexOf('INVOKEVIRTUAL', result.indexOf('--BEGIN--')) assert result.hasStrictSequence([ - 'INVOKESTATIC org/codehaus/groovy/runtime/ScriptBytecodeAdapter.createList', - 'CHECKCAST java/util/ArrayList', // not 'INVOKEDYNAMIC cast' + 'INVOKEVIRTUAL java/util/ArrayList.add', + 'POP', 'ICONST_0', 'ANEWARRAY java/lang/String', // no 'INVOKEDYNAMIC cast' to Object[] - 'INVOKEVIRTUAL java/util/ArrayList.toArray' + 'INVOKEVIRTUAL java/util/ArrayList.toArray', + 'ARETURN' ], offset) } } diff --git a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/ArraysAndCollectionsStaticCompileTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/ArraysAndCollectionsStaticCompileTest.groovy index da720a4c05..0adcbf0dc0 100644 --- a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/ArraysAndCollectionsStaticCompileTest.groovy +++ b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/ArraysAndCollectionsStaticCompileTest.groovy @@ -31,9 +31,9 @@ final class ArraysAndCollectionsStaticCompileTest extends ArraysAndCollectionsST @Test void testShouldNotThrowVerifyError() { assertScript ''' - def al = new ArrayList<Double>() - al.add(2.0d) - assert al.get(0) + 1 == 3.0d + def list = new ArrayList<Double>() + list.add(2.0d) + assert list.get(0) + 1 == 3.0d ''' } @@ -41,9 +41,9 @@ final class ArraysAndCollectionsStaticCompileTest extends ArraysAndCollectionsST @Test void testShouldNotThrowForbiddenAccessWithMapProperty() { assertScript ''' - Map<String, Integer> m = ['abcd': 1234] - assert m['abcd'] == 1234 - assert m.abcd == 1234 + Map<String, Integer> map = ['abcd': 1234] + assert map['abcd'] == 1234 + assert map.abcd == 1234 ''' } @@ -71,33 +71,41 @@ final class ArraysAndCollectionsStaticCompileTest extends ArraysAndCollectionsST @Test void testSpreadSafeMethodCallsOnListLiteralShouldNotCreateListTwice() { assertScript ''' - class Foo { - static void test() { - def list = [1, 2] - def lengths = [list << 3]*.size() - assert lengths == [3] - assert list == [1, 2, 3] - } + void check(List items, List sizes) { + assert items == [1, 2, 3] + assert sizes == [3] + } + void test() { + def items = [1, 2] + def sizes = [items << 3]*.size() + check(items, sizes) } - Foo.test() + test() ''' - assert astTrees['Foo'][1].count('ScriptBytecodeAdapter.createList') == 4 + String bytecode = astTrees.values()[0][1] + int offset = bytecode.indexOf('test()V') + bytecode = bytecode.substring(offset, bytecode.indexOf('RETURN', offset)) + + assert bytecode.count('ScriptBytecodeAdapter.createList') == 0 // GROOVY-8699 + assert bytecode.count('INVOKESPECIAL java/util/ArrayList.<init>') == 3 // one for the spread result } // GROOVY-7688 @Test void testSpreadSafeMethodCallReceiversWithSideEffectsShouldNotBeVisitedTwice() { assertScript ''' - class Foo { - static void test() { - def list = ['a', 'b'] - def lengths = list.toList()*.length() - assert lengths == [1, 1] - } + void test() { + def list = ['a', 'b'] + def lengths = list.toList()*.length() + assert lengths == [1, 1] } - Foo.test() + test() ''' - assert astTrees['Foo'][1].count('DefaultGroovyMethods.toList') == 1 + String bytecode = astTrees.values()[0][1] + int offset = bytecode.indexOf('test()V') + bytecode = bytecode.substring(offset, bytecode.indexOf('RETURN', offset)) + + assert bytecode.count('DefaultGroovyMethods.toList') == 1 } @Override @Test
