Skip to content

Commit 9ae3aa8

Browse files
davleopodavleopo
committed
[GR-60280] Invalidate compiler test cache on new options.
PullRequest: graal/19610
2 parents fcb4ce9 + d7d24b5 commit 9ae3aa8

File tree

4 files changed

+153
-17
lines changed

4 files changed

+153
-17
lines changed

‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/CacheResetTest.java‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
/*
2+
* Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation. Oracle designates this
8+
* particular file as subject to the "Classpath" exception as provided
9+
* by Oracle in the LICENSE file that accompanied this code.
10+
*
11+
* This code is distributed in the hope that it will be useful, but WITHOUT
12+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
13+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
14+
* version 2 for more details (a copy is included in the LICENSE file that
15+
* accompanied this code).
16+
*
17+
* You should have received a copy of the GNU General Public License version
18+
* 2 along with this work; if not, write to the Free Software Foundation,
19+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
20+
*
21+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
22+
* or visit www.oracle.com if you need additional information or have any
23+
* questions.
24+
*/
25+
package jdk.graal.compiler.core.test;
26+
27+
import java.util.HashMap;
28+
29+
import org.graalvm.collections.Pair;
30+
import org.junit.Assert;
31+
import org.junit.Test;
32+
33+
import jdk.graal.compiler.core.phases.HighTier;
34+
import jdk.graal.compiler.options.OptionValues;
35+
import jdk.vm.ci.code.InstalledCode;
36+
import jdk.vm.ci.meta.ResolvedJavaMethod;
37+
38+
/**
39+
* Tests that the cache used by the compiler tests properly handles different options.
40+
*/
41+
public class CacheResetTest extends GraalCompilerTest {
42+
43+
public static void snippet() {
44+
// do nothing
45+
}
46+
47+
private static final String SNIPPET_NAME = "snippet";
48+
49+
@Test
50+
public void testSameOptionsReuse() {
51+
final ResolvedJavaMethod installedCodeOwner = getResolvedJavaMethod(SNIPPET_NAME);
52+
53+
OptionValues opt = new OptionValues(getInitialOptions(), HighTier.Options.Inline, true);
54+
55+
test(opt, SNIPPET_NAME);
56+
InstalledCode firstInstalled = getCachedCode(installedCodeOwner);
57+
58+
test(opt, SNIPPET_NAME);
59+
InstalledCode secondInstalled = getCachedCode(installedCodeOwner);
60+
61+
assert firstInstalled.isValid();
62+
assert secondInstalled.isValid();
63+
64+
Assert.assertEquals(firstInstalled.getStart(), secondInstalled.getStart());
65+
}
66+
67+
@Test
68+
public void testSameOptionsDeepReuse() {
69+
final ResolvedJavaMethod installedCodeOwner = getResolvedJavaMethod(SNIPPET_NAME);
70+
71+
OptionValues opt = new OptionValues(getInitialOptions(), HighTier.Options.Inline, true);
72+
test(opt, SNIPPET_NAME);
73+
InstalledCode firstInstalled = getCachedCode(installedCodeOwner);
74+
75+
OptionValues opt1 = new OptionValues(getInitialOptions(), HighTier.Options.Inline, true);
76+
test(opt1, SNIPPET_NAME);
77+
InstalledCode secondInstalled = getCachedCode(installedCodeOwner);
78+
79+
assert firstInstalled.isValid();
80+
assert secondInstalled.isValid();
81+
82+
Assert.assertEquals(firstInstalled.getStart(), secondInstalled.getStart());
83+
}
84+
85+
private static InstalledCode getCachedCode(ResolvedJavaMethod method) {
86+
HashMap<ResolvedJavaMethod, Pair<OptionValues, InstalledCode>> tlCache = cache.get();
87+
Pair<OptionValues, InstalledCode> cached = tlCache.get(method);
88+
return cached.getRight();
89+
}
90+
91+
@Test
92+
public void testDifferentOptionsNoReuse() {
93+
final ResolvedJavaMethod installedCodeOwner = getResolvedJavaMethod(SNIPPET_NAME);
94+
95+
OptionValues opt = new OptionValues(getInitialOptions(), HighTier.Options.Inline, true);
96+
test(opt, SNIPPET_NAME);
97+
InstalledCode firstInstalled = getCachedCode(installedCodeOwner);
98+
99+
OptionValues opt1 = new OptionValues(getInitialOptions(), HighTier.Options.Inline, false);
100+
test(opt1, SNIPPET_NAME);
101+
InstalledCode secondInstalled = getCachedCode(installedCodeOwner);
102+
103+
assert firstInstalled.isValid();
104+
assert secondInstalled.isValid();
105+
106+
Assert.assertNotEquals(firstInstalled.getStart(), secondInstalled.getStart());
107+
}
108+
}

‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/GraalCompilerTest.java‎

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@
5959
import java.util.concurrent.ConcurrentHashMap;
6060
import java.util.function.Supplier;
6161

62+
import org.graalvm.collections.Pair;
63+
import org.graalvm.collections.UnmodifiableEconomicMap;
6264
import org.junit.After;
6365
import org.junit.Assert;
6466
import org.junit.Before;
@@ -129,6 +131,7 @@
129131
import jdk.graal.compiler.nodes.spi.ProfileProvider;
130132
import jdk.graal.compiler.nodes.spi.Replacements;
131133
import jdk.graal.compiler.nodes.virtual.VirtualObjectNode;
134+
import jdk.graal.compiler.options.OptionKey;
132135
import jdk.graal.compiler.options.OptionValues;
133136
import jdk.graal.compiler.phases.BasePhase;
134137
import jdk.graal.compiler.phases.OptimisticOptimizations;
@@ -395,7 +398,7 @@ protected LIRSuites createLIRSuites(OptionValues opts) {
395398
return ret;
396399
}
397400

398-
private static final ThreadLocal<HashMap<ResolvedJavaMethod, InstalledCode>> cache = ThreadLocal.withInitial(HashMap::new);
401+
protected static final ThreadLocal<HashMap<ResolvedJavaMethod, Pair<OptionValues, InstalledCode>>> cache = ThreadLocal.withInitial(HashMap::new);
399402

400403
/**
401404
* Reset the entire {@linkplain #cache} of {@linkplain InstalledCode}. Additionally, invalidate
@@ -404,8 +407,8 @@ protected LIRSuites createLIRSuites(OptionValues opts) {
404407
*/
405408
@BeforeClass
406409
public static void resetCodeCache() {
407-
for (InstalledCode code : cache.get().values()) {
408-
code.invalidate();
410+
for (Pair<OptionValues, InstalledCode> code : cache.get().values()) {
411+
code.getRight().invalidate();
409412
}
410413
cache.get().clear();
411414
}
@@ -1133,11 +1136,14 @@ protected final InstalledCode getCode(final ResolvedJavaMethod installedCodeOwne
11331136
protected InstalledCode getCode(final ResolvedJavaMethod installedCodeOwner, StructuredGraph graph, boolean forceCompile, boolean installAsDefault, OptionValues options) {
11341137
boolean useCache = !forceCompile && getArgumentToBind() == null;
11351138
if (useCache && graph == null) {
1136-
HashMap<ResolvedJavaMethod, InstalledCode> tlCache = cache.get();
1137-
InstalledCode cached = tlCache.get(installedCodeOwner);
1139+
HashMap<ResolvedJavaMethod, Pair<OptionValues, InstalledCode>> tlCache = cache.get();
1140+
Pair<OptionValues, InstalledCode> cached = tlCache.get(installedCodeOwner);
11381141
if (cached != null) {
1139-
if (cached.isValid()) {
1140-
return cached;
1142+
// Reuse the cached code if it is still valid and the same options was used for
1143+
// the compilation. We use a deep equals for the option values to catch cases where
1144+
// users create new option values but with the same values.
1145+
if (cached.getRight().isValid() && (options.getMap().equals(cached.getLeft().getMap()) || optionsMapDeepEquals(options.getMap(), cached.getLeft().getMap()))) {
1146+
return cached.getRight();
11411147
} else {
11421148
tlCache.remove(installedCodeOwner);
11431149
}
@@ -1184,13 +1190,35 @@ protected InstalledCode getCode(final ResolvedJavaMethod installedCodeOwner, Str
11841190
}
11851191

11861192
if (useCache) {
1187-
cache.get().put(installedCodeOwner, installedCode);
1193+
cache.get().put(installedCodeOwner, Pair.create(options, installedCode));
11881194
}
11891195
return installedCode;
11901196
}
11911197
throw GraalError.shouldNotReachHere("Bailout limit reached"); // ExcludeFromJacocoGeneratedReport
11921198
}
11931199

1200+
private static boolean optionsMapDeepEquals(UnmodifiableEconomicMap<OptionKey<?>, Object> map1, UnmodifiableEconomicMap<OptionKey<?>, Object> map2) {
1201+
if (map1.size() != map2.size()) {
1202+
return false;
1203+
}
1204+
var c1 = map1.getEntries();
1205+
var c2 = map2.getEntries();
1206+
while (c1.advance() && c2.advance()) {
1207+
Object c1Key = c1.getKey();
1208+
Object c2Key = c2.getKey();
1209+
if (!c1Key.equals(c2Key)) {
1210+
return false;
1211+
}
1212+
Object c1Val = c1.getValue();
1213+
Object c2Val = c2.getValue();
1214+
if (!c1Val.equals(c2Val)) {
1215+
return false;
1216+
}
1217+
}
1218+
1219+
return true;
1220+
}
1221+
11941222
/**
11951223
* Used to produce a graph for a method about to be compiled by
11961224
* {@link #compile(ResolvedJavaMethod, StructuredGraph)} if the second parameter to that method

‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/truffle/test/EncodedGraphCacheTest.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,8 @@ private static RootTestNode rootTestNode() {
128128
}
129129

130130
private static boolean encodedGraphCacheContains(TruffleCompilerImpl compiler, ResolvedJavaMethod method) {
131-
EconomicMap<ResolvedJavaMethod, EncodedGraph> cache = compiler.getPartialEvaluator().getOrCreateEncodedGraphCache();
132-
return cache.containsKey(method);
131+
EconomicMap<ResolvedJavaMethod, EncodedGraph> cache1 = compiler.getPartialEvaluator().getOrCreateEncodedGraphCache();
132+
return cache1.containsKey(method);
133133
}
134134

135135
@SuppressWarnings("try")
@@ -218,8 +218,8 @@ public void testUnboundedCacheCapacity() {
218218
boolean encodedGraphCache = true;
219219
testHelper(encodedGraphCache, 100_000, compiler -> {
220220
runWithBooleanFlag(compiler, EncodedGraphCacheTest::disableEncodedGraphCachePurges, true, () -> {
221-
EconomicMap<?, ?> cache = compiler.getPartialEvaluator().getOrCreateEncodedGraphCache();
222-
nonEmptyGraphCache[0] = !cache.isEmpty();
221+
EconomicMap<?, ?> cache1 = compiler.getPartialEvaluator().getOrCreateEncodedGraphCache();
222+
nonEmptyGraphCache[0] = !cache1.isEmpty();
223223
});
224224
});
225225
}

‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/truffle/test/strings/TStringOpsTest.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -176,18 +176,18 @@ protected ResolvedJavaMethod getIndexOf2ConsecutiveWithStrideIntl() {
176176
}
177177

178178
protected InstalledCode cacheInstalledCodeConstantStride(ResolvedJavaMethod installedCodeOwner, StructuredGraph graph, OptionValues options, ResolvedJavaMethod expectedMethod,
179-
InstalledCode[] cache, int strideA, int strideB) {
180-
return cacheInstalledCodeConstantStrideLength(installedCodeOwner, graph, options, expectedMethod, cache, strideA, strideB, 0);
179+
InstalledCode[] cache1, int strideA, int strideB) {
180+
return cacheInstalledCodeConstantStrideLength(installedCodeOwner, graph, options, expectedMethod, cache1, strideA, strideB, 0);
181181
}
182182

183183
protected InstalledCode cacheInstalledCodeConstantStrideLength(ResolvedJavaMethod installedCodeOwner, StructuredGraph graph, OptionValues options, ResolvedJavaMethod expectedMethod,
184-
InstalledCode[] cache, int strideA, int strideB, int iLength) {
184+
InstalledCode[] cache1, int strideA, int strideB, int iLength) {
185185
Assert.assertEquals(expectedMethod, installedCodeOwner);
186186
int index = (iLength * 9) + (strideA * 3) + strideB;
187-
InstalledCode installedCode = cache[index];
187+
InstalledCode installedCode = cache1[index];
188188
while (installedCode == null || !installedCode.isValid()) {
189189
installedCode = super.getCode(installedCodeOwner, graph, true, false, options);
190-
cache[index] = installedCode;
190+
cache1[index] = installedCode;
191191
}
192192
return installedCode;
193193
}

0 commit comments

Comments
 (0)