Skip to content

Speed up the gradle test invocation #37

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

Speed up the gradle test invocation #37

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b72e98b
Speed up the gradle test execution
andreaTP Nov 5, 2021
3e715f6
minor cleanup
andreaTP Nov 5, 2021
1cd917c
fix
andreaTP Nov 9, 2021
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
9 changes: 7 additions & 2 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# === Build builder image ===

FROM gradle:6.8.3-jdk11 AS build
FROM gradle:7.1-jdk11 AS build
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Align version with the version used in the exercises


WORKDIR /home/builder

Expand All @@ -20,7 +20,7 @@ RUN gradle build

# === Build runtime image ===

FROM gradle:6.8.3-jdk11
FROM gradle:7.1-jdk11
WORKDIR /opt/test-runner

# Copy binary and launcher script
Expand All @@ -30,4 +30,9 @@ COPY --from=build /home/builder/autotest-runner.jar ./
# Copy cached dependencies
COPY --from=build /home/gradle /home/gradle

RUN pkill -f '.*GradleDaemon.*'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is one stopped gradle daemon in the image, those two lines remove it (and the time spent to look it up)

RUN rm -rf ~/.gradle/daemon

ENV GRADLE_OPTS="--add-opens java.base/java.util=ALL-UNNAMED --add-opens java.base/java.lang=ALL-UNNAMED --add-opens java.base/java.lang.invoke=ALL-UNNAMED --add-opens java.base/java.util=ALL-UNNAMED --add-opens java.prefs/java.util.prefs=ALL-UNNAMED --add-opens java.prefs/java.util.prefs=ALL-UNNAMED --add-opens java.base/java.nio.charset=ALL-UNNAMED --add-opens java.base/java.net=ALL-UNNAMED --add-opens java.base/java.util.concurrent.atomic=ALL-UNNAMED -Xms256m -Xmx2048M -Dfile.encoding=UTF-8 -Duser.country=US -Duser.language=en -Duser.variant"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is one core change, basically, the --no-daemon option passed from the command line was ineffective causing the startup of a daemon every execution (it was dominating the used time).
I struggled with this for a long and the only "solution/workaround" I've found is this:
gradle/gradle#2660 (comment)

This is ugly and weak (especially maintenance-wise) but is the only thing that actually works.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, my investigations pointed out that no daemon was used, but I didn't know how to fix it. Great work!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andreaTP It looks like in the above changes you're killing all Gradle daemons, right? If so, would a running Gradle daemon that can be re-used when running the tests be faster than --no-daemon? In some tracks, what I did is to build a "dummy" project to pre-load/cache things. I have no idea if that makes sense here though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what I did is to build a "dummy" project to pre-load/cache things

do you mind elaborating or providing a link? might be interesting to investigate further this path!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! For Purescript, we have a regular exercise https://github.com/exercism/purescript-test-runner/tree/main/pre-compiled that we copy into the Docker image and then build/compile https://github.com/exercism/purescript-test-runner/blob/main/Dockerfile#L14. Doing a build/compile forces the Docker container to at least have loaded all the infrastructure once. Whether that is beneficial depends on the runtime that is used.

I tried using this approach with the Java test runner too, as I hoped that being able to re-use an existing Daemon that had already done some work might allow things to be sped up later. I couldn't get the gradle test command to re-use the started Gradle daemon, but I think you could with the GRADLE_OPTS specified.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't get the gradle test command to re-use the started Gradle daemon, but I think you could with the GRADLE_OPTS specified.

correct but it doesn't seem to produce great results ...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, that could be because there actually isn't any Gradle daemon already running, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct


ENTRYPOINT ["sh", "/opt/test-runner/bin/run.sh"]
4 changes: 3 additions & 1 deletion bin/run.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,7 @@ cp -R $input_folder/* .

find . -mindepth 1 -type f | grep 'Test.java' | xargs -I file sed -i "s/@Ignore(.*)//g;s/@Ignore//g;" file

java -jar /opt/test-runner/autotest-runner.jar $problem_slug
echo "test {\n maxHeapSize = '2G'\n reports.html.required = false\n}" >> build.gradle
timeout 20 gradle test --offline --no-daemon --warning-mode=none --no-watch-fs --console=plain 2> gradle-test.err
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved the external process execution from Java to the command line for a couple of reasons:

  • faster iteration cycles (no need to recompile to change options passed to gradle)
  • easier to change
  • log handling for debug purposes

I think that I prefer this here, but I can revert this change if required, not fussy about it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think those are all good arguments in favor of running this in bash.

java -jar /opt/test-runner/autotest-runner.jar $?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

slug was unused (a leftover?) and here we are passing the return code of the previous command to re-use the error handling logic.

mv results.json $output_folder
17 changes: 4 additions & 13 deletions lib/src/main/java/com/exercism/runner/TestRunner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,8 @@ public static void main(String[] args) throws InterruptedException, IOException
run(args[0]);
}

private static void run(String slug) throws InterruptedException, IOException {
Process gradleTest = new ProcessBuilder(
"gradle",
"test",
"--offline",
"--no-daemon",
"--warning-mode=none")
.redirectError(new File(GRADLE_TEST_ERR))
.start();
if (!gradleTest.waitFor(20, SECONDS)) {
throw new IllegalStateException("gradle test did not complete within 20 seconds");
}
if (gradleTest.exitValue() != 0) {
private static void run(String returnCode) throws InterruptedException, IOException {
if (returnCode != "0") {
String gradleErrorOutput = Files.asCharSource(
Paths.get(GRADLE_TEST_ERR).toFile(), StandardCharsets.UTF_8)
.read();
Expand All @@ -54,13 +43,15 @@ private static void run(String slug) throws InterruptedException, IOException {
testParser.parse(filePath.toFile());
}
}

ImmutableMap<String, String> testCodeByTestName = testParser.buildTestCodeMap();
JUnitXmlParser xmlParser = new JUnitXmlParser(testCodeByTestName);
for (Path filePath : MoreFiles.listFiles(Paths.get("build", "test-results", "test"))) {
if (MoreFiles.getFileExtension(filePath).equals("xml")) {
xmlParser.parse(filePath.toFile());
}
}

Report report = xmlParser.buildReport();
ReportGenerator.report(report);
}
Expand Down