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

Update JavaGCFlagChecker to recommend G1GC for Java 17+ #258

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

NickFirmani
Copy link

tl;dr g1gc is faster in Java17

@runningcode
Copy link
Owner

Thanks @NickFirmani do you have any benchmarks or information on in which scenarios G1GC is faster?

Copy link
Owner

@runningcode runningcode left a comment

Choose a reason for hiding this comment

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

Looks great. Thanks for adding the appropriate tests and updated the documentation

Copy link
Owner

@runningcode runningcode left a comment

Choose a reason for hiding this comment

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

On second thought, for migration purposes, let's leave the old flag around as well, see my comment.

*/
warnWhenNotUsingParallelGC = true
failWhenNotUsingOptimalGC = true
Copy link
Owner

Choose a reason for hiding this comment

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

thanks for updating the verbiage here from "warn" to "fail" this is a breaking change but I agree with it.
can you also keep the old extension around (along with the new one) with a deprecated flag for backwards compatibility? we'll remove it in a future version but I want to make it easy to migrate.

Copy link
Author

Choose a reason for hiding this comment

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

For sure, I added the old extension back, but set it to default to false. I think that makes sense, as a consumer of the library who updates will expect new build failures due to improvements they can make to their build.

@NickFirmani
Copy link
Author

Bumping this up @runningcode, ty!

@runningcode
Copy link
Owner

This article seems to suggest that the parallel GC still gives the best performance: https://dev.to/cdsap/using-parellgc-for-the-kotlin-process-in-android-builds-2geh

@NickFirmani
Copy link
Author

Yeah, I saw that article, which is super interesting, because for us, G1 was quite a bit faster, perhaps due to some quirk of our environment

@PaulWoitaschek
Copy link

What about modifying the logic that it checks that at least one is present?
In the end it's often project specific but having one of them is usually a good thing.

@sgammon
Copy link

sgammon commented Dec 3, 2023

@runningcode i believe that article points out that it is running pre-jvm17

Finally, as you know, with AGP 8, JDK is set to 17. I will publish a new article covering this experiment with the JDK 17 Garbage collector results.

I almost didn't catch it...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants