Skip to content

Commit dc19964

Browse files
chumerchumer
committed
Cleanup minor inefficiencies during resource install.
1 parent 5ad07fd commit dc19964

File tree

3 files changed

+144
-126
lines changed

3 files changed

+144
-126
lines changed

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

Lines changed: 129 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,13 @@
4040
*/
4141
package com.oracle.truffle.api.test.polyglot;
4242

43+
import static com.oracle.truffle.api.test.polyglot.AbstractPolyglotTest.assertFails;
44+
import static org.junit.Assert.assertEquals;
45+
import static org.junit.Assert.assertFalse;
46+
import static org.junit.Assert.assertNotNull;
47+
import static org.junit.Assert.assertTrue;
48+
import static org.junit.Assume.assumeFalse;
49+
4350
import java.io.BufferedReader;
4451
import java.io.ByteArrayOutputStream;
4552
import java.io.IOException;
@@ -66,16 +73,6 @@
6673
import java.util.Set;
6774
import java.util.regex.Pattern;
6875

69-
import com.oracle.truffle.api.CompilerDirectives;
70-
import com.oracle.truffle.api.InstrumentInfo;
71-
import com.oracle.truffle.api.TruffleFile;
72-
import com.oracle.truffle.api.TruffleLanguage;
73-
import com.oracle.truffle.api.instrumentation.TruffleInstrument;
74-
import com.oracle.truffle.api.test.OSUtils;
75-
import com.oracle.truffle.api.test.ReflectionUtils;
76-
import com.oracle.truffle.api.test.SubprocessTestUtils;
77-
import com.oracle.truffle.api.test.common.TestUtils;
78-
import com.oracle.truffle.tck.tests.TruffleTestAssumptions;
7976
import org.graalvm.nativeimage.ImageInfo;
8077
import org.graalvm.polyglot.Context;
8178
import org.graalvm.polyglot.Engine;
@@ -84,18 +81,21 @@
8481
import org.junit.BeforeClass;
8582
import org.junit.Test;
8683

84+
import com.oracle.truffle.api.CompilerDirectives;
8785
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
86+
import com.oracle.truffle.api.InstrumentInfo;
8887
import com.oracle.truffle.api.InternalResource;
88+
import com.oracle.truffle.api.TruffleFile;
89+
import com.oracle.truffle.api.TruffleLanguage;
8990
import com.oracle.truffle.api.TruffleLanguage.Registration;
91+
import com.oracle.truffle.api.instrumentation.TruffleInstrument;
9092
import com.oracle.truffle.api.nodes.RootNode;
93+
import com.oracle.truffle.api.test.OSUtils;
94+
import com.oracle.truffle.api.test.ReflectionUtils;
95+
import com.oracle.truffle.api.test.SubprocessTestUtils;
9196
import com.oracle.truffle.api.test.common.AbstractExecutableTestLanguage;
92-
93-
import static org.junit.Assert.assertEquals;
94-
import static org.junit.Assert.assertFalse;
95-
import static org.junit.Assert.assertNotNull;
96-
import static org.junit.Assert.assertTrue;
97-
import static com.oracle.truffle.api.test.polyglot.AbstractPolyglotTest.assertFails;
98-
import static org.junit.Assume.assumeFalse;
97+
import com.oracle.truffle.api.test.common.TestUtils;
98+
import com.oracle.truffle.tck.tests.TruffleTestAssumptions;
9999

