RFR: 8254598: StringDedupTable should use OopStorage [v5]
Kim Barrett
kbarrett at openjdk.java.net
Fri May 14 18:42:06 UTC 2021
> Please review this change to the String Deduplication facility. It is being
> changed to use OopStorage to hold weak references to relevant objects,
> rather than bespoke data structures requiring dedicated processing phases by
> supporting GCs.
>
> (The Shenandoah update was contributed by Zhengyu Gu.)
>
> This change significantly simplifies the interface between a given GC and
> the String Deduplication facility, which should make it much easier for
> other GCs to opt in. However, this change does not alter the set of GCs
> providing support; currently only G1 and Shenandoah support string
> deduplication. Adding support by other GCs will be in followup RFEs.
>
> Reviewing via the diffs might not be all that useful for some parts, as
> several files have been essentially completely replaced, and a number of
> files have been added or eliminated. The full webrev might be a better
> place to look.
>
> The major changes are in gc/shared/stringdedup/* and in the supporting
> collectors, but there are also some smaller changes in other places, most
> notably classfile/{stringTable,javaClasses}.
>
> This change is additionally labeled for review by core-libs, although there
> are no source changes there. This change injects a byte field of bits into
> java.lang.String, using one of the previously unused padding bytes in that
> class. (There were two unused bytes, now only one.)
>
> Testing:
> mach5 tier1-2 with and without -XX:+UseStringDeduplication
>
> Locally (linux-x64) ran all of the existing tests that use string
> deduplication with both G1 and Shenandoah. Note that
> TestStringDeduplicationInterned.java is disabled for shenandoah, as it
> currently fails, for reasons I haven't figured out but suspect are test
> related.
>
> - gc/stringdedup/ -- these used to be in gc/g1
> - runtime/cds/SharedStringsDedup.java
> - runtime/cds/appcds/cacheObject/DifferentHeapSizes.java
> - runtime/cds/appcds/sharedStrings/*
>
> shenandoah-only:
> - gc/shenandoah/jvmti/TestHeapDump.java
> - gc/shenandoah/TestStringDedup.java
> - gc/shenandoah/TestStringDedupStress.java
>
> Performance tested baseline, baseline + stringdedup, new with stringdedup,
> finding no significant differences.
Kim Barrett has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 10 commits:
- Merge branch 'master' into new_dedup2
- fix shenandoah merge
- Merge branch 'master' into new_dedup2
- misc cleanups
- refactor Requests::add
- fix typo
- more comment improvements
- improve naming and comments around injected String flags
- fix some typos in comments
- New string deduplication
-------------
Changes: https://git.openjdk.java.net/jdk/pull/3662/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3662&range=04
Stats: 6192 lines in 105 files changed: 2369 ins; 3204 del; 619 mod
Patch: https://git.openjdk.java.net/jdk/pull/3662.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/3662/head:pull/3662
PR: https://git.openjdk.java.net/jdk/pull/3662
More information about the core-libs-dev
mailing list