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.

Sign up for GitHub

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

Jump to bottom

Enable String Deduplication on JDK 17+ #1380

Merged
merged 3 commits into from
Mar 15, 2023
Merged

Enable String Deduplication on JDK 17+ #1380

merged 3 commits into from
Mar 15, 2023

Conversation

schlosna
Copy link
Contributor

@schlosna schlosna commented Aug 11, 2022 •
edited
Loading

Before this PR

JVM string deduplication was not enabled

After this PR

Reduce heap size by allowing JVM garbage collectors to deduplicate strings.
==COMMIT_MSG==
Enable String Deduplication on JDK 17+

G1 and Shenandoah GC support string deduplication as of JDK 17

Only enable on JDK 17+ due to fix
https://bugs.openjdk.org/browse/JDK-8277981

JDK 18+ enable string deduplication for SerialGC, ParallalGC, and ZGC.
https://malloc.se/blog/zgc-jdk18
==COMMIT_MSG==

Closes #1378

Possible downsides?

Enabling this by default may trigger some unknown JDK bug(s).

@schlosna schlosna marked this pull request as ready for review August 11, 2022 12:38
@carterkozak
Copy link
Contributor

Let's pick a couple performance-sensitive canary services and try this out before including in sls-packaging

@schlosna schlosna mentioned this pull request Nov 2, 2022
Draft
@schlosna schlosna requested a review from iamdanfox March 15, 2023 13:42
@iamdanfox
Copy link
Contributor

we've run with this one one apollo node for 24 hours and interestingly haven't seen a noticeable reduction in heap yet (but also haven't seen any downside in terms of CPU either). Perhaps we should trial it on a really performance sensitive service like MP or timelock before blasting to everyone?

@carterkozak
Copy link
Contributor

@pkoenig10 Thoughts on giving this a shot in MP?

@carterkozak
Copy link
Contributor

Happy to merge this. Currently this branch targets #1374 which is still do-not-merge, up to you how you'd like to sequence things.

@schlosna
Copy link
Contributor Author

I will rebase off develop later tonight/tomorrow and update this to target develop

@schlosna
Enable String Deduplication on JDK 17+
5f9b9f4 
G1 and Shenandoah GC support string deduplication as of JDK 17
  * https://openjdk.java.net/jeps/192
  * https://bugs.openjdk.org/browse/JDK-8264718

Only enable on JDK 17+ due to fix https://bugs.openjdk.org/browse/JDK-8277981

JDK 18+ enable string deduplication for SerialGC, ParallalGC, and ZGC.
See https://malloc.se/blog/zgc-jdk18
@schlosna schlosna changed the base branch from davids/async-log to develop March 15, 2023 21:24
@changelog-app
Copy link

changelog-app bot commented Mar 15, 2023 •
edited by schlosna
Loading

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

Enable String Deduplication on JDK 17+

G1 and Shenandoah GC support string deduplication as of JDK 17

Only enable on JDK 17+ due to fix
https://bugs.openjdk.org/browse/JDK-8277981

JDK 18+ enable string deduplication for SerialGC, ParallalGC, and ZGC.
https://malloc.se/blog/zgc-jdk18

Check the box to generate changelog(s)

  • Generate changelog entry

carterkozak
carterkozak approved these changes Mar 15, 2023
View reviewed changes
Copy link
Contributor

@carterkozak carterkozak left a comment

Choose a reason for hiding this comment

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

Thanks!

@bulldozer-bot bulldozer-bot bot merged commit 902de8d into develop Mar 15, 2023
@bulldozer-bot bulldozer-bot bot deleted the davids/string-dedupe branch March 15, 2023 22:31
@svc-autorelease
Copy link
Collaborator

Released 7.32.0

schlosna added a commit that referenced this pull request Mar 17, 2023
@schlosna
Revert "Enable String Deduplication on JDK 17+ (#1380)"
0624947 
This reverts commit 902de8d.
@schlosna schlosna mentioned this pull request Mar 17, 2023
Merged
bulldozer-bot bot pushed a commit that referenced this pull request Mar 17, 2023
@schlosna
Revert "Enable String Deduplication on JDK 17+ (#1380)" (#1452)
10be2f1 
This reverts commit 902de8d.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carterkozak carterkozak carterkozak approved these changes

@iamdanfox iamdanfox Awaiting requested review from iamdanfox

Assignees
No one assigned
Labels
autorelease merge when ready
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Enable UseStringDeduplication where feasible
5 participants
@schlosna @carterkozak @iamdanfox @svc-autorelease @svc-changelog