Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GR-53712] Create subprocess argfiles in current working directory. #8860

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ci/common.jsonnet
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ local common_json = import "../common.json";
# Keep in sync with com.oracle.svm.hosted.NativeImageOptions#DEFAULT_ERROR_FILE_NAME
" (?P<filename>.+/svm_err_b_\\d+T\\d+\\.\\d+_pid\\d+\\.md)",
# Keep in sync with jdk.graal.compiler.test.SubprocessUtil#makeArgfile
" @(?P<filename>.*SubprocessUtil.*\\.argfile)",
"@(?P<filename>.*SubprocessUtil-argfiles.*\\.argfile)",
],
},

Expand Down
3 changes: 0 additions & 3 deletions compiler/mx.compiler/mx_compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -664,9 +664,6 @@ def compiler_gate_benchmark_runner(tasks, extraVMarguments=None, prefix='', task

# run benchmark with non default setup #
########################################
# ensure -Xbatch still works
with Task(prefix + 'DaCapo_pmd:BatchMode', tasks, tags=GraalTags.test, report=task_report_component) as t:
if t: _gate_dacapo('pmd', 1, benchVmArgs + ['-Xbatch'])

# Ensure benchmark counters still work but omit this test on
# fastdebug as benchmark counter threads may not produce
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,15 @@
import java.nio.file.DirectoryStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;

import org.graalvm.collections.EconomicMap;
import org.junit.Test;

import jdk.graal.compiler.debug.DebugOptions;
import jdk.graal.compiler.debug.DebugOptions.PrintGraphTarget;
import jdk.graal.compiler.debug.TTY;
import jdk.graal.compiler.options.OptionKey;
import jdk.graal.compiler.options.OptionValues;
import org.junit.Test;

/**
* Check that setting the dump path results in files ending up in the right directory with matching
Expand All @@ -52,7 +52,7 @@ public static Object snippet() {
@Test
public void testDump() throws Exception {
assumeManagementLibraryIsLoadable();
try (TemporaryDirectory temp = new TemporaryDirectory(Paths.get("."), "DumpPathTest")) {
try (TemporaryDirectory temp = new TemporaryDirectory(this, "DumpPathTest")) {
String[] extensions = new String[]{".cfg", ".bgv", ".graph-strings"};
EconomicMap<OptionKey<?>, Object> overrides = OptionValues.newOptionMap();
overrides.put(DebugOptions.DumpPath, temp.toString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,8 @@
*/
package jdk.graal.compiler.core.test;

import java.io.File;
import java.io.IOException;
import java.lang.reflect.Field;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Comparator;
import java.util.stream.Stream;

import org.junit.Assume;
import org.junit.Test;
Expand All @@ -51,8 +45,8 @@ public void createUniqueTest() throws Exception {
Assume.assumeFalse("If InaccessibleObjectException is thrown, skip the test, we are on JDK9", ex.getClass().getSimpleName().equals("InaccessibleObjectException"));
}
int maxFileNameLength = maxFileNameLengthField.getInt(null);
Path tmpDir = Files.createTempDirectory(Paths.get("."), "createUniqueTest");
try {
try (TemporaryDirectory temp = new TemporaryDirectory(this, "createUniqueTest")) {
Path tmpDir = temp.path;
for (boolean createDirectory : new boolean[]{true, false}) {
for (String ext : new String[]{"", ".bgv", ".graph-strings"}) {
for (int i = 0; i < maxFileNameLength + 5; i++) {
Expand All @@ -66,14 +60,6 @@ public void createUniqueTest() throws Exception {
}
}
}
} finally {
deleteTree(tmpDir);
}
}

private static void deleteTree(Path root) throws IOException {
try (Stream<Path> elems = Files.walk(root)) {
elems.sorted(Comparator.reverseOrder()).map(Path::toFile).forEach(File::delete);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
package jdk.graal.compiler.core.test;

import java.io.IOException;
import java.nio.file.Paths;
import java.util.Arrays;

import org.graalvm.collections.EconomicMap;
Expand Down Expand Up @@ -67,15 +66,15 @@ public void snippet02(int i) {
@SuppressWarnings("try")
@Test
public void testDump() throws IOException {
try (TTY.Filter suppressTTY = new TTY.Filter(); TemporaryDirectory temp = new TemporaryDirectory(Paths.get("."), "GraphDumpingTest")) {
try (TTY.Filter suppressTTY = new TTY.Filter(); TemporaryDirectory temp = new TemporaryDirectory(this, "GraphDumpingTest")) {
compileWithDumping("snippet01", temp);
}
}

@SuppressWarnings("try")
@Test
public void testInvalidNodeProperties() throws IOException {
try (TTY.Filter suppressTTY = new TTY.Filter(); TemporaryDirectory temp = new TemporaryDirectory(Paths.get("."), "GraphDumpingTest")) {
try (TTY.Filter suppressTTY = new TTY.Filter(); TemporaryDirectory temp = new TemporaryDirectory(this, "GraphDumpingTest")) {
StructuredGraph graph = compileWithDumping("snippet02", temp);

// introduce an invalid node with broken properties
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

import java.util.Optional;

import jdk.graal.compiler.debug.DebugOptions;
import org.junit.Assert;
import org.junit.Test;

Expand Down Expand Up @@ -72,7 +73,8 @@ OptionValues testOptions() {
@SuppressWarnings("try")
public void test01() {
try (AutoCloseable c = new TTY.Filter()) {
OptionValues opt = new OptionValues(testOptions(), GraalOptions.FullUnroll, false);
// Do not capture graphs for expected compilation failures.
OptionValues opt = new OptionValues(testOptions(), GraalOptions.FullUnroll, false, DebugOptions.DumpOnError, false);
test(opt, "snippet01");
Assert.fail("Should have detected that the phase in this class does not retain the mustNotSafepoint flag of a loop begin");
} catch (Throwable t) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ public static void testHelper(List<Probe> initialOutputProbes,
List<ZipProbe> initialZipProbes,
List<String> extraVmArgs,
String... mainClassAndArgs) throws IOException, InterruptedException {
final File dumpPath = new File(CompilationWrapperTest.class.getSimpleName() + "_" + System.currentTimeMillis()).getAbsoluteFile();
final Path dumpPath = getOutputDirectory(CompilationWrapperTest.class).resolve(CompilationWrapperTest.class.getSimpleName() + "_" + nowAsFileName());
List<String> vmArgs = withoutDebuggerArguments(getVMCommandLine());
vmArgs.removeIf(a -> a.startsWith("-Djdk.graal."));
vmArgs.remove("-esa");
Expand Down Expand Up @@ -284,7 +284,7 @@ public static void testHelper(List<Probe> initialOutputProbes,
Assert.assertTrue(line, m.find());
String diagnosticOutputZip = m.group(1);

List<String> dumpPathEntries = Arrays.asList(dumpPath.list());
List<String> dumpPathEntries = List.of(dumpPath.toFile().list());

File zip = new File(diagnosticOutputZip).getAbsoluteFile();
Assert.assertTrue(zip.toString(), zip.exists());
Expand Down Expand Up @@ -324,7 +324,7 @@ public void verify(ZipEntry entry, ZipFile file) throws IOException {
zip.delete();
}
} finally {
Path directory = dumpPath.toPath();
Path directory = dumpPath;
removeDirectory(directory);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
package jdk.graal.compiler.hotspot.test;

import java.io.IOException;
import java.nio.file.Paths;

import jdk.graal.compiler.api.directives.GraalDirectives;
import jdk.graal.compiler.core.test.GraalCompilerTest;
Expand Down Expand Up @@ -111,7 +110,7 @@ public void testProfileReplay() throws IOException {
foo(i, i % 4, use);
}
final ResolvedJavaMethod method = getResolvedJavaMethod("foo");
try (TemporaryDirectory temp = new TemporaryDirectory(Paths.get("."), "ProfileReplayTest")) {
try (TemporaryDirectory temp = new TemporaryDirectory(getClass(), "ProfileReplayTest")) {
OptionValues overrides = new OptionValues(getInitialOptions(), DebugOptions.DumpPath, temp.toString());
runInitialCompilation(method, overrides, jvmciRuntime, compiler);
runSanityCompilation(temp.toString(), method, overrides, jvmciRuntime, compiler);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,20 @@
*/
package jdk.graal.compiler.hotspot.test;

import jdk.graal.compiler.code.CompilationResult;
import jdk.graal.compiler.core.common.CompilationIdentifier;
import jdk.graal.compiler.core.test.GraalCompilerTest;
import jdk.graal.compiler.debug.DebugOptions;
import jdk.graal.compiler.nodes.StructuredGraph;
import jdk.graal.compiler.options.OptionValues;
import jdk.vm.ci.meta.ResolvedJavaMethod;
import org.junit.Assert;
import org.junit.Test;

import jdk.graal.compiler.api.directives.GraalDirectives;
import jdk.graal.compiler.core.test.SubprocessTest;
import jdk.graal.compiler.debug.TTY;

public class TestDoNotMoveAllocationIntrinsic extends SubprocessTest {
public class TestDoNotMoveAllocationIntrinsic extends GraalCompilerTest {

static Object O;

Expand Down Expand Up @@ -59,6 +65,13 @@ public static void snippet02Local() {
O = array;
}

@Override
protected CompilationResult compile(ResolvedJavaMethod installedCodeOwner, StructuredGraph graph, CompilationResult compilationResult, CompilationIdentifier compilationId, OptionValues options) {
// Do not capture graphs for expected compilation failures.
OptionValues newOptions = new OptionValues(options, DebugOptions.DumpOnError, false);
return super.compile(installedCodeOwner, graph, compilationResult, compilationId, newOptions);
}

@Test
@SuppressWarnings("try")
public void test02Local() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public class InvokerSignatureMismatchTest extends CustomizedBytecodePatternTest
@Test
public void test() throws Throwable {
List<String> args = withoutDebuggerArguments(getVMCommandLine());
try (TemporaryDirectory temp = new TemporaryDirectory(null, getClass().getSimpleName())) {
try (TemporaryDirectory temp = new TemporaryDirectory(this, getClass().getSimpleName())) {
args.add("--class-path=" + temp);
args.add("--patch-module=java.base=" + temp);
args.add("-XX:-TieredCompilation");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,16 @@
import java.io.PrintWriter;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.net.URISyntaxException;
import java.net.URL;
import java.nio.file.FileVisitResult;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.SimpleFileVisitor;
import java.nio.file.attribute.BasicFileAttributes;
import java.nio.file.attribute.FileAttribute;
import java.security.CodeSource;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
Expand Down Expand Up @@ -591,13 +594,61 @@ public static TestRule createTimeoutMillis(long milliseconds) {
return createTimeout(milliseconds, TimeUnit.MILLISECONDS);
}

/**
* Find the "mxbuild" directory in the file system path of {@code testClass} if possible.
*/
public static Path findMxBuildDirectory(Class<?> testClass) {
Class<?> tc = testClass == null ? GraalTest.class : testClass;
CodeSource codeSource = tc.getProtectionDomain().getCodeSource();
if (codeSource != null) {
URL location = codeSource.getLocation();
try {
Path path = Path.of(location.toURI());
for (Path c : path) {
if (c.toString().equals("mxbuild") && Files.isDirectory(c)) {
return c;
}
}
} catch (URISyntaxException e) {
// ignore
}
}
return null;
}

/**
* Gets the value of {@link Instant#now()} as a string suitable for use as a file name.
*/
public static String nowAsFileName() {
// Sanitize for Windows by replacing ':' with '_'
return String.valueOf(Instant.now()).replace(':', '_');
}

/**
* Gets the directory under which output for {@code testClass} should be generated.
*/
public static Path getOutputDirectory(Class<?> testClass) {
Path parent = findMxBuildDirectory(testClass);
if (parent == null) {
parent = Path.of("mxbuild");
if (!Files.isDirectory(parent)) {
parent = Path.of(".");
}
}
return parent.toAbsolutePath();
}

public static class TemporaryDirectory implements AutoCloseable {

public final Path path;
private IOException closeException;

public TemporaryDirectory(Path dir, String prefix, FileAttribute<?>... attrs) throws IOException {
path = Files.createTempDirectory(dir == null ? Paths.get(".") : dir, prefix, attrs);
public TemporaryDirectory(Class<?> testClass, String prefix, FileAttribute<?>... attrs) throws IOException {
path = Files.createTempDirectory(getOutputDirectory(testClass), prefix, attrs);
}

public TemporaryDirectory(GraalTest test, String prefix, FileAttribute<?>... attrs) throws IOException {
path = Files.createTempDirectory(getOutputDirectory(test.getClass()), prefix, attrs);
}

@Override
Expand Down
Loading