chore: parallel test and build #7093
Conversation
| @@ -0,0 +1 @@ | |||
| -XX:-TieredCompilation -XX:TieredStopAtLevel=1 -XX:+UseParallelGC -Djava.awt.headless=true | |||
There was a problem hiding this comment.
Older versions of Java and Maven don't seem to pick these up, but need to confirm with testing to make sure -XX:+UseParallelGC etc. don't break specific JVM versions.
There was a problem hiding this comment.
Yep, JDK 8 seems fine.
| @@ -0,0 +1,2 @@ | |||
| -T2C | |||
| --strict-checksums | |||
There was a problem hiding this comment.
--strict-checksums made it in, but this is a good thing: Maven will reject dependencies with checksum failures. Surprisingly, this is not the default.
| <!-- Enable parallel test execution --> | ||
| <parallel>all</parallel> | ||
| <perCoreThreadCount>false</perCoreThreadCount> | ||
| <threadCount>48</threadCount> |
There was a problem hiding this comment.
Parallel settings; tunable as properties from outside the build
| @@ -31,6 +35,7 @@ | |||
| <otherVariant.version>HEAD-android-SNAPSHOT</otherVariant.version> | |||
| <otherVariant.jvmEnvironment>android</otherVariant.jvmEnvironment> | |||
| <otherVariant.jvmEnvironmentVariantName>android</otherVariant.jvmEnvironmentVariantName> | |||
| <surefire.rerunFailingTestsCount>3</surefire.rerunFailingTestsCount> | |||
| <reuseForks>true</reuseForks> | ||
| <forkCount>2C</forkCount> |
sgammon
force-pushed
the
chore/build-test-performance
branch
from
0d243b6
to
a726c91
Compare
|
Thanks, this is something we probably should have looked into long ago. I have lots of thoughts:
|
|
@cpovirk My thoughts are surprisingly aligned with yours, actually:
I noticed this too and found it to be an interesting point. I hope to flush it out a bit more in my CI PR.
I thought about that, or perhaps just suppressing failures until they actually fail--that test seems to flake but always passes when it is retried, rendering the error messages useless anyway. I'll look into options in the Surefire plugin. The build still hasn't actually flaked because of this test. I assume that, if someone is building Guava from source, they should be okay with failure logs as long as the build itself doesn't consider it a failure, but the point you make here is valid and it bothers me too.
Yes, I figure this is worth looking at. Honestly, it's literally 4-5 tests that seem to be angry out of 1M+. I could maybe gather which tests are flaky so a determination could be made about simply disabling them. I think there are ~500 ignored tests as it is so it may not be a big deal, I don't know. I definitely didn't want to do this without asking first, though.
I noticed this, too, once I actually visualized the benchmarks. I think you are right, and I think most of the "risk" comes from the threaded building in Maven, which is new and not as well supported as VM forking during tests. I'll keep the configurable properties but rollback the threaded build as a default; that should offer a nice balance, where the threaded build can be tried but it isn't on by default. The test forking obviously provides a huge benefit so I'll leave that on by default unless otherwise advised
I think that's smart if only because threaded building is still a bit buggy in Maven. But you're right that the build should not necessarily assume it is the only thing running on the machine. (Still bugs me, then, that CI sees less of an improvement, but anyway.)
That's right, it's Maven's multi-threaded building, not forking. I'm not sure if Maven supports forking/re-use for builds, but I do know there is now a Maven daemon, maybe that is where forking can safely take place, as jobs are assigned/consumed? In any case, it's a moot point because of the risk identified above for multi-threaded builds. It should just be optional bc risk / low return. |
This changeset optimizes the Guava build significantly by enabling parallel build and test features supported by Maven. With these flags enabled, only a few tests exhibit flaky behavior; applying a sensible count of test retries (3) solves the problem. As a result, the testsuite can now be executed often, because it takes about 2 minutes to run. Building is also much faster. After benchmarking different configurations, 2-threads-per-core and 2-test-forks-per-core seems optimal: ``` [INFO] Guava Maven Parent ..................... SUCCESS [ 0.121 s] [INFO] Guava: Google Core Libraries for Java .. SUCCESS [ 9.681 s] [INFO] Guava BOM .............................. SUCCESS [ 0.120 s] [INFO] Guava Testing Library .................. SUCCESS [ 47.883 s] [INFO] Guava Unit Tests ....................... SUCCESS [01:57 min] <-- [INFO] Guava GWT compatible libs .............. SUCCESS [ 6.909 s] ``` When built and executed serially: ``` [INFO] Guava Maven Parent ..................... SUCCESS [ 0.129 s] [INFO] Guava: Google Core Libraries for Java .. SUCCESS [ 15.653 s] [INFO] Guava BOM .............................. SUCCESS [ 0.064 s] [INFO] Guava Testing Library .................. SUCCESS [01:26 min] [INFO] Guava Unit Tests ....................... SUCCESS [06:26 min] <-- [INFO] Guava GWT compatible libs .............. SUCCESS [ 11.092 s] ``` Benchmark hardware: - Apple M2 Max, 96GB RAM - macOS Sonoma 14.3.1 - GraalVM CE JVM 21.0.2 ``` openjdk version "21.0.2" 2024-01-16 OpenJDK Runtime Environment GraalVM CE 21.0.2+13.1 (build 21.0.2+13-jvmci-23.1-b30) OpenJDK 64-Bit Server VM GraalVM CE 21.0.2+13.1 (build 21.0.2+13-jvmci-23.1-b30, mixed mode, sharing) ``` - chore: enable parallel build - chore: enable parallel test execution - chore: enable parallel gc for maven - chore: tune tiered compilation for maven - chore: tune thread count for maven - fix: enable test retries (max = 3) for parallel-flaky tests Signed-off-by: Sam Gammon <[email protected]>
sgammon
force-pushed
the
chore/build-test-performance
branch
from
a726c91
to
e946f2b
Compare
I couldn't find the alleged log files or dump files, and the normal
|
|
I just kicked off another run with I'll be interested to see what the "normal" output looks like, but I'm out of time for the day. (If it's related to ports: I want to say that our machines use up a surprising number of ports, but I wouldn't have expected it to be enough to interfere with some modest Maven parallelism. Maybe the messages above are mostly spam and the real problem is still a timeout? I haven't experimented with a higher timeout yet....) |
|
@cpovirk I'm working on an approach locally which will let us tag tests which need to be run serially. The vast majority of the tests work totally fine under parallel execution; and, I think So, if we can tag such tests and just run them after in their own suite, we should get most of the gain with no interference with the actual test code, which I'd like to avoid. If this can be a build-only PR it should be easier to understand the impacts. |
|
Nice, that sounds good. Updates on my end:
|
|
Ha, I spoke too soon! Try number 12 went on to crash. The problem was in This makes me nervous about even fork-level parallelism for tests :\ But even if we rule out all test parallelism, we should be able to get some benefits from the rest of the changes here. Or, if we're lucky, the crashes are restricted to a few tests, too, in which case we can exclude them. (Hopefully we can narrow it down to something more specific than "all of |
|
I later bumped the timeout to 6000, and try number 13 failed, this time in |
|
It occurred to me that there's one small thing that we could do to improve build performance: disable Error Prone. We run it over the code internally, so there's no strict need to run it externally, too. (I think I added it partially to bring Error Prone to people's attention and partially to help anyone who might write a PR.) I just did 1 run of As things are today:
|
|
@cpovirk Nice! I wonder if I can get a quick build matrix going so we can test all of these variants out. I'll be returning to this soon |
Summary
This changeset improves Guava's build and test speed by activating Maven's built-in features for parallel build and test execution. Guava's testsuite is enormous, with over a million tests executed during a build; this PR cuts the execution time for all tests from ~6.5 minutes to ~2 minutes. Compile steps also see ~33% speedup.
Some of Guava's tests are understandably flaky when executed like this. There are only 4-5 tests that I've seen be flaky after many runs. Setting a reasonable test-retry count (
3in this PR) covers this, and I haven't seen the testsuite flake to a failure state since. Split out from work in #7094.Representative sample when built and executed serially:
Representative sample with this PR applied:
guavabench.mp4
Benchmark command:
CI runs
Concurrency tuning
The current settings are:
The full JVM argline for Maven:
Benchmarking environment
PR Tree
This PR includes the following PRs, which should be rebased away after they are merged:
Changelog
cc / @cushon @cpovirk