Skip to content

Commit 3190f42

Browse files
daniellansunpaulk-asert
authored andcommitted
GROOVY-12288: Cache ClassWriter getCommonSuperClass lookups per class
1 parent 05b836c commit 3190f42

3 files changed

Lines changed: 566 additions & 25 deletions

File tree

src/main/java/org/codehaus/groovy/control/CompilationUnit.java

Lines changed: 75 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@
6262
import java.util.ArrayList;
6363
import java.util.Comparator;
6464
import java.util.Deque;
65+
import java.util.HashMap;
6566
import java.util.HashSet;
6667
import java.util.Iterator;
6768
import java.util.LinkedHashMap;
@@ -905,36 +906,85 @@ public boolean needSortedInput() {
905906
* @return the class visitor used to emit bytecode
906907
*/
907908
protected ClassVisitor createClassVisitor() {
908-
return new ClassWriter(ClassWriter.COMPUTE_MAXS | ClassWriter.COMPUTE_FRAMES) {
909-
private ClassNode getClassNode(String name) {
910-
// try classes under compilation
911-
CompileUnit cu = getAST();
912-
ClassNode cn = cu.getClass(name);
913-
if (cn != null) return cn;
909+
return new CachingClassWriter();
910+
}
911+
912+
/**
913+
* Per-class {@link ClassWriter} that memoizes {@link #getCommonSuperClass}.
914+
* {@code COMPUTE_FRAMES} repeats the same internal-name pairs at merge points;
915+
* the writer (and its maps) live for one generated class and are then discarded.
916+
*/
917+
private final class CachingClassWriter extends ClassWriter {
918+
private final Map<String, ClassNode> classNodeByInternalName = new HashMap<>(8);
919+
private final Map<String, Map<String, String>> commonSuperByPair = new HashMap<>(8);
920+
921+
private CachingClassWriter() {
922+
super(COMPUTE_MAXS | COMPUTE_FRAMES);
923+
}
924+
925+
@Override
926+
protected String getCommonSuperClass(final String type1, final String type2) {
927+
Map<String, String> bySecond = commonSuperByPair.get(type1);
928+
if (bySecond != null) {
929+
String cached = bySecond.get(type2);
930+
if (cached != null) {
931+
return cached;
932+
}
933+
}
934+
ClassNode class1 = getClassNode(type1);
935+
ClassNode class2 = getClassNode(type2);
936+
if (class1 == null || class2 == null) {
937+
throw new GroovyBugError("Unable to determine common super class of " + type1 + " and " + type2);
938+
}
939+
ClassNode commonNode = getCommonSuperClassNode(class1, class2);
940+
String common;
941+
if (commonNode == class1) {
942+
common = type1;
943+
} else if (commonNode == class2) {
944+
common = type2;
945+
} else if (ClassHelper.isObjectType(commonNode)) {
946+
common = "java/lang/Object";
947+
} else {
948+
common = commonNode.getName().replace('.', '/');
949+
}
950+
commonSuperByPair.computeIfAbsent(type1, k -> new HashMap<>(4)).put(type2, common);
951+
commonSuperByPair.computeIfAbsent(type2, k -> new HashMap<>(4)).put(type1, common);
952+
return common;
953+
}
954+
955+
private ClassNode getClassNode(final String internalName) {
956+
ClassNode cached = classNodeByInternalName.get(internalName);
957+
if (cached != null) {
958+
return cached;
959+
}
960+
String name = internalName.replace('/', '.');
961+
// try classes under compilation
962+
CompileUnit cu = getAST();
963+
ClassNode cn = cu.getClass(name);
964+
if (cn == null) {
914965
// try inner classes
915966
cn = cu.getGeneratedInnerClass(name);
916-
if (cn != null) return cn;
967+
}
968+
if (cn == null) {
917969
ClassNodeResolver.LookupResult lookupResult = getClassNodeResolver().resolveName(name, CompilationUnit.this);
918-
return lookupResult == null ? null : lookupResult.getClassNode();
970+
cn = lookupResult == null ? null : lookupResult.getClassNode();
919971
}
920-
private ClassNode getCommonSuperClassNode(ClassNode c, ClassNode d) {
921-
// adapted from ClassWriter code
922-
if (c.isDerivedFrom(d)) return d;
923-
if (d.isDerivedFrom(c)) return c;
924-
if (c.isInterface() || d.isInterface()) return ClassHelper.OBJECT_TYPE;
925-
do {
926-
c = c.getSuperClass();
927-
} while (c != null && !d.isDerivedFrom(c));
928-
if (c == null) return ClassHelper.OBJECT_TYPE;
929-
return c;
972+
if (cn != null) {
973+
classNodeByInternalName.put(internalName, cn);
930974
}
931-
@Override
932-
protected String getCommonSuperClass(String arg1, String arg2) {
933-
ClassNode a = getClassNode(arg1.replace('/', '.'));
934-
ClassNode b = getClassNode(arg2.replace('/', '.'));
935-
return getCommonSuperClassNode(a,b).getName().replace('.','/');
936-
}
937-
};
975+
return cn;
976+
}
977+
978+
private ClassNode getCommonSuperClassNode(ClassNode c, ClassNode d) {
979+
// adapted from ClassWriter code
980+
if (c.isDerivedFrom(d)) return d;
981+
if (d.isDerivedFrom(c)) return c;
982+
if (c.isInterface() || d.isInterface()) return ClassHelper.OBJECT_TYPE;
983+
do {
984+
c = c.getSuperClass();
985+
} while (c != null && !d.isDerivedFrom(c));
986+
return c == null ? ClassHelper.OBJECT_TYPE : c;
987+
}
938988
}
939989

940990
//---------------------------------------------------------------------------
Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.codehaus.groovy.control
20+
21+
import org.codehaus.groovy.tools.GroovyClass
22+
import org.junit.jupiter.api.Test
23+
24+
import static org.junit.jupiter.api.Assertions.assertEquals
25+
import static org.junit.jupiter.api.Assertions.assertNotNull
26+
import static org.junit.jupiter.api.Assertions.assertTrue
27+
28+
/**
29+
* Tests for {@link CompilationUnit}'s ClassWriter and getCommonSuperClass lookups (GROOVY-12288).
30+
*/
31+
class ClassWriterCommonSuperClassTest {
32+
33+
@Test
34+
void testBasicClassHierarchyMergeAndExecution() {
35+
GroovyClassLoader gcl = new GroovyClassLoader()
36+
String script = '''
37+
package test.pkg
38+
39+
class Base {}
40+
class ChildA extends Base {}
41+
class ChildB extends Base {}
42+
43+
class Tester {
44+
static Base choose(boolean flag) {
45+
return flag ? new ChildA() : new ChildB()
46+
}
47+
}
48+
'''
49+
CompilationUnit cu = new CompilationUnit(CompilerConfiguration.DEFAULT, null, gcl)
50+
cu.addSource("test/pkg/Test.groovy", script)
51+
cu.compile(Phases.CLASS_GENERATION)
52+
53+
List<GroovyClass> classes = cu.getClasses()
54+
assertNotNull(classes)
55+
assertTrue(classes.size() >= 4)
56+
57+
for (GroovyClass gc : classes) {
58+
gcl.defineClass(gc.getName(), gc.getBytes())
59+
}
60+
61+
Class<?> testerClass = gcl.loadClass('test.pkg.Tester')
62+
def resA = testerClass.getMethod('choose', boolean.class).invoke(null, true)
63+
def resB = testerClass.getMethod('choose', boolean.class).invoke(null, false)
64+
assertEquals('test.pkg.ChildA', resA.class.name)
65+
assertEquals('test.pkg.ChildB', resB.class.name)
66+
}
67+
68+
@Test
69+
void testDeepHierarchyAndMultiCatchExecution() {
70+
GroovyClassLoader gcl = new GroovyClassLoader()
71+
String script = '''
72+
package test.deep
73+
74+
class L0 {}
75+
class L1 extends L0 {}
76+
class L2 extends L1 {}
77+
class L3 extends L2 {}
78+
class Leaf1 extends L3 {}
79+
class Leaf2 extends L3 {}
80+
81+
class DeepTester {
82+
static Object process(int mode, boolean flag) {
83+
def val = null
84+
try {
85+
if (mode == 1) throw new java.io.FileNotFoundException("fnf")
86+
if (mode == 2) throw new java.io.EOFException("eof")
87+
val = flag ? new Leaf1() : new Leaf2()
88+
} catch (java.io.FileNotFoundException | java.io.EOFException e) {
89+
val = flag ? new java.util.ArrayList() : new java.util.LinkedList()
90+
} catch (Exception e) {
91+
val = flag ? new java.util.HashMap() : new java.util.TreeMap()
92+
}
93+
return val
94+
}
95+
}
96+
'''
97+
CompilationUnit cu = new CompilationUnit(CompilerConfiguration.DEFAULT, null, gcl)
98+
cu.addSource("test/deep/DeepTest.groovy", script)
99+
cu.compile(Phases.CLASS_GENERATION)
100+
101+
for (GroovyClass gc : cu.getClasses()) {
102+
gcl.defineClass(gc.getName(), gc.getBytes())
103+
}
104+
105+
Class<?> testerClass = gcl.loadClass('test.deep.DeepTester')
106+
def res0 = testerClass.getMethod('process', int.class, boolean.class).invoke(null, 0, true)
107+
def res1 = testerClass.getMethod('process', int.class, boolean.class).invoke(null, 1, false)
108+
def res2 = testerClass.getMethod('process', int.class, boolean.class).invoke(null, 2, true)
109+
110+
assertEquals('test.deep.Leaf1', res0.class.name)
111+
assertEquals(java.util.LinkedList.class, res1.class)
112+
assertEquals(java.util.ArrayList.class, res2.class)
113+
}
114+
115+
@Test
116+
void testStaticCompileWithComplexGenericsAndLoops() {
117+
GroovyClassLoader gcl = new GroovyClassLoader()
118+
String script = '''
119+
package test.sc
120+
import groovy.transform.CompileStatic
121+
122+
@CompileStatic
123+
class StaticTester {
124+
static Object mergeComplex(int count, boolean flag) {
125+
Object result = flag ? new java.util.ArrayList<String>() : new java.util.LinkedList<String>()
126+
for (int i = 0; i < count; i++) {
127+
Object inner = (i % 2 == 0) ? (flag ? new java.util.HashSet<Integer>() : new java.util.TreeSet<Integer>())
128+
: (flag ? new StringBuilder() : new StringBuffer())
129+
if (i == count - 1) {
130+
result = inner
131+
}
132+
}
133+
return result
134+
}
135+
}
136+
'''
137+
CompilationUnit cu = new CompilationUnit(CompilerConfiguration.DEFAULT, null, gcl)
138+
cu.addSource("test/sc/StaticTester.groovy", script)
139+
cu.compile(Phases.CLASS_GENERATION)
140+
141+
for (GroovyClass gc : cu.getClasses()) {
142+
gcl.defineClass(gc.getName(), gc.getBytes())
143+
}
144+
145+
Class<?> testerClass = gcl.loadClass('test.sc.StaticTester')
146+
def res = testerClass.getMethod('mergeComplex', int.class, boolean.class).invoke(null, 4, true)
147+
assertEquals(StringBuilder.class, res.class)
148+
}
149+
150+
@Test
151+
void testInterfaceAndNestedClosureCompilation() {
152+
GroovyClassLoader gcl = new GroovyClassLoader()
153+
String script = '''
154+
package test.closure
155+
156+
interface IntfA {}
157+
interface IntfB {}
158+
class ImplA implements IntfA {}
159+
class ImplB implements IntfB {}
160+
161+
class ClosureTester {
162+
static List evaluate() {
163+
def list = [1, 2, 3, 4]
164+
def c1 = { int x -> (x % 2 == 0) ? new ImplA() : new ImplB() }
165+
def c2 = { int x -> (x % 2 == 0) ? new java.util.ArrayList() : new java.util.HashSet() }
166+
return list.collect(c1) + list.collect(c2)
167+
}
168+
}
169+
'''
170+
CompilationUnit cu = new CompilationUnit(CompilerConfiguration.DEFAULT, null, gcl)
171+
cu.addSource("test/closure/ClosureTester.groovy", script)
172+
cu.compile(Phases.CLASS_GENERATION)
173+
174+
for (GroovyClass gc : cu.getClasses()) {
175+
gcl.defineClass(gc.getName(), gc.getBytes())
176+
}
177+
178+
Class<?> testerClass = gcl.loadClass('test.closure.ClosureTester')
179+
List res = (List) testerClass.getMethod('evaluate').invoke(null)
180+
assertEquals(8, res.size())
181+
}
182+
}

0 commit comments

Comments
 (0)