Skip to content

Commit

Permalink
Clear polyglot source cache more aggressively.
Browse files Browse the repository at this point in the history
  • Loading branch information
@jchalou
jchalou committed Oct 17, 2024
1 parent 3ee833a commit 8dd003d
Show file tree
Hide file tree
Showing 4 changed files with 124 additions and 18 deletions.
83 changes: 81 additions & 2 deletions ...c/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/source/SourceCacheTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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";
Expand Down Expand Up @@ -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;
Expand All @@ -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")
Expand All @@ -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());
}
}
}
}
9 changes: 8 additions & 1 deletion ...e/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotEngineImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -456,6 +459,10 @@ void freeSharingLayer(PolyglotSharingLayer layer, PolyglotContextImpl context) {
}
}

ReferenceQueue<Source> getDeadSourcesQueue() {
return deadSourcesQueue;
}

void ensureRuntimeInitialized(PolyglotContextImpl context) {
assert Thread.holdsLock(lock);

Expand Down
3 changes: 2 additions & 1 deletion ...src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotSharingLayer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -340,6 +340,7 @@ public void freeSharingLayer(PolyglotContextImpl context) {
assert isClaimed();

shared.claimedCount--;
shared.sourceCache.cleanupStaleEntries();

if (engine.getEngineOptionValues().get(PolyglotEngineOptions.TraceCodeSharing)) {
traceFreeLayer(context);
Expand Down
47 changes: 33 additions & 14 deletions .../src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotSourceCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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());
}
}
}
Expand Down Expand Up @@ -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;
}

Expand Down

0 comments on commit 8dd003d

Please sign in to comment.