search

exercism / java-test-runner

GNU Affero General Public License v3.0
9 stars 13 forks source link

Speed up the gradle test invocation #37

Closed andreaTP closed 2 years ago

andreaTP commented 2 years ago

This is the aggregated result of digging into: https://github.com/exercism/exercism/issues/6097

Feel free to ask me to split this into multiple PRs if you are uncomfortable merging such a big change, I will do it tomorrow.

I'll comment inline the motivation for the various changes.

andreaTP commented 2 years ago

The observed speedup on my local machine running in docker is about:

andreaTP commented 2 years ago

For the records, I have quickly attempted to use GraalVM native-image ( here ) but realized the implementation is reflection heavy.

We need either some sort of tests or integration tests to run this.

ErikSchierboom commented 2 years ago

Thanks a ton work for working on this! I'll have a look next week, I'm terribly busy at the moment.

andreaTP commented 2 years ago

@ErikSchierboom enjoy the weekend! 🙂

SleeplessByte commented 2 years ago

Wow. 1/3 speed-up already is really great @andreaTP!!

iHiD commented 2 years ago

This is great :)

For the records, I have quickly attempted to use GraalVM native-image

For those of us not au fait, would you mind explaining a little more about what this would do pls?

andreaTP commented 2 years ago

Sure @iHiD ! Graal is an umbrella project from Oracle, one aspect of it is native-image that enables producing binaries (as opposed to jars) for jvm projects, reducing a lot startup time. The test runner has a lot in common with command line applications and it will probably benefits from running it as a binary.

Ref: https://www.graalvm.org/reference-manual/native-image/

andreaTP commented 2 years ago

@iHiD I hope this explains!

(Closed by mistake, sorry for the noise)

andreaTP commented 2 years ago

Quick update just to share the experience, (aka another failed experiment 🙂 ).

I thought that running the TestRunner within Gradle itself(as a plugin) would save the additional JVM startup time. A quick experiment (available here) demonstrates that this is not the case (back to ~16-17 seconds), possibly due to Gradle plugins loading overhead.

iHiD commented 2 years ago

The test runner has a lot in common with command line applications and it will probably benefits from running it as a binary.

Nice. That sounds useful indeed!

ErikSchierboom commented 2 years ago

@andreaTP If I try to test this locally, I get the following error:

Exception in thread "main" java.nio.file.NoSuchFileException: build/test-results/test
        at java.base/sun.nio.fs.UnixException.translateToIOException(UnixException.java:92)
        at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:111)
        at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:116)
        at java.base/sun.nio.fs.UnixFileSystemProvider.newDirectoryStream(UnixFileSystemProvider.java:432)
        at java.base/java.nio.file.Files.newDirectoryStream(Files.java:472)
        at com.google.common.io.MoreFiles.listFiles(MoreFiles.java:261)
        at com.exercism.runner.TestRunner.run(TestRunner.java:49)
        at com.exercism.runner.TestRunner.main(TestRunner.java:22)

Any ideas? How did you test on your machine?

andreaTP commented 2 years ago

@ErikSchierboom you are right indeed: here is the fix: https://github.com/exercism/java-test-runner/pull/37/commits/1cd917cfe8ac0bf576aa00d39bda9f4303f753ba

I was unconsciously relying on cached results 🙁 still this PR trim down execution times around ~14-15 seconds per run.

ErikSchierboom commented 2 years ago

@andreaTP Yes, that fixes it. I've tried to benchmark the differences but could not find any on my machine. Maybe I'm doing something wrong? I've tried to benchmark it by adding time before the docker run command in the run_in_docker.sh command and then running ./run_in_docker.sh anagram $PWD/exercise $PWD/exercise. Via this "benchmarking" I couldn't really find any difference between the current test runner and your PR. How did you test the performance?

andreaTP commented 2 years ago

Hey @ErikSchierboom , I was testing against bird-watcher the same way you are doing, will have another look later.

andreaTP commented 2 years ago

Thanks for having a second look @ErikSchierboom ! I figured out that local caches have been tripping me 🙁 , to measure something meaningful I need something like this: https://github.com/andreaTP/java-test-runner/blob/06f0ad2ae6934765ef3d0b4fcfbda1cc94bcdf67/bin/run.sh#L25-L26 and with this, the performance is about the same as you correctly observed.

Takeaways:

I will try to have another look but seems like you have already maxed the performance of this design ... What to try next?

I'm sorry that those experiments have failed, I'm available to have a chat on Slack about possible different approaches.

ErikSchierboom commented 2 years ago

I think discussing things here is probably best, as we then have a public record of the things we've looked into, which makes it easier for other people

aggressive caching? (mounting external folders or in docker etc. etc.) What would be cached? Caching within Docker is perfectly fine, so if you have an idea what we could cache, let me know!

test using a different build tool (maven?)

I have no idea what the ramifications of that change would be, but if the test results are still the same, I'm fine with that. I'll leave it to @exercism/java to judge whether or not this is something to pursue.

increase the timeouts 😜 We can't really do that, as we're already at the maximum runtime we can allow on our infrastructure.

andreaTP commented 2 years ago

I've investigated a bit more:

aggressive caching

  • attempted with --build-cache, --configuration-cache and more: it's not effective

Basically, the very first gradle invocation on a clean project takes 11-12 seconds(on my machine) to resolve the configuration (startup phase of gradle itself).

Given all those failed attempts I'm drafting a PR to switch to Maven, exercism's builds are simple and pretty much constrained, of course, there are trade-offs, but might be worth giving this approach a spin!

iHiD commented 2 years ago

Nice. Thanks @andreaTP!

ErikSchierboom commented 2 years ago

Great! It's definitely worth a look.