100100
public class InternalResourceTest {
101101

@@ -1113,111 +1113,131 @@ protected Object execute(RootNode node, Env env, Object[] contextArguments, Obje
11131113
}
11141114

11151115
@Test
1116-
public void testLogging() {
1116+
public void testLogging() throws IOException, InterruptedException {
11171117
TruffleTestAssumptions.assumeNotAOT();
1118-
PrintStream originalErr = System.err;
1119-
ByteArrayOutputStream testErr = new ByteArrayOutputStream();
1120-
System.setErr(new PrintStream(testErr));
1121-
System.setProperty("polyglotimpl.TraceInternalResources", "true");
1122-
try (Context context = Context.create()) {
1123-
String languageId = TestUtils.getDefaultLanguageId(TestLogging.class);
1124-
AbstractExecutableTestLanguage.execute(context, TestLogging.class);
1125-
String[] lines = testErr.toString().split("\n");
1126-
String line = findLine("^\\[engine\\]\\[resource\\].*" + languageId + "::" + SourcesResource.ID, lines);
1127-
assertNotNull(line);
1128-
line = findLine("^\\[engine\\]\\[resource\\].*" + languageId + "::" + LibraryResource.ID, lines);
1129-
assertNotNull(line);
1130-
} finally {
1131-
System.getProperties().remove("polyglotimpl.TraceInternalResources");
1132-
System.setErr(originalErr);
1133-
}
1118+
executeWithLogging(() -> {
1119+
PrintStream originalErr = System.err;
1120+
ByteArrayOutputStream testErr = new ByteArrayOutputStream();
1121+
System.setErr(new PrintStream(testErr));
1122+
try (Context context = Context.create()) {
1123+
String languageId = TestUtils.getDefaultLanguageId(TestLogging.class);
1124+
AbstractExecutableTestLanguage.execute(context, TestLogging.class);
1125+
String[] lines = testErr.toString().split("\n");
1126+
String line = findLine("^\\[engine\\]\\[resource\\].*" + languageId + "::" + SourcesResource.ID, lines);
1127+
assertNotNull(line);
1128+
line = findLine("^\\[engine\\]\\[resource\\].*" + languageId + "::" + LibraryResource.ID, lines);
1129+
assertNotNull(line);
1130+
} finally {
1131+
System.getProperties().remove("polyglotimpl.TraceInternalResources");
1132+
System.setErr(originalErr);
1133+
}
1134+
});
1135+
}
1136+
1137+
private static void executeWithLogging(Runnable action) throws IOException, InterruptedException {
1138+
SubprocessTestUtils.newBuilder(InternalResourceTest.class, action).prefixVmOption("-Dpolyglotimpl.TraceInternalResources=true").run();
11341139
}
11351140

11361141
@Test
1137-
public void testLoggingOverriddenCacheRoot() throws IOException {
1142+
public void testLoggingOverriddenCacheRoot() throws IOException, InterruptedException {
11381143
TruffleTestAssumptions.assumeNotAOT();
1139-
PrintStream originalErr = System.err;
1140-
ByteArrayOutputStream testErr = new ByteArrayOutputStream();
1141-
Path tmpDir = Files.createTempDirectory("resources");
1142-
String languageId = TestUtils.getDefaultLanguageId(TestLogging.class);
1143-
Engine.copyResources(tmpDir, languageId);
1144-
System.setErr(new PrintStream(testErr));
1145-
System.setProperty("polyglotimpl.TraceInternalResources", "true");
1146-
System.setProperty("polyglot.engine.resourcePath", tmpDir.toString());
1147-
try (Context context = Context.create()) {
1148-
AbstractExecutableTestLanguage.execute(context, TestLogging.class, tmpDir.toString());
1149-
String[] lines = testErr.toString().split("\n");
1150-
String line = findLine("^\\[engine\\]\\[resource\\].*" + languageId + "::" + SourcesResource.ID, lines);
1151-
assertNotNull(line);
1152-
assertTrue(line.contains(tmpDir.toString()));
1153-
line = findLine("^\\[engine\\]\\[resource\\].*" + languageId + "::" + LibraryResource.ID, lines);
1154-
assertNotNull(line);
1155-
assertTrue(line.contains(tmpDir.toString()));
1156-
} finally {
1157-
System.getProperties().remove("polyglot.engine.resourcePath");
1158-
System.getProperties().remove("polyglotimpl.TraceInternalResources");
1159-
System.setErr(originalErr);
1160-
}
1144+
executeWithLogging(() -> {
1145+
try {
1146+
PrintStream originalErr = System.err;
1147+
ByteArrayOutputStream testErr = new ByteArrayOutputStream();
1148+
Path tmpDir = Files.createTempDirectory("resources");
1149+
String languageId = TestUtils.getDefaultLanguageId(TestLogging.class);
1150+
Engine.copyResources(tmpDir, languageId);
1151+
System.setErr(new PrintStream(testErr));
1152+
System.setProperty("polyglot.engine.resourcePath", tmpDir.toString());
1153+
try (Context context = Context.create()) {
1154+
AbstractExecutableTestLanguage.execute(context, TestLogging.class, tmpDir.toString());
1155+
String[] lines = testErr.toString().split("\n");
1156+
String line = findLine("^\\[engine\\]\\[resource\\].*" + languageId + "::" + SourcesResource.ID, lines);
1157+
assertNotNull(line);
1158+
assertTrue(line.contains(tmpDir.toString()));
1159+
line = findLine("^\\[engine\\]\\[resource\\].*" + languageId + "::" + LibraryResource.ID, lines);
1160+
assertNotNull(line);
1161+
assertTrue(line.contains(tmpDir.toString()));
1162+
} finally {
1163+
System.getProperties().remove("polyglot.engine.resourcePath");
1164+
System.getProperties().remove("polyglotimpl.TraceInternalResources");
1165+
System.setErr(originalErr);
1166+
}
1167+
} catch (IOException e) {
1168+
throw new AssertionError(e);
1169+
}
1170+
});
11611171
}
11621172

11631173
@Test
1164-
public void testLoggingOverriddenComponentRoot() throws IOException {
1174+
public void testLoggingOverriddenComponentRoot() throws IOException, InterruptedException {
11651175
TruffleTestAssumptions.assumeNotAOT();
1166-
PrintStream originalErr = System.err;
1167-
ByteArrayOutputStream testErr = new ByteArrayOutputStream();
1168-
Path tmpDir = Files.createTempDirectory("resources");
1169-
String languageId = TestUtils.getDefaultLanguageId(TestLogging.class);
1170-
String overridePropName = "polyglot.engine.resourcePath." + languageId;
1171-
Path languageResources = tmpDir.resolve(languageId);
1172-
Engine.copyResources(tmpDir, languageId);
1173-
System.setErr(new PrintStream(testErr));
1174-
System.setProperty("polyglotimpl.TraceInternalResources", "true");
1175-
System.setProperty(overridePropName, languageResources.toString());
1176-
try (Context context = Context.create()) {
1177-
AbstractExecutableTestLanguage.execute(context, TestLogging.class);
1178-
String[] lines = testErr.toString().split("\n");
1179-
String line = findLine("^\\[engine\\]\\[resource\\].*" + languageId + "::" + SourcesResource.ID, lines);
1180-
assertNotNull(line);
1181-
assertTrue(line.contains(languageResources.toString()));
1182-
line = findLine("^\\[engine\\]\\[resource\\].*" + languageId + "::" + LibraryResource.ID, lines);
1183-
assertNotNull(line);
1184-
assertTrue(line.contains(languageResources.toString()));
1185-
} finally {
1186-
System.getProperties().remove(overridePropName);
1187-
System.getProperties().remove("polyglotimpl.TraceInternalResources");
1188-
System.setErr(originalErr);
1189-
delete(tmpDir);
1190-
}
1176+
executeWithLogging(() -> {
1177+
try {
1178+
PrintStream originalErr = System.err;
1179+
ByteArrayOutputStream testErr = new ByteArrayOutputStream();
1180+
Path tmpDir = Files.createTempDirectory("resources");
1181+
String languageId = TestUtils.getDefaultLanguageId(TestLogging.class);
1182+
String overridePropName = "polyglot.engine.resourcePath." + languageId;
1183+
Path languageResources = tmpDir.resolve(languageId);
1184+
Engine.copyResources(tmpDir, languageId);
1185+
System.setErr(new PrintStream(testErr));
1186+
System.setProperty(overridePropName, languageResources.toString());
1187+
try (Context context = Context.create()) {
1188+
AbstractExecutableTestLanguage.execute(context, TestLogging.class);
1189+
String[] lines = testErr.toString().split("\n");
1190+
String line = findLine("^\\[engine\\]\\[resource\\].*" + languageId + "::" + SourcesResource.ID, lines);
1191+
assertNotNull(line);
1192+
assertTrue(line.contains(languageResources.toString()));
1193+
line = findLine("^\\[engine\\]\\[resource\\].*" + languageId + "::" + LibraryResource.ID, lines);
1194+
assertNotNull(line);
1195+
assertTrue(line.contains(languageResources.toString()));
1196+
} finally {
1197+
System.getProperties().remove(overridePropName);
1198+
System.getProperties().remove("polyglotimpl.TraceInternalResources");
1199+
System.setErr(originalErr);
1200+
delete(tmpDir);
1201+
}
1202+
} catch (IOException e) {
1203+
throw new AssertionError(e);
1204+
}
1205+
});
11911206
}
11921207

11931208
@Test
1194-
public void testLoggingOverriddenResourceRoot() throws IOException {
1209+
public void testLoggingOverriddenResourceRoot() throws IOException, InterruptedException {
11951210
TruffleTestAssumptions.assumeNotAOT();
1196-
PrintStream originalErr = System.err;
1197-
ByteArrayOutputStream testErr = new ByteArrayOutputStream();
1198-
Path tmpDir = Files.createTempDirectory("resources");
1199-
String languageId = TestUtils.getDefaultLanguageId(TestLogging.class);
1200-
String overridePropName = "polyglot.engine.resourcePath." + languageId + '.' + SourcesResource.ID;
1201-
Engine.copyResources(tmpDir, languageId);
1202-
Path sourceResource = tmpDir.resolve(languageId).resolve(SourcesResource.ID);
1203-
System.setErr(new PrintStream(testErr));
1204-
System.setProperty("polyglotimpl.TraceInternalResources", "true");
1205-
System.setProperty(overridePropName, sourceResource.toString());
1206-
try (Context context = Context.create()) {
1207-
AbstractExecutableTestLanguage.execute(context, TestLogging.class);
1208-
String[] lines = testErr.toString().split("\n");
1209-
String line = findLine("^\\[engine\\]\\[resource\\].*" + languageId + "::" + SourcesResource.ID, lines);
1210-
assertNotNull(line);
1211-
assertTrue(line.contains(sourceResource.toString()));
1212-
line = findLine("^\\[engine\\]\\[resource\\].*" + languageId + "::" + LibraryResource.ID, lines);
1213-
assertNotNull(line);
1214-
assertFalse(line.contains(sourceResource.toString()));
1215-
} finally {
1216-
System.getProperties().remove(overridePropName);
1217-
System.getProperties().remove("polyglotimpl.TraceInternalResources");
1218-
System.setErr(originalErr);
1219-
delete(tmpDir);
1220-
}
1211+
executeWithLogging(() -> {
1212+
try {
1213+
PrintStream originalErr = System.err;
1214+
ByteArrayOutputStream testErr = new ByteArrayOutputStream();
1215+
Path tmpDir = Files.createTempDirectory("resources");
1216+
String languageId = TestUtils.getDefaultLanguageId(TestLogging.class);
1217+
String overridePropName = "polyglot.engine.resourcePath." + languageId + '.' + SourcesResource.ID;
1218+
Engine.copyResources(tmpDir, languageId);
1219+
Path sourceResource = tmpDir.resolve(languageId).resolve(SourcesResource.ID);
1220+
System.setErr(new PrintStream(testErr));
1221+
System.setProperty(overridePropName, sourceResource.toString());
1222+
try (Context context = Context.create()) {
1223+
AbstractExecutableTestLanguage.execute(context, TestLogging.class);
1224+
String[] lines = testErr.toString().split("\n");
1225+
String line = findLine("^\\[engine\\]\\[resource\\].*" + languageId + "::" + SourcesResource.ID, lines);
1226+
assertNotNull(line);
1227+
assertTrue(line.contains(sourceResource.toString()));
1228+
line = findLine("^\\[engine\\]\\[resource\\].*" + languageId + "::" + LibraryResource.ID, lines);
1229+
assertNotNull(line);
1230+
assertFalse(line.contains(sourceResource.toString()));
1231+
} finally {
1232+
System.getProperties().remove(overridePropName);
1233+
System.getProperties().remove("polyglotimpl.TraceInternalResources");
1234+
System.setErr(originalErr);
1235+
delete(tmpDir);
1236+
}
1237+
} catch (IOException e) {
1238+
throw new AssertionError(e);
1239+
}
1240+
});
12211241
}
12221242

12231243
@Test

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ void initializeOwningRoot(InternalResourceRoots.Root root) {
163163
case UNVERSIONED -> findStandaloneResourceRoot(root.path());
164164
case VERSIONED -> null;
165165
};
166-
if (path != null && InternalResourceRoots.isTraceInternalResourceEvents()) {
166+
if (path != null && InternalResourceRoots.TRACE_INTERNAL_RESOURCE_EVENTS) {
167167
/*
168168
* The path for the VERSIONED resource is logged when the resource is requested.
169169
* Computation of this path is expensive and involves a call to
@@ -254,7 +254,9 @@ private Path installResource(Function<InternalResource, InternalResource.Env> re
254254
}
255255
Path target = owningRoot.path().resolve(Path.of(sanitize(id), sanitize(resourceId), sanitize(versionHash)));
256256
if (!Files.exists(target)) {
257-
InternalResourceRoots.logInternalResourceEvent("Resolved a directory for the internal resource %s::%s to: %s, unpacking resource files.", id, resourceId, target);
257+
if (InternalResourceRoots.TRACE_INTERNAL_RESOURCE_EVENTS) {
258+
InternalResourceRoots.logInternalResourceEvent("Resolved a directory for the internal resource %s::%s to: %s, unpacking resource files.", id, resourceId, target);
259+
}
258260
Path parent = target.getParent();
259261
if (parent == null) {
260262
throw new AssertionError("Target must have a parent directory but was " + target);
@@ -280,8 +282,10 @@ private Path installResource(Function<InternalResource, InternalResource.Env> re
280282
}
281283
}
282284
} else {
283-
InternalResourceRoots.logInternalResourceEvent("Resolved a directory for the internal resource %s::%s to: %s, using existing resource files.",
284-
id, resourceId, target);
285+
if (InternalResourceRoots.TRACE_INTERNAL_RESOURCE_EVENTS) {
286+
InternalResourceRoots.logInternalResourceEvent("Resolved a directory for the internal resource %s::%s to: %s, using existing resource files.",
287+
id, resourceId, target);
288+
}
285289
verifyResourceRoot(target);
286290
}
287291
return target;

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

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ final class InternalResourceRoots {
6565
private static final String PROPERTY_RESOURCE_PATH = "polyglot.engine.resourcePath";
6666
private static final String PROPERTY_USER_RESOURCE_CACHE = "polyglot.engine.userResourceCache";
6767
private static final Map<Collection<EngineAccessor.AbstractClassLoaderSupplier>, InternalResourceRoots> runtimeCaches = new ConcurrentHashMap<>();
68+
static final boolean TRACE_INTERNAL_RESOURCE_EVENTS = Boolean.getBoolean("polyglotimpl.TraceInternalResources");
6869

6970
static String overriddenComponentRootProperty(String componentId) {
7071
StringBuilder builder = new StringBuilder(PROPERTY_RESOURCE_PATH);
@@ -252,8 +253,10 @@ private static Pair<Path, Root.Kind> findDefaultRoot() {
252253
root = findCacheRootDefault();
253254
kind = Root.Kind.VERSIONED;
254255
}
255-
logInternalResourceEvent("Resolved the root directory for the internal resource cache to: %s, determined by the %s with the value %s.",
256-
root.path(), root.hint(), root.hintValue());
256+
if (TRACE_INTERNAL_RESOURCE_EVENTS) {
257+
logInternalResourceEvent("Resolved the root directory for the internal resource cache to: %s, determined by the %s with the value %s.",
258+
root.path(), root.hint(), root.hintValue());
259+
}
257260
return Pair.create(root.path(), kind);
258261
}
259262

@@ -344,18 +347,9 @@ private static ResolvedCacheFolder findCacheRootDefault() {
344347
return container.resolve("org.graalvm.polyglot");
345348
}
346349

347-
static boolean isTraceInternalResourceEvents() {
348-
/*
349-
* Internal resources are utilized before the Engine is created; hence, we cannot leverage
350-
* engine options and engine logger.
351-
*/
352-
return Boolean.getBoolean("polyglotimpl.TraceInternalResources");
353-
}
354-
355350
static void logInternalResourceEvent(String message, Object... args) {
356-
if (isTraceInternalResourceEvents()) {
357-
PolyglotEngineImpl.logFallback(String.format("[engine][resource] " + message + "%n", args));
358-
}
351+
assert TRACE_INTERNAL_RESOURCE_EVENTS : "need to check for TRACE_INTERNAL_RESOURCE_EVENTS before use";
352+
PolyglotEngineImpl.logFallback(String.format("[engine][resource] " + message + "%n", args));
359353
}
360354

361355
private static void emitWarning(String message, Object... args) {

0 commit comments

Comments
 (0)