[GR-58670] Clear polyglot source cache more aggressively.

PullRequest: graal/18976

Author: jchalou

truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/source/SourceCacheTest.java

@@ -54,14 +54,21 @@
 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;
 import org.junit.Test;
 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<String, String> options, S
         }
     }
 
-    @TruffleLanguage.Registration
+    @TruffleLanguage.Registration(contextPolicy = TruffleLanguage.ContextPolicy.SHARED)
     static class SourceCacheTestLanguage extends TruffleLanguage<TruffleLanguage.Env> {
         static final String ID = TestUtils.getDefaultLanguageId(SourceCacheTestLanguage.class);
 
+        @Option(category = OptionCategory.USER, help = "Sharing Group.", stability = OptionStability.STABLE) //
+        static OptionKey<String> 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<Source> 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<Source> 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<String> 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());
+            }
+        }
+    }
 }

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<PolyglotSharingLayer> sharedLayers = new ArrayList<>();
 
+    private final ReferenceQueue<Source> deadSourcesQueue = new ReferenceQueue<>();
+
     private boolean runtimeInitialized;
 
     AbstractPolyglotHostService polyglotHostService; // effectively final after engine patching
@@ -456,6 +459,10 @@ void freeSharingLayer(PolyglotSharingLayer layer, PolyglotContextImpl context) {
         }
     }
 
+    ReferenceQueue<Source> getDeadSourcesQueue() {
+        return deadSourcesQueue;
+    }
+
     void ensureRuntimeInitialized(PolyglotContextImpl context) {
         assert Thread.holdsLock(lock);
 

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<PolyglotLanguage, OptionValuesImpl> 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);

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<Source> 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<Object> 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<Object> sources) {
     private final class WeakCache extends Cache {
 
         private final ConcurrentHashMap<WeakSourceKey, WeakCacheValue> sourceCache = new ConcurrentHashMap<>();
-        private final ReferenceQueue<Source> deadSources = new ReferenceQueue<>();
+        private final ReferenceQueue<Source> deadSources;
+        private final WeakReference<WeakCache> cacheRef;
+
+        WeakCache(ReferenceQueue<Source> 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<Object> 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<Source> 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<Source> {
 
         final SourceKey key;
+        final WeakReference<WeakCache> cacheRef;
 
-        WeakSourceKey(SourceKey key, Source value, ReferenceQueue<? super Source> q) {
+        WeakSourceKey(SourceKey key, Source value, WeakReference<WeakCache> cacheRef, ReferenceQueue<? super Source> q) {
             super(value, q);
+            this.cacheRef = cacheRef;
             this.key = key;
         }