From 8dd003d3c5eb09affc6e2877ecd0cb76ceed077c Mon Sep 17 00:00:00 2001 From: Jakub Chaloupka Date: Tue, 8 Oct 2024 11:23:34 +0200 Subject: [PATCH] Clear polyglot source cache more aggressively. --- .../api/test/source/SourceCacheTest.java | 83 ++++++++++++++++++- .../truffle/polyglot/PolyglotEngineImpl.java | 9 +- .../polyglot/PolyglotSharingLayer.java | 3 +- .../truffle/polyglot/PolyglotSourceCache.java | 47 +++++++---- 4 files changed, 124 insertions(+), 18 deletions(-) diff --git a/truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/source/SourceCacheTest.java b/truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/source/SourceCacheTest.java index 12c3344d3125..7202c8ce392c 100644 --- a/truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/source/SourceCacheTest.java +++ b/truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/source/SourceCacheTest.java @@ -54,7 +54,13 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.graalvm.options.OptionCategory; +import org.graalvm.options.OptionDescriptors; +import org.graalvm.options.OptionKey; +import org.graalvm.options.OptionStability; +import org.graalvm.options.OptionValues; import org.graalvm.polyglot.Context; +import org.graalvm.polyglot.Engine; import org.graalvm.polyglot.PolyglotException; import org.graalvm.polyglot.Source; import org.junit.Assert; @@ -62,6 +68,7 @@ import org.junit.function.ThrowingRunnable; import com.oracle.truffle.api.CallTarget; +import com.oracle.truffle.api.Option; import com.oracle.truffle.api.TruffleLanguage; import com.oracle.truffle.api.exception.AbstractTruffleException; import com.oracle.truffle.api.frame.VirtualFrame; @@ -89,7 +96,7 @@ public void testTraceSourceCacheFailure() throws Throwable { @Test public void testTraceSourceCacheEviction() throws IOException { - TruffleTestAssumptions.assumeWeakEncapsulation(); + TruffleTestAssumptions.assumeWeakEncapsulation(); // Can't control GC in the isolate. try (ByteArrayOutputStream out = new ByteArrayOutputStream(); Context context = Context.newBuilder().option("engine.TraceSourceCache", "true").out(out).err(out).build()) { Source auxiliarySource = Source.newBuilder(SourceCacheTestLanguage.ID, "x", "AuxiliarySource").build(); String sourceName = "TestSource"; @@ -162,10 +169,13 @@ private static void testCommon(String languageId, Map options, S } } - @TruffleLanguage.Registration + @TruffleLanguage.Registration(contextPolicy = TruffleLanguage.ContextPolicy.SHARED) static class SourceCacheTestLanguage extends TruffleLanguage { static final String ID = TestUtils.getDefaultLanguageId(SourceCacheTestLanguage.class); + @Option(category = OptionCategory.USER, help = "Sharing Group.", stability = OptionStability.STABLE) // + static OptionKey SharingGroup = new OptionKey<>(""); + @Override protected Env createContext(Env env) { return env; @@ -181,6 +191,20 @@ public Object execute(VirtualFrame frame) { } }.getCallTarget(); } + + @Override + protected boolean areOptionsCompatible(OptionValues firstOptions, OptionValues newOptions) { + /* + * Forces creation of a new sharing layer for each context where the option SharingGroup + * has a different value. + */ + return firstOptions.get(SharingGroup).equals(newOptions.get(SharingGroup)); + } + + @Override + protected OptionDescriptors getOptionDescriptors() { + return new SourceCacheTestLanguageOptionDescriptors(); + } } @SuppressWarnings("serial") @@ -205,4 +229,59 @@ protected CallTarget parse(ParsingRequest request) throws Exception { throw new ParseException(); } } + + @Test + public void testSourceCachesCleared() throws IOException { + TruffleTestAssumptions.assumeWeakEncapsulation(); // Can't control GC in the isolate. + try (ByteArrayOutputStream out = new ByteArrayOutputStream(); Engine engine = Engine.newBuilder().option("engine.TraceSourceCache", "true").out(out).err(out).build()) { + String sourceName1; + String sourceName2; + Source source1 = Source.newBuilder(SourceCacheTestLanguage.ID, "1", sourceName1 = "TestSource1").build(); + Source source2 = Source.newBuilder(SourceCacheTestLanguage.ID, "2", sourceName2 = "TestSource2").build(); + try (Context context1 = Context.newBuilder().engine(engine).option(SourceCacheTestLanguage.ID + ".SharingGroup", "one").build()) { + String sourceHash1 = String.format("0x%08x", context1.eval(source1).asInt()); + String sourceHash2 = String.format("0x%08x", context1.eval(source2).asInt()); + /* + * context2 creates a separate sharing layer because of the option SharingGroup and + * the implementation of the method SourceCacheTestLanguage#areOptionCompatible. + */ + try (Context context2 = Context.newBuilder().engine(engine).option(SourceCacheTestLanguage.ID + ".SharingGroup", "two").build()) { + WeakReference souceRef2 = new WeakReference<>(source2); + source2 = null; + GCUtils.assertGc("Source 2 was not collected", souceRef2); + /* + * The following context2 eval is supposed to clear source2 from context1 + * layer's source cache. + */ + context2.eval(source1); + WeakReference souceRef1 = new WeakReference<>(source1); + source1 = null; + GCUtils.assertGc("Source 1 was not collected", souceRef1); + /* + * The following context2 close is supposed to clear source1 both from context1 + * layer's source cache and from context2 layer's source cache. + */ + } + List logs = new ArrayList<>(); + forEachLog(out.toByteArray(), (matcher) -> { + String logType = matcher.group(1); + String suffix; + String loggedHash = matcher.group(2); + String loggedName = matcher.group(3); + if (sourceName1.equals(loggedName)) { + Assert.assertEquals(sourceHash1, loggedHash); + suffix = "1"; + } else if (sourceName2.equals(loggedName)) { + Assert.assertEquals(sourceHash2, loggedHash); + suffix = "2"; + } else { + suffix = "Unknown"; + } + logs.add(logType + suffix); + }); + String[] expectedSequence = new String[]{"miss1", "miss2", "evict2", "miss1", "evict1", "evict1"}; + Assert.assertArrayEquals(expectedSequence, logs.toArray()); + } + } + } } diff --git a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotEngineImpl.java b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotEngineImpl.java index d6e27be24e0c..3d772c92b839 100644 --- a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotEngineImpl.java +++ b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotEngineImpl.java @@ -76,6 +76,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Predicate; import java.util.function.Supplier; @@ -124,6 +125,7 @@ import com.oracle.truffle.api.nodes.LanguageInfo; import com.oracle.truffle.api.nodes.Node; import com.oracle.truffle.api.nodes.RootNode; +import com.oracle.truffle.api.source.Source; import com.oracle.truffle.api.source.SourceSection; import com.oracle.truffle.polyglot.PolyglotContextConfig.FileSystemConfig; import com.oracle.truffle.polyglot.PolyglotContextConfig.PreinitConfig; @@ -135,7 +137,6 @@ import com.oracle.truffle.polyglot.PolyglotLoggers.EngineLoggerProvider; import com.oracle.truffle.polyglot.PolyglotLoggers.LoggerCache; import com.oracle.truffle.polyglot.SystemThread.InstrumentSystemThread; -import java.util.function.Consumer; /** The implementation of {@link org.graalvm.polyglot.Engine}, stored in the receiver field. */ final class PolyglotEngineImpl implements com.oracle.truffle.polyglot.PolyglotImpl.VMObject { @@ -239,6 +240,8 @@ final class PolyglotEngineImpl implements com.oracle.truffle.polyglot.PolyglotIm final List sharedLayers = new ArrayList<>(); + private final ReferenceQueue deadSourcesQueue = new ReferenceQueue<>(); + private boolean runtimeInitialized; AbstractPolyglotHostService polyglotHostService; // effectively final after engine patching @@ -456,6 +459,10 @@ void freeSharingLayer(PolyglotSharingLayer layer, PolyglotContextImpl context) { } } + ReferenceQueue getDeadSourcesQueue() { + return deadSourcesQueue; + } + void ensureRuntimeInitialized(PolyglotContextImpl context) { assert Thread.holdsLock(lock); diff --git a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotSharingLayer.java b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotSharingLayer.java index 2036ce68cd98..bbc24404ade6 100644 --- a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotSharingLayer.java +++ b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotSharingLayer.java @@ -113,7 +113,7 @@ static final class Shared { int claimedCount; private Shared(PolyglotEngineImpl engine, ContextPolicy contextPolicy, Map previousLanguageOptions) { - this.sourceCache = new PolyglotSourceCache(TracingSourceCacheListener.createOrNull(engine)); + this.sourceCache = new PolyglotSourceCache(engine.getDeadSourcesQueue(), TracingSourceCacheListener.createOrNull(engine)); this.contextPolicy = contextPolicy; this.instances = new PolyglotLanguageInstance[engine.languageCount]; this.previousLanguageOptions = previousLanguageOptions; @@ -340,6 +340,7 @@ public void freeSharingLayer(PolyglotContextImpl context) { assert isClaimed(); shared.claimedCount--; + shared.sourceCache.cleanupStaleEntries(); if (engine.getEngineOptionValues().get(PolyglotEngineOptions.TraceCodeSharing)) { traceFreeLayer(context); diff --git a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotSourceCache.java b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotSourceCache.java index 8b502ddb4ed1..33f50b010f60 100644 --- a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotSourceCache.java +++ b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotSourceCache.java @@ -56,13 +56,13 @@ final class PolyglotSourceCache { - private final Cache strongCache; - private final Cache weakCache; + private final StrongCache strongCache; + private final WeakCache weakCache; private SourceCacheListener sourceCacheListener; - PolyglotSourceCache(SourceCacheListener sourceCacheListener) { + PolyglotSourceCache(ReferenceQueue deadSourcesQueue, TracingSourceCacheListener sourceCacheListener) { this.sourceCacheListener = sourceCacheListener; - this.weakCache = new WeakCache(); + this.weakCache = new WeakCache(deadSourcesQueue); this.strongCache = new StrongCache(); } @@ -103,6 +103,10 @@ void listCachedSources(PolyglotImpl polyglot, Collection source) { weakCache.listSources(polyglot, source); } + void cleanupStaleEntries() { + WeakCache.cleanupStaleEntries(weakCache.deadSources, sourceCacheListener); + } + private static CallTarget parseImpl(PolyglotLanguageContext context, String[] argumentNames, Source source) { validateSource(context, source); CallTarget parsedTarget = LANGUAGE.parse(context.requireEnv(), source, null, argumentNames); @@ -256,14 +260,20 @@ void listSources(PolyglotImpl polyglot, Collection sources) { private final class WeakCache extends Cache { private final ConcurrentHashMap sourceCache = new ConcurrentHashMap<>(); - private final ReferenceQueue deadSources = new ReferenceQueue<>(); + private final ReferenceQueue deadSources; + private final WeakReference cacheRef; + + WeakCache(ReferenceQueue deadSources) { + this.deadSources = deadSources; + this.cacheRef = new WeakReference<>(this); + } @Override CallTarget lookup(PolyglotLanguageContext context, Source source, String[] argumentNames, boolean parse) { - cleanupStaleEntries(); + cleanupStaleEntries(deadSources, sourceCacheListener); Object sourceId = EngineAccessor.SOURCE.getSourceIdentifier(source); Source sourceValue = EngineAccessor.SOURCE.copySource(source); - WeakSourceKey ref = new WeakSourceKey(new SourceKey(sourceId, argumentNames), source, deadSources); + WeakSourceKey ref = new WeakSourceKey(new SourceKey(sourceId, argumentNames), source, cacheRef, deadSources); WeakCacheValue value = sourceCache.get(ref); if (value == null) { if (parse) { @@ -308,18 +318,25 @@ boolean isEmpty() { @Override void listSources(PolyglotImpl polyglot, Collection sources) { - cleanupStaleEntries(); + cleanupStaleEntries(deadSources, sourceCacheListener); for (WeakCacheValue value : sourceCache.values()) { sources.add(PolyglotImpl.getOrCreatePolyglotSource(polyglot, value.source)); } } - private void cleanupStaleEntries() { + /** + * This is deliberately a static method, because the dead sources queue is + * {@link PolyglotEngineImpl#getDeadSourcesQueue() shared} among all weak caches on a + * particular polyglot engine, and so the elements of {@link WeakCache#deadSources the + * queue} may {@link WeakSourceKey#cacheRef refer} to different weak caches. + */ + private static void cleanupStaleEntries(ReferenceQueue deadSourcesQueue, SourceCacheListener cacheListener) { WeakSourceKey sourceRef; - while ((sourceRef = (WeakSourceKey) deadSources.poll()) != null) { - WeakCacheValue value = sourceCache.remove(sourceRef); - if (value != null && sourceCacheListener != null) { - sourceCacheListener.onCacheEvict(value.source, value.target, SourceCacheListener.CacheType.WEAK, value.hits.get()); + while ((sourceRef = (WeakSourceKey) deadSourcesQueue.poll()) != null) { + WeakCache cache = sourceRef.cacheRef.get(); + WeakCacheValue value = cache != null ? cache.sourceCache.remove(sourceRef) : null; + if (value != null && cacheListener != null) { + cacheListener.onCacheEvict(value.source, value.target, SourceCacheListener.CacheType.WEAK, value.hits.get()); } } } @@ -373,9 +390,11 @@ public boolean equals(Object obj) { private static final class WeakSourceKey extends WeakReference { final SourceKey key; + final WeakReference cacheRef; - WeakSourceKey(SourceKey key, Source value, ReferenceQueue q) { + WeakSourceKey(SourceKey key, Source value, WeakReference cacheRef, ReferenceQueue q) { super(value, q); + this.cacheRef = cacheRef; this.key = key; }