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

Keep forked Java compilers around after a build if requested #21034

Merged
merged 3 commits into from
Apr 5, 2023

Conversation

octylFractal
Copy link
Member

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 using largeJavaMultiProject shows a 32% improvement on average for abiChange, and a 52% (!) improvement for cleanAssemble.

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.

@octylFractal octylFractal added a:feature A new functionality in:java-plugins java-library, java, java-base, java-platform, java-test-fixtures labels Jun 16, 2022
@octylFractal octylFractal added this to the 7.6 RC1 milestone Jun 16, 2022
@octylFractal octylFractal requested review from a team June 16, 2022 21:25
@octylFractal octylFractal self-assigned this Jun 16, 2022
@ijuma
Copy link
Contributor

ijuma commented Jun 17, 2022

Something similar was done for Scala recently, but it relies on a configuration instead of changing the default #20580

@lptr
Copy link
Member

lptr commented Jun 17, 2022

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.

@lptr
Copy link
Member

lptr commented Jun 17, 2022

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.

@big-guy big-guy modified the milestones: 7.6 RC1, 8.0 RC1 Jun 29, 2022
@lptr lptr removed the request for review from a team August 1, 2022 09:04
@big-guy big-guy removed this from the 8.0 RC1 milestone Oct 21, 2022
@github-actions
Copy link
Contributor

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.

@octylFractal octylFractal force-pushed the ot/feature/keep-java-compiler-around branch from 27ce800 to e36795c Compare April 3, 2023 20:35
@octylFractal octylFractal requested a review from tresat April 4, 2023 16:14
@octylFractal
Copy link
Member Author

@bot-gradle test and merge

@bot-gradle
Copy link
Collaborator

No milestone for this PR. Please set a milestone and retry.

@octylFractal octylFractal modified the milestones: 8.3 RC1, 8.2 RC1 Apr 5, 2023
@octylFractal
Copy link
Member Author

@bot-gradle test and merge

@gradle gradle deleted a comment from octylFractal Apr 5, 2023
@bot-gradle
Copy link
Collaborator

I've triggered a build for you.

@bot-gradle bot-gradle merged commit abbc465 into master Apr 5, 2023
@bot-gradle bot-gradle deleted the ot/feature/keep-java-compiler-around branch April 5, 2023 06:23
@octylFractal octylFractal changed the title Keep forked Java compilers around after a build Keep forked Java compilers around after a build if requested Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:feature A new functionality in:java-plugins java-library, java, java-base, java-platform, java-test-fixtures
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants