Skip to content

Commit d59ba4e

Browse files
tzezulatzezula
committed
Removed id cache from TruffleLogger.
1 parent 2ec37d4 commit d59ba4e

File tree

7 files changed

+141
-66
lines changed

7 files changed

+141
-66
lines changed

‎truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/polyglot/SeparatedClassLoadersTest.java‎

Lines changed: 99 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,21 @@
4141
package com.oracle.truffle.api.test.polyglot;
4242

4343
import static org.junit.Assert.assertNotNull;
44+
import static org.junit.Assert.assertTrue;
4445

46+
import java.lang.reflect.InvocationTargetException;
47+
import java.lang.reflect.Method;
4548
import java.net.URL;
4649
import java.net.URLClassLoader;
4750
import java.security.ProtectionDomain;
4851

52+
import com.oracle.truffle.api.TruffleLanguage.Registration;
53+
import com.oracle.truffle.api.TruffleLogger;
54+
import com.oracle.truffle.api.nodes.RootNode;
55+
import com.oracle.truffle.api.test.ReflectionUtils;
56+
import com.oracle.truffle.api.test.SubprocessTestUtils;
57+
import com.oracle.truffle.api.test.common.AbstractExecutableTestLanguage;
58+
import com.oracle.truffle.api.test.common.TestUtils;
4959
import org.graalvm.collections.EconomicMap;
5060
import org.graalvm.nativeimage.ImageInfo;
5161
import org.graalvm.polyglot.Engine;
@@ -74,7 +84,72 @@ public void storeLoader() {
7484
}
7585

