Skip to content

Commit 7b18440

Browse files
eric-milleseric-milles
authored
GROOVY-8699: SC: emit direct bytecode for simple list (#2395)
1 parent 50f267d commit 7b18440

4 files changed

Lines changed: 104 additions & 31 deletions

File tree

‎src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1981,8 +1981,8 @@ public void visitListExpression(final ListExpression expression) {
19811981
MethodVisitor mv = controller.getMethodVisitor();
19821982
BytecodeHelper.pushConstant(mv, size);
19831983
mv.visitTypeInsn(ANEWARRAY, "java/lang/Object");
1984-
int maxInit = 1000;
1985-
if (size<maxInit || !containsOnlyConstants) {
1984+
final int maxInit = 1000; // top end for unroll
1985+
if (size < maxInit || !containsOnlyConstants) {
19861986
for (int i = 0; i < size; i += 1) {
19871987
mv.visitInsn(DUP);
19881988
BytecodeHelper.pushConstant(mv, i);

‎src/main/java/org/codehaus/groovy/transform/sc/transformers/ListExpressionTransformer.java‎

Lines changed: 66 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,28 @@
1818
*/
1919
package org.codehaus.groovy.transform.sc.transformers;
2020

21+
import org.codehaus.groovy.ast.ClassHelper;
22+
import org.codehaus.groovy.ast.ClassNode;
2123
import org.codehaus.groovy.ast.ConstructorNode;
24+
import org.codehaus.groovy.ast.GroovyCodeVisitor;
2225
import org.codehaus.groovy.ast.MethodNode;
26+
import org.codehaus.groovy.ast.Parameter;
2327
import org.codehaus.groovy.ast.expr.ArgumentListExpression;
2428
import org.codehaus.groovy.ast.expr.ArrayExpression;
29+
import org.codehaus.groovy.ast.expr.ConstantExpression;
2530
import org.codehaus.groovy.ast.expr.ConstructorCallExpression;
2631
import org.codehaus.groovy.ast.expr.Expression;
32+
import org.codehaus.groovy.ast.expr.ExpressionTransformer;
2733
import org.codehaus.groovy.ast.expr.ListExpression;
34+
import org.codehaus.groovy.classgen.AsmClassGenerator;
2835
import org.codehaus.groovy.transform.stc.StaticTypesMarker;
2936

37+
import java.util.ArrayList;
3038
import java.util.List;
3139

32-
import static java.util.stream.Collectors.toList;
40+
import static org.codehaus.groovy.classgen.AsmClassGenerator.containsSpreadExpression;
41+
import static org.objectweb.asm.Opcodes.DUP;
42+
import static org.objectweb.asm.Opcodes.INVOKEVIRTUAL;
3343

3444
class ListExpressionTransformer {
3545

@@ -42,7 +52,7 @@ class ListExpressionTransformer {
4252
Expression transformListExpression(final ListExpression le) {
4353
MethodNode mn = le.getNodeMetaData(StaticTypesMarker.DIRECT_METHOD_CALL_TARGET);
4454
if (mn instanceof ConstructorNode) {
45-
List<Expression> elements = le.getExpressions().stream().map(scTransformer::transform).collect(toList());
55+
List<Expression> elements = le.getExpressions().stream().map(scTransformer::transform).toList();
4656

4757
if (mn.getDeclaringClass().isArray()) {
4858
var ae = new ArrayExpression(mn.getDeclaringClass().getComponentType(), elements);
@@ -56,6 +66,60 @@ Expression transformListExpression(final ListExpression le) {
5666
return cce;
5767
}
5868

69+
// GROOVY-8699: emit direct bytecode for simple list
70+
if (mn == null && le.getExpressions().size() < 25 && !containsSpreadExpression(le)) {
71+
ClassNode type = scTransformer.getTypeChooser().resolveType(le, scTransformer.getClassNode());
72+
if (ArrayList_TYPE.equals(type)) { // annotation attributes cannot be transformed
73+
var list = new NewListExpression(le.getExpressions().stream().map(scTransformer::transform).toList());
74+
list.setSourcePosition(le);
75+
list.copyNodeMetaData(le);
76+
return list;
77+
}
78+
}
79+
5980
return scTransformer.superTransform(le);
6081
}
82+
83+
//--------------------------------------------------------------------------
84+
85+
private static final ClassNode ArrayList_TYPE = ClassHelper.makeWithoutCaching(ArrayList.class);
86+
87+
private static final MethodNode ArrayList_NEW = ArrayList_TYPE.getDeclaredConstructor(new Parameter[] {new Parameter(ClassHelper.int_TYPE, "capacity")});
88+
89+
private static class NewListExpression extends ListExpression {
90+
91+
NewListExpression(final List<Expression> values) {
92+
super(values);
93+
}
94+
95+
@Override
96+
public Expression transformExpression(final ExpressionTransformer transformer) {
97+
var list = new NewListExpression(transformExpressions(getExpressions(), transformer));
98+
list.setSourcePosition(this);
99+
list.copyNodeMetaData(this);
100+
return list;
101+
}
102+
103+
@Override
104+
public void visit(final GroovyCodeVisitor visitor) {
105+
if (!(visitor instanceof AsmClassGenerator g)) {
106+
super.visit(visitor);
107+
} else {
108+
var mv = g.getController().getMethodVisitor();
109+
var os = g.getController().getOperandStack ();
110+
111+
var list = new ConstructorCallExpression(ArrayList_TYPE, new ConstantExpression(getExpressions().size(), true));
112+
list.putNodeMetaData(StaticTypesMarker.DIRECT_METHOD_CALL_TARGET, ArrayList_NEW);
113+
list.visit(visitor);
114+
115+
for (Expression li : getExpressions()) {
116+
mv.visitInsn(DUP);
117+
li.visit(visitor);
118+
os.box();
119+
mv.visitMethodInsn(INVOKEVIRTUAL, "java/util/ArrayList", "add", "(Ljava/lang/Object;)Z", false);
120+
os.pop(); // boolean return value
121+
}
122+
}
123+
}
124+
}
61125
}

‎src/test/groovy/bugs/Groovy10034.groovy‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,15 @@ final class Groovy10034 extends AbstractBytecodeTestCase {
3131
["x"].toArray(new String[0])
3232
}
3333
'''
34-
int offset = result.indexOf('INVOKESTATIC', result.indexOf('--BEGIN--'))
34+
int offset = result.indexOf('INVOKEVIRTUAL', result.indexOf('--BEGIN--'))
3535
assert result.hasStrictSequence([
36-
'INVOKESTATIC org/codehaus/groovy/runtime/ScriptBytecodeAdapter.createList',
37-
'CHECKCAST java/util/ArrayList', // not 'INVOKEDYNAMIC cast'
36+
'INVOKEVIRTUAL java/util/ArrayList.add',
37+
'POP',
3838
'ICONST_0',
3939
'ANEWARRAY java/lang/String',
4040
// no 'INVOKEDYNAMIC cast' to Object[]
41-
'INVOKEVIRTUAL java/util/ArrayList.toArray'
41+
'INVOKEVIRTUAL java/util/ArrayList.toArray',
42+
'ARETURN'
4243
], offset)
4344
}
4445
}

‎src/test/groovy/org/codehaus/groovy/classgen/asm/sc/ArraysAndCollectionsStaticCompileTest.groovy‎

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -31,19 +31,19 @@ final class ArraysAndCollectionsStaticCompileTest extends ArraysAndCollectionsST
3131
@Test
3232
void testShouldNotThrowVerifyError() {
3333
assertScript '''
34-
def al = new ArrayList<Double>()
35-
al.add(2.0d)
36-
assert al.get(0) + 1 == 3.0d
34+
def list = new ArrayList<Double>()
35+
list.add(2.0d)
36+
assert list.get(0) + 1 == 3.0d
3737
'''
3838
}
3939

4040
// GROOVY-5654
4141
@Test
4242
void testShouldNotThrowForbiddenAccessWithMapProperty() {
4343
assertScript '''
44-
Map<String, Integer> m = ['abcd': 1234]
45-
assert m['abcd'] == 1234
46-
assert m.abcd == 1234
44+
Map<String, Integer> map = ['abcd': 1234]
45+
assert map['abcd'] == 1234
46+
assert map.abcd == 1234
4747
'''
4848
}
4949

@@ -71,33 +71,41 @@ final class ArraysAndCollectionsStaticCompileTest extends ArraysAndCollectionsST
7171
@Test
7272
void testSpreadSafeMethodCallsOnListLiteralShouldNotCreateListTwice() {
7373
assertScript '''
74-
class Foo {
75-
static void test() {
76-
def list = [1, 2]
77-
def lengths = [list << 3]*.size()
78-
assert lengths == [3]
79-
assert list == [1, 2, 3]
80-
}
74+
void check(List items, List sizes) {
75+
assert items == [1, 2, 3]
76+
assert sizes == [3]
77+
}
78+
void test() {
79+
def items = [1, 2]
80+
def sizes = [items << 3]*.size()
81+
check(items, sizes)
8182
}
82-
Foo.test()
83+
test()
8384
'''
84-
assert astTrees['Foo'][1].count('ScriptBytecodeAdapter.createList') == 4
85+
String bytecode = astTrees.values()[0][1]
86+
int offset = bytecode.indexOf('test()V')
87+
bytecode = bytecode.substring(offset, bytecode.indexOf('RETURN', offset))
88+
89+
assert bytecode.count('ScriptBytecodeAdapter.createList') == 0 // GROOVY-8699
90+
assert bytecode.count('INVOKESPECIAL java/util/ArrayList.<init>') == 3 // one for the spread result
8591
}
8692

8793
// GROOVY-7688
8894
@Test
8995
void testSpreadSafeMethodCallReceiversWithSideEffectsShouldNotBeVisitedTwice() {
9096
assertScript '''
91-
class Foo {
92-
static void test() {
93-
def list = ['a', 'b']
94-
def lengths = list.toList()*.length()
95-
assert lengths == [1, 1]
96-
}
97+
void test() {
98+
def list = ['a', 'b']
99+
def lengths = list.toList()*.length()
100+
assert lengths == [1, 1]
97101
}
98-
Foo.test()
102+
test()
99103
'''
100-
assert astTrees['Foo'][1].count('DefaultGroovyMethods.toList') == 1
104+
String bytecode = astTrees.values()[0][1]
105+
int offset = bytecode.indexOf('test()V')
106+
bytecode = bytecode.substring(offset, bytecode.indexOf('RETURN', offset))
107+
108+
assert bytecode.count('DefaultGroovyMethods.toList') == 1
101109
}
102110

103111
@Override @Test

0 commit comments

Comments
 (0)