-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Keep forked Java compilers around after a build if requested #21034
Conversation
Something similar was done for Scala recently, but it relies on a configuration instead of changing the default #20580 |
We haven't done this yet for a number of reasons, despite the obvious benefits. If we want to keep compiler (or other) forked processes running between builds (=forked daemons), then we need better memory and child-process management. Users might end up with multiple Gradle daemons starting many forked daemons that end up taking up all the available memory. There needs to be a way to detect and kill forked daemons that somehow got in a deadlock, or developed a memory leak, too. It would be great if we could share forked daemons between Gradle daemons, too, though that's not a strict dependency of rolling this out. |
Taking a step back, I think doing the benchmark is already a step forward. We now moved from our anecdotal/self-convinced confidence about the potential improvements we could get by doing this, and confirmed/quantified it with a benchmark. One option could be to do something similar to Scala, and allow users to enable the long-lived compiler daemons already. I think we need to be careful here, though, as doing so can lead to hard to debug problems. But it would be good to get some user feedback about how useful this could be in the field, so putting this behind a feature flag might be something we could do. I would only do this, though, if we are actually planning to implement the memory/child-process management features soon after. |
This pull request has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be closed if no further activity occurs. If you intend to work on this pull request, please ask the team to reopen the PR or push a new PR. Thank you for your contributions. |
27ce800
to
e36795c
Compare
...ts/language-java/src/main/java/org/gradle/api/internal/tasks/compile/DaemonJavaCompiler.java
Outdated
Show resolved
Hide resolved
@bot-gradle test and merge |
No milestone for this PR. Please set a milestone and retry. |
@bot-gradle test and merge |
I've triggered a build for you. |
This results in significant performance improvements when repeatedly running builds by allowing us to re-use the JITed code from the already running
java
processes. A simple benchmark I ran locally usinglargeJavaMultiProject
shows a 32% improvement on average forabiChange
, and a 52% (!) improvement forcleanAssemble
.The benchmarks can be found attached, they were run on the latest nightly (7.6-20220615224043+0000) and this branch which was near the same commit as the nightly.
benchmarks.zip
I would like to get a confirmation from the @gradle/bt-execution team that there are no known problems with this. Otherwise, this seems like a fairly straightforward performance win to me.