7686
@Test
77-
public void sdkAndTruffleAPIInSeparateClassLoaders() throws Exception {
87+
public void sdkAndTruffleAPIInSeparateClassLoaders() {
88+
ClassLoaders classLoaders = createContextClassLoaders();
89+
Object contextClassLoaderEngine = createEngineInContextClassLoader(classLoaders);
90+
try {
91+
assertNotNull("Engine has been created", contextClassLoaderEngine);
92+
} finally {
93+
close(contextClassLoaderEngine);
94+
}
95+
}
96+
97+
/**
98+
* Verifies that TruffleLogger does not cache language IDs.
99+
*/
100+
@Test
101+
@SuppressWarnings("try")
102+
public void testLoggers() throws Exception {
103+
/*
104+
* This test is specific to HotSpot, as native-image has a fixed set of languages determined
105+
* at build time.
106+
*/
107+
Assume.assumeFalse(ImageInfo.inImageCode());
108+
SubprocessTestUtils.newBuilder(SeparatedClassLoadersTest.class, () -> {
109+
/*
110+
* The test uses an engine with a custom classloader that does not contain TestLanguage.
111+
* Therefore, getLogger is expected to fail.
112+
*/
113+
ClassLoaders classLoaders = createContextClassLoaders();
114+
Object contextClassLoaderEngine = createEngineInContextClassLoader(classLoaders);
115+
try {
116+
AbstractPolyglotTest.assertFails(() -> getLoggerReflective(classLoaders.truffleLoader, TestUtils.getDefaultLanguageId(TestLanguage.class)),
117+
IllegalArgumentException.class,
118+
(e) -> assertTrue(e.getMessage().startsWith("Unknown language or instrument")));
119+
} finally {
120+
close(contextClassLoaderEngine);
121+
}
122+
123+
Thread.currentThread().setContextClassLoader(null);
124+
try (Engine systemClassLoaderEngine = Engine.create()) {
125+
// Engine using a system classloader must contain TestLanguage
126+
assertNotNull(TruffleLogger.getLogger(TestUtils.getDefaultLanguageId(TestLanguage.class)));
127+
}
128+
}).run();
129+
}
130+
131+
private static Object getLoggerReflective(ClassLoader loader, String loggerId) {
132+
try {
133+
Class<?> truffleLogger = Class.forName(TruffleLogger.class.getName(), false, loader);
134+
Method m = truffleLogger.getDeclaredMethod("getLogger", String.class);
135+
ReflectionUtils.setAccessible(m, true);
136+
return m.invoke(null, loggerId);
137+
} catch (InvocationTargetException invocationException) {
138+
throw sthrow(invocationException.getCause());
139+
} catch (ReflectiveOperationException e) {
140+
throw new AssertionError(e);
141+
}
142+
}
143+
144+
@SuppressWarnings("unchecked")
145+
private static <T extends RuntimeException> T sthrow(Throwable t) throws T {
146+
throw (T) t;
147+
}
148+
149+
private record ClassLoaders(ClassLoader sdkLoader, ClassLoader truffleLoader) {
150+
}
151+
152+
private static ClassLoaders createContextClassLoaders() {
78153
final ProtectionDomain polyglotDomain = Engine.class.getProtectionDomain();
79154
Assume.assumeNotNull(polyglotDomain);
80155
Assume.assumeNotNull(polyglotDomain.getCodeSource());
@@ -97,15 +172,34 @@ public void sdkAndTruffleAPIInSeparateClassLoaders() throws Exception {
97172

98173
URLClassLoader sdkLoader = new URLClassLoader(new URL[]{collectionsURL, wordURL, nativeURL, polyglotURL}, parent);
99174
URLClassLoader truffleLoader = new URLClassLoader(new URL[]{truffleURL}, sdkLoader);
100-
Thread.currentThread().setContextClassLoader(truffleLoader);
175+
return new ClassLoaders(sdkLoader, truffleLoader);
176+
}
177+
178+
private static Object createEngineInContextClassLoader(ClassLoaders classLoaders) {
179+
Thread.currentThread().setContextClassLoader(classLoaders.truffleLoader);
180+
try {
181+
Class<?> engineClass = classLoaders.sdkLoader.loadClass(Engine.class.getName());
182+
return engineClass.getMethod("create").invoke(null);
183+
} catch (ReflectiveOperationException roe) {
184+
throw new AssertionError(roe);
185+
}
186+
}
101187

102-
Class<?> engineClass = sdkLoader.loadClass(Engine.class.getName());
103-
Object engine = engineClass.getMethod("create").invoke(null);
104-
assertNotNull("Engine has been created", engine);
188+
private static void close(Object engine) {
189+
ReflectionUtils.invoke(engine, "close");
105190
}
106191

107192
@After
108193
public void resetLoader() {
109194
Thread.currentThread().setContextClassLoader(loader);
110195
}
196+
197+
@Registration
198+
public static final class TestLanguage extends AbstractExecutableTestLanguage {
199+
200+
@Override
201+
protected Object execute(RootNode node, Env env, Object[] contextArguments, Object[] frameArguments) throws Exception {
202+
return true;
203+
}
204+
}
111205
}

‎truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/TruffleLogger.java‎

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -948,7 +948,6 @@ static final class LoggerCache {
948948
private final LoggerNode root;
949949
private final Set<ContextWeakReference> activeContexts;
950950
private Map<String, Level> effectiveLevels;
951-
private volatile Set<String> knownIds;
952951

953952
LoggerCache(Object loggerCacheSpi) {
954953
Objects.requireNonNull(loggerCacheSpi);
@@ -1047,26 +1046,13 @@ private TruffleLogger getOrCreateLogger(final String loggerName) {
10471046
}
10481047

10491048
private TruffleLogger getOrCreateLogger(final String id, final String loggerName) {
1050-
Set<String> ids = getKnownIds();
1051-
if (!ids.contains(id)) {
1052-
throw new IllegalArgumentException("Unknown language or instrument id " + id + ", known ids: " + String.join(", ", ids));
1049+
if (!LanguageAccessor.ENGINE.isKnownLoggerId(id)) {
1050+
throw new IllegalArgumentException("Unknown language or instrument id " + id + ", known ids: " + String.join(", ", LanguageAccessor.ENGINE.getKnownLoggerIds()));
10531051
}
10541052
final String globalLoggerId = loggerName == null || loggerName.isEmpty() ? id : id + '.' + loggerName;
10551053
return getOrCreateLogger(globalLoggerId);
10561054
}
10571055

1058-
private Set<String> getKnownIds() {
1059-
Set<String> result = knownIds;
1060-
if (result == null) {
1061-
result = new HashSet<>();
1062-
result.addAll(LanguageAccessor.engineAccess().getInternalIds());
1063-
result.addAll(LanguageAccessor.engineAccess().getLanguageIds());
1064-
result.addAll(LanguageAccessor.engineAccess().getInstrumentIds());
1065-
knownIds = result;
1066-
}
1067-
return result;
1068-
}
1069-
10701056
Object getSPI() {
10711057
return this.loggerCache;
10721058
}

‎truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/Accessor.java‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -554,6 +554,10 @@ public abstract Thread createThread(Object polyglotLanguageContext, Runnable run
554554

555555
public abstract LogRecord createLogRecord(Object loggerCache, Level level, String loggerName, String message, String className, String methodName, Object[] parameters, Throwable thrown);
556556

557+
public abstract boolean isKnownLoggerId(String id);
558+
559+
public abstract Collection<String> getKnownLoggerIds();
560+
557561
public abstract boolean isContextBoundLogger(Object loggerCache);
558562

559563
public abstract Object getOuterContext(Object polyglotContext);
@@ -607,12 +611,6 @@ public abstract Process createSubProcess(Object polyglotLanguageContext, List<St
607611

608612
public abstract ZoneId getTimeZone(Object polyglotLanguageContext);
609613

610-
public abstract Set<String> getLanguageIds();
611-
612-
public abstract Set<String> getInstrumentIds();
613-
614-
public abstract Set<String> getInternalIds();
615-
616614
public abstract String getUnparsedOptionValue(OptionValues optionValues, OptionKey<?> optionKey);
617615

618616
public abstract String getRelativePathInResourceRoot(TruffleFile truffleFile);

‎truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/EngineAccessor.java‎

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1434,6 +1434,21 @@ public LogRecord createLogRecord(Object loggerCache, Level level, String loggerN
14341434
return ((PolyglotLoggers.LoggerCache) loggerCache).createLogRecord(level, loggerName, message, className, methodName, parameters, thrown);
14351435
}
14361436

1437+
@Override
1438+
public boolean isKnownLoggerId(String id) {
1439+
return PolyglotLoggers.getInternalIds().contains(id) || LanguageCache.languages().containsKey(id) || InstrumentCache.load().containsKey(id);
1440+
}
1441+
1442+
@Override
1443+
public Collection<String> getKnownLoggerIds() {
1444+
List<String> ids = new ArrayList<>();
1445+
ids.addAll(PolyglotLoggers.getInternalIds());
1446+
ids.addAll(LanguageCache.languages().keySet());
1447+
ids.addAll(InstrumentCache.load().keySet());
1448+
Collections.sort(ids);
1449+
return ids;
1450+
}
1451+
14371452
@Override
14381453
public boolean isContextBoundLogger(Object loggerCache) {
14391454
return ((PolyglotLoggers.LoggerCache) loggerCache).isContextBoundLogger();
@@ -1464,25 +1479,6 @@ public Object getLoggerOwner(Object loggerCache) {
14641479
return ((PolyglotLoggers.LoggerCache) loggerCache).getOwner();
14651480
}
14661481

1467-
@Override
1468-
public Set<String> getLanguageIds() {
1469-
return LanguageCache.languages().keySet();
1470-
}
1471-
1472-
@Override
1473-
public Set<String> getInstrumentIds() {
1474-
Set<String> ids = new HashSet<>();
1475-
for (InstrumentCache cache : InstrumentCache.load()) {
1476-
ids.add(cache.getId());
1477-
}
1478-
return ids;
1479-
}
1480-
1481-
@Override
1482-
public Set<String> getInternalIds() {
1483-
return PolyglotLoggers.getInternalIds();
1484-
}
1485-
14861482
@Override
14871483
public Object asHostObject(Object languageContext, Object obj) {
14881484
PolyglotContextImpl context = ((PolyglotLanguageContext) languageContext).context;
@@ -2210,9 +2206,9 @@ public Collection<String> getResourceIds(String componentId) {
22102206
if (languageCache != null) {
22112207
return languageCache.getResourceIds();
22122208
}
2213-
for (InstrumentCache instrumentCache : InstrumentCache.load()) {
2214-
if (instrumentCache.getId().equals(componentId)) {
2215-
return instrumentCache.getResourceIds();
2209+
for (Map.Entry<String, InstrumentCache> entry : InstrumentCache.load().entrySet()) {
2210+
if (entry.getKey().equals(componentId)) {
2211+
return entry.getValue().getResourceIds();
22162212
}
22172213
}
22182214
throw new IllegalArgumentException(componentId);

‎truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/InstrumentCache.java‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
import java.util.Comparator;
4747
import java.util.HashMap;
4848
import java.util.HashSet;
49+
import java.util.LinkedHashMap;
4950
import java.util.List;
5051
import java.util.Map;
5152
import java.util.ServiceLoader;
@@ -64,8 +65,8 @@
6465
import org.graalvm.polyglot.SandboxPolicy;
6566

6667
final class InstrumentCache {
67-
private static final List<InstrumentCache> nativeImageCache = TruffleOptions.AOT ? new ArrayList<>() : null;
68-
private static Map<List<AbstractClassLoaderSupplier>, List<InstrumentCache>> runtimeCaches = new HashMap<>();
68+
private static final Map<String, InstrumentCache> nativeImageCache = TruffleOptions.AOT ? new LinkedHashMap<>() : null;
69+
private static Map<List<AbstractClassLoaderSupplier>, Map<String, InstrumentCache>> runtimeCaches = new HashMap<>();
6970

7071
private final String className;
7172
private final String id;
@@ -87,7 +88,7 @@ final class InstrumentCache {
8788
*/
8889
@SuppressWarnings("unused")
8990
private static void initializeNativeImageState(ClassLoader imageClassLoader) {
90-
nativeImageCache.addAll(doLoad(List.of(new StrongClassLoaderSupplier(imageClassLoader))));
91+
nativeImageCache.putAll(doLoad(List.of(new StrongClassLoaderSupplier(imageClassLoader))));
9192
}
9293

9394
/**
@@ -98,11 +99,7 @@ private static void initializeNativeImageState(ClassLoader imageClassLoader) {
9899
@SuppressWarnings("unused")
99100
private static Set<String> collectInstruments() {
100101
assert TruffleOptions.AOT : "Only supported during image generation";
101-
Set<String> res = new HashSet<>();
102-
for (InstrumentCache instrumentCache : nativeImageCache) {
103-
res.add(instrumentCache.id);
104-
}
105-
return res;
102+
return nativeImageCache.keySet();
106103
}
107104

108105
/**
@@ -134,13 +131,13 @@ boolean isInternal() {
134131
return internal;
135132
}
136133

137-
static List<InstrumentCache> load() {
134+
static Map<String, InstrumentCache> load() {
138135
if (TruffleOptions.AOT) {
139136
return nativeImageCache;
140137
}
141138
synchronized (InstrumentCache.class) {
142139
List<AbstractClassLoaderSupplier> classLoaders = EngineAccessor.locatorOrDefaultLoaders();
143-
List<InstrumentCache> cache = runtimeCaches.get(classLoaders);
140+
Map<String, InstrumentCache> cache = runtimeCaches.get(classLoaders);
144141
if (cache == null) {
145142
cache = doLoad(classLoaders);
146143
runtimeCaches.put(classLoaders, cache);
@@ -151,15 +148,15 @@ static List<InstrumentCache> load() {
151148

152149
static Collection<InstrumentCache> internalInstruments() {
153150
Set<InstrumentCache> result = new HashSet<>();
154-
for (InstrumentCache i : load()) {
151+
for (InstrumentCache i : load().values()) {
155152
if (i.isInternal()) {
156153
result.add(i);
157154
}
158155
}
159156
return result;
160157
}
161158

162-
static List<InstrumentCache> doLoad(List<AbstractClassLoaderSupplier> suppliers) {
159+
static Map<String, InstrumentCache> doLoad(List<AbstractClassLoaderSupplier> suppliers) {
163160
List<InstrumentCache> list = new ArrayList<>();
164161
Set<String> classNamesUsed = new HashSet<>();
165162
ClassLoader truffleClassLoader = InstrumentCache.class.getClassLoader();
@@ -186,7 +183,11 @@ static List<InstrumentCache> doLoad(List<AbstractClassLoaderSupplier> suppliers)
186183
forEach((p) -> loadInstrumentImpl(p, list, classNamesUsed, optionalResources));
187184
}
188185
list.sort(Comparator.comparing(InstrumentCache::getId));
189-
return list;
186+
Map<String, InstrumentCache> result = new LinkedHashMap<>();
187+
for (InstrumentCache cache : list) {
188+
result.put(cache.getId(), cache);
189+
}
190+
return result;
190191
}
191192

192193
private static Stream<? extends TruffleInstrumentProvider> loadProviders(ClassLoader loader) {

‎truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/InternalResourceCache.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -348,13 +348,13 @@ static boolean copyResourcesForNativeImage(Path target, String... components) th
348348
Collections.addAll(requiredComponentIds, components);
349349
Set<String> requiredLanguageIds = new HashSet<>(LanguageCache.languages().keySet());
350350
requiredLanguageIds.retainAll(requiredComponentIds);
351-
Set<String> requiredInstrumentIds = InstrumentCache.load().stream().map(InstrumentCache::getId).collect(Collectors.toSet());
351+
Set<String> requiredInstrumentIds = new HashSet<>(InstrumentCache.load().keySet());
352352
requiredInstrumentIds.retainAll(requiredComponentIds);
353353
requiredComponentIds.removeAll(requiredLanguageIds);
354354
requiredComponentIds.removeAll(requiredInstrumentIds);
355355
if (!requiredComponentIds.isEmpty()) {
356356
Set<String> installedComponents = new TreeSet<>(LanguageCache.languages().keySet());
357-
InstrumentCache.load().stream().map(InstrumentCache::getId).forEach(installedComponents::add);
357+
installedComponents.addAll(InstrumentCache.load().keySet());
358358
throw new IllegalArgumentException(String.format("Components with ids %s are not installed. Installed components are: %s.",
359359
String.join(", ", requiredComponentIds),
360360
String.join(", ", installedComponents)));
@@ -467,7 +467,7 @@ static <T extends Throwable> void walkAllResources(ResourcesVisitor<T> consumer)
467467
consumer.visit(language.getId(), language.getResources());
468468
}
469469
}
470-
for (InstrumentCache instrument : InstrumentCache.load()) {
470+
for (InstrumentCache instrument : InstrumentCache.load().values()) {
471471
Collection<InternalResourceCache> resources = instrument.getResources();
472472
if (!resources.isEmpty()) {
473473
consumer.visit(instrument.getId(), resources);

‎truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotEngineImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -975,7 +975,7 @@ private Map<String, PolyglotInstrument> initializeInstruments(Map<String, Instru
975975
return Collections.emptyMap();
976976
}
977977
Map<String, PolyglotInstrument> instruments = new LinkedHashMap<>();
978-
List<InstrumentCache> cachedInstruments = InstrumentCache.load();
978+
Collection<InstrumentCache> cachedInstruments = InstrumentCache.load().values();
979979
for (InstrumentCache instrumentCache : cachedInstruments) {
980980
PolyglotInstrument instrumentImpl = new PolyglotInstrument(this, instrumentCache);
981981
instrumentImpl.info = LANGUAGE.createInstrument(instrumentImpl, instrumentCache.getId(), instrumentCache.getName(), instrumentCache.getVersion());

0 commit comments

Comments
 (0)