Skip to content

Commit d9ad51c

Browse files
committed
[GR-10706] Remove WeakHashMap from TruffleLogger.
PullRequest: graal/1774
2 parents 66bf5fe + 7d84c2d commit d9ad51c

File tree

5 files changed

+161
-43
lines changed

5 files changed

+161
-43
lines changed

truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/polyglot/LoggingTest.java

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@
3535
import com.oracle.truffle.api.nodes.RootNode;
3636
import java.io.ByteArrayOutputStream;
3737
import java.io.IOException;
38+
import java.lang.ref.Reference;
39+
import java.lang.ref.WeakReference;
3840
import java.util.AbstractMap;
3941
import java.util.ArrayList;
4042
import java.util.Arrays;
@@ -437,6 +439,49 @@ public boolean test(final LoggingContext context, final Collection<TruffleLogger
437439
}
438440
}
439441

442+
@Test
443+
public void testGcedContext() {
444+
TestHandler handler = new TestHandler();
445+
Context gcedContext = Context.newBuilder().options(createLoggingOptions(LoggingLanguageFirst.ID, null, Level.FINEST.toString())).logHandler(handler).build();
446+
gcedContext.eval(LoggingLanguageFirst.ID, "");
447+
List<Map.Entry<Level, String>> expected = new ArrayList<>();
448+
expected.addAll(createExpectedLog(LoggingLanguageFirst.ID, Level.FINEST, Collections.emptyMap()));
449+
Assert.assertEquals(expected, handler.getLog());
450+
Reference<Context> gcedContextRef = new WeakReference<>(gcedContext);
451+
gcedContext = null;
452+
assertGc("Cannot free context.", gcedContextRef);
453+
handler = new TestHandler();
454+
Context newContext = Context.newBuilder().logHandler(handler).build();
455+
newContext.eval(LoggingLanguageFirst.ID, "");
456+
expected = new ArrayList<>();
457+
expected.addAll(createExpectedLog(LoggingLanguageFirst.ID, Level.INFO, Collections.emptyMap()));
458+
Assert.assertEquals(expected, handler.getLog());
459+
}
460+
461+
@Test
462+
public void testGcedContext2() {
463+
TestHandler gcedContextHandler = new TestHandler();
464+
Context gcedContext = Context.newBuilder().options(createLoggingOptions(LoggingLanguageFirst.ID, null, Level.FINEST.toString())).logHandler(gcedContextHandler).build();
465+
TestHandler contextHandler = new TestHandler();
466+
Context context = Context.newBuilder().options(createLoggingOptions(LoggingLanguageFirst.ID, null, Level.FINE.toString())).logHandler(contextHandler).build();
467+
gcedContext.eval(LoggingLanguageFirst.ID, "");
468+
List<Map.Entry<Level, String>> expected = new ArrayList<>();
469+
expected.addAll(createExpectedLog(LoggingLanguageFirst.ID, Level.FINEST, Collections.emptyMap()));
470+
Assert.assertEquals(expected, gcedContextHandler.getLog());
471+
context.eval(LoggingLanguageFirst.ID, "");
472+
expected = new ArrayList<>();
473+
expected.addAll(createExpectedLog(LoggingLanguageFirst.ID, Level.FINE, Collections.emptyMap()));
474+
Assert.assertEquals(expected, contextHandler.getLog());
475+
Reference<Context> gcedContextRef = new WeakReference<>(gcedContext);
476+
gcedContext = null;
477+
assertGc("Cannot free context.", gcedContextRef);
478+
contextHandler.clear();
479+
context.eval(LoggingLanguageFirst.ID, "");
480+
expected = new ArrayList<>();
481+
expected.addAll(createExpectedLog(LoggingLanguageFirst.ID, Level.FINE, Collections.emptyMap()));
482+
Assert.assertEquals(expected, contextHandler.getLog());
483+
}
484+
440485
private static void assertImmutable(final LogRecord r) {
441486
try {
442487
r.setLevel(Level.FINEST);
@@ -559,6 +604,38 @@ private static LoggerNode createLevelsTree(Map<String, Level> levels) {
559604
return root;
560605
}
561606

607+
private static void assertGc(final String message, final Reference<?> ref) {
608+
int blockSize = 100_000;
609+
final List<byte[]> blocks = new ArrayList<>();
610+
for (int i = 0; i < 50; i++) {
611+
if (ref.get() == null) {
612+
return;
613+
}
614+
try {
615+
System.gc();
616+
} catch (OutOfMemoryError oom) {
617+
}
618+
try {
619+
System.runFinalization();
620+
} catch (OutOfMemoryError oom) {
621+
}
622+
try {
623+
blocks.add(new byte[blockSize]);
624+
blockSize = (int) (blockSize * 1.3);
625+
} catch (OutOfMemoryError oom) {
626+
blockSize >>>= 1;
627+
}
628+
if (i % 10 == 0) {
629+
try {
630+
Thread.sleep(500);
631+
} catch (InterruptedException ie) {
632+
break;
633+
}
634+
}
635+
}
636+
Assert.fail(message);
637+
}
638+
562639
public static final class LoggingContext {
563640
private final TruffleLanguage.Env env;
564641

truffle/src/com.oracle.truffle.api.vm/src/com/oracle/truffle/api/vm/PolyglotEngine.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1524,5 +1524,10 @@ public LogRecord createLogRecord(Level level, String loggerName, String message,
15241524
public Object getCurrentOuterContext() {
15251525
return null;
15261526
}
1527+
1528+
@Override
1529+
public Map<String, Level> getLogLevels(Object context) {
1530+
return Collections.emptyMap();
1531+
}
15271532
}
15281533
}

truffle/src/com.oracle.truffle.api.vm/src/com/oracle/truffle/api/vm/PolyglotImpl.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -955,5 +955,13 @@ public LogRecord createLogRecord(Level level, String loggerName, String message,
955955
public Object getCurrentOuterContext() {
956956
return PolyglotLogHandler.getCurrentOuterContext();
957957
}
958+
959+
@Override
960+
public Map<String, Level> getLogLevels(final Object context) {
961+
if (!(context instanceof PolyglotContextImpl)) {
962+
throw new AssertionError();
963+
}
964+
return ((PolyglotContextImpl) context).config.logLevels;
965+
}
958966
}
959967
}

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

Lines changed: 69 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@
3737
import java.util.Map;
3838
import java.util.Objects;
3939
import java.util.Set;
40-
import java.util.WeakHashMap;
4140
import java.util.concurrent.atomic.AtomicBoolean;
4241
import java.util.function.Supplier;
4342
import java.util.logging.Handler;
@@ -926,62 +925,57 @@ public void close() {
926925
}
927926

928927
static final class LoggerCache {
928+
private static final ReferenceQueue<Object> contextsRefQueue = new ReferenceQueue<>();
929929
private static final LoggerCache INSTANCE = new LoggerCache();
930930
private final TruffleLogger polyglotRootLogger;
931931
private final Map<String, NamedLoggerRef> loggers;
932932
private final LoggerNode root;
933-
private final Map<Object, Map<String, Level>> levelsByContext;
933+
private final Set<ContextWeakReference> activeContexts;
934934
private Map<String, Level> effectiveLevels;
935935

936936
private LoggerCache() {
937937
this.polyglotRootLogger = new TruffleLogger();
938938
this.loggers = new HashMap<>();
939939
this.loggers.put(ROOT_NAME, new NamedLoggerRef(this.polyglotRootLogger, ROOT_NAME));
940940
this.root = new LoggerNode(null, new NamedLoggerRef(this.polyglotRootLogger, ROOT_NAME));
941-
this.levelsByContext = new WeakHashMap<>();
941+
this.activeContexts = new HashSet<>();
942942
this.effectiveLevels = Collections.emptyMap();
943943
}
944944

945-
void addLogLevelsForContext(final Object context, final Map<String, Level> addedLevels) {
946-
synchronized (this) {
947-
levelsByContext.put(context, addedLevels);
948-
final Collection<String> removedLevels = new HashSet<>();
949-
final Collection<String> changedLevels = new HashSet<>();
950-
effectiveLevels = computeEffectiveLevels(
951-
effectiveLevels,
952-
Collections.emptySet(),
953-
addedLevels,
954-
levelsByContext,
955-
removedLevels,
956-
changedLevels);
957-
reconfigure(removedLevels, changedLevels);
958-
}
945+
synchronized void addLogLevelsForContext(final Object context, final Map<String, Level> addedLevels) {
946+
activeContexts.add(new ContextWeakReference(context, contextsRefQueue, addedLevels));
947+
final Set<String> toRemove = collectRemovedLevels();
948+
reconfigure(addedLevels, toRemove);
959949
}
960950

961951
synchronized void removeLogLevelsForContext(final Object context) {
962-
final Map<String, Level> levels = levelsByContext.remove(context);
963-
final Collection<String> removedLevels = new HashSet<>();
964-
final Collection<String> changedLevels = new HashSet<>();
965-
effectiveLevels = computeEffectiveLevels(
966-
effectiveLevels,
967-
levels.keySet(),
968-
Collections.emptyMap(),
969-
levelsByContext,
970-
removedLevels,
971-
changedLevels);
972-
reconfigure(removedLevels, changedLevels);
952+
final Set<String> toRemove = collectRemovedLevels();
953+
for (Iterator<ContextWeakReference> it = activeContexts.iterator(); it.hasNext();) {
954+
final Object activeContext = it.next().get();
955+
if (context.equals(activeContext)) {
956+
toRemove.addAll(TruffleLanguage.AccessAPI.engineAccess().getLogLevels(context).keySet());
957+
it.remove();
958+
break;
959+
}
960+
}
961+
reconfigure(Collections.emptyMap(), toRemove);
973962
}
974963

975964
synchronized boolean isLoggable(final String loggerName, final Object currentContext, final Level level) {
976-
final Map<String, Level> current = levelsByContext.get(currentContext);
977-
if (current == null) {
965+
final Set<String> toRemove = collectRemovedLevels();
966+
if (!toRemove.isEmpty()) {
967+
reconfigure(Collections.emptyMap(), toRemove);
968+
// Logger's effective level may changed
969+
return getLogger(loggerName).isLoggable(level);
970+
}
971+
final Map<String, Level> current = TruffleLanguage.AccessAPI.engineAccess().getLogLevels(currentContext);
972+
if (current.isEmpty()) {
978973
final int currentLevel = DEFAULT_VALUE;
979974
return level.intValue() >= currentLevel && currentLevel != OFF_VALUE;
980975
}
981-
// GR-10762 does not work if contexts are GCed
982-
// if (levelsByContext.size() == 1) {
983-
// return true;
984-
// }
976+
if (activeContexts.size() == 1) {
977+
return true;
978+
}
985979
final int currentLevel = Math.min(computeLevel(loggerName, current), DEFAULT_VALUE);
986980
return level.intValue() >= currentLevel && currentLevel != OFF_VALUE;
987981
}
@@ -1064,14 +1058,36 @@ private Level getEffectiveLevel(final String loggerName) {
10641058
return effectiveLevels.get(loggerName);
10651059
}
10661060

1067-
private void reconfigure(final Collection<? extends String> removedLoggers, final Collection<? extends String> changedLoogers) {
1068-
for (String loggerName : removedLoggers) {
1061+
private Set<String> collectRemovedLevels() {
1062+
assert Thread.holdsLock(this);
1063+
final Set<String> toRemove = new HashSet<>();
1064+
ContextWeakReference ref;
1065+
while ((ref = (ContextWeakReference) contextsRefQueue.poll()) != null) {
1066+
activeContexts.remove(ref);
1067+
toRemove.addAll(ref.configuredLoggerNames);
1068+
}
1069+
return toRemove;
1070+
}
1071+
1072+
private void reconfigure(final Map<String, Level> addedLevels, final Set<String> toRemove) {
1073+
assert Thread.holdsLock(this);
1074+
assert !addedLevels.isEmpty() || !toRemove.isEmpty();
1075+
final Collection<String> loggersWithRemovedLevels = new HashSet<>();
1076+
final Collection<String> loggersWithChangedLevels = new HashSet<>();
1077+
effectiveLevels = computeEffectiveLevels(
1078+
effectiveLevels,
1079+
toRemove,
1080+
addedLevels,
1081+
activeContexts,
1082+
loggersWithRemovedLevels,
1083+
loggersWithChangedLevels);
1084+
for (String loggerName : loggersWithRemovedLevels) {
10691085
final TruffleLogger logger = getLogger(loggerName);
10701086
if (logger != null) {
10711087
logger.setLevel(null);
10721088
}
10731089
}
1074-
for (String loggerName : changedLoogers) {
1090+
for (String loggerName : loggersWithChangedLevels) {
10751091
final TruffleLogger logger = getLogger(loggerName);
10761092
if (logger != null) {
10771093
setLoggerLevel(logger, loggerName);
@@ -1137,18 +1153,18 @@ private static Map<String, Level> computeEffectiveLevels(
11371153
final Map<String, Level> currentEffectiveLevels,
11381154
final Set<String> removed,
11391155
final Map<String, Level> added,
1140-
final Map<Object, Map<String, Level>> levelsByContext,
1156+
final Collection<? extends Reference<Object>> contexts,
11411157
final Collection<? super String> removedLevels,
11421158
final Collection<? super String> changedLevels) {
11431159
final Map<String, Level> newEffectiveLevels = new HashMap<>(currentEffectiveLevels);
11441160
for (String loggerName : removed) {
1145-
final Level level = findMinLevel(loggerName, levelsByContext);
1161+
final Level level = findMinLevel(loggerName, contexts);
11461162
if (level == null) {
11471163
newEffectiveLevels.remove(loggerName);
11481164
removedLevels.add(loggerName);
11491165
} else {
11501166
final Level currentLevel = newEffectiveLevels.get(loggerName);
1151-
if (min(level, currentLevel) != currentLevel) {
1167+
if (currentLevel != level) {
11521168
newEffectiveLevels.put(loggerName, level);
11531169
changedLevels.add(loggerName);
11541170
}
@@ -1166,10 +1182,11 @@ private static Map<String, Level> computeEffectiveLevels(
11661182
return newEffectiveLevels;
11671183
}
11681184

1169-
private static Level findMinLevel(final String loggerName, final Map<Object, Map<String, Level>> levelsByContext) {
1185+
private static Level findMinLevel(final String loggerName, final Collection<? extends Reference<Object>> contexts) {
11701186
Level min = null;
1171-
for (Map<String, Level> levels : levelsByContext.values()) {
1172-
Level level = levels.get(loggerName);
1187+
for (Reference<Object> contextRef : contexts) {
1188+
final Object context = contextRef.get();
1189+
final Level level = context == null ? null : TruffleLanguage.AccessAPI.engineAccess().getLogLevels(context).get(loggerName);
11731190
if (level == null) {
11741191
continue;
11751192
}
@@ -1259,6 +1276,15 @@ private void updateChildParentsImpl(final TruffleLogger parentLogger) {
12591276
}
12601277
}
12611278
}
1279+
1280+
private static final class ContextWeakReference extends WeakReference<Object> {
1281+
private final Set<String> configuredLoggerNames;
1282+
1283+
ContextWeakReference(final Object context, final ReferenceQueue<Object> referenceQueue, final Map<String, Level> logLevels) {
1284+
super(context, referenceQueue);
1285+
configuredLoggerNames = logLevels.keySet();
1286+
}
1287+
}
12621288
}
12631289

12641290
private static final class PolyglotLogHandlerProvider implements Supplier<Handler> {

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,8 @@ public final void detachOutputConsumer(DispatchOutputStream dos, OutputStream ou
318318

319319
public abstract Handler getLogHandler();
320320

321+
public abstract Map<String, Level> getLogLevels(Object context);
322+
321323
public abstract LogRecord createLogRecord(Level level, String loggerName, String message, String className, String methodName, Object[] parameters, Throwable thrown);
322324

323325
public abstract Object getCurrentOuterContext();

0 commit comments

Comments
 (0)