8279241: G1 Full GC does not always slide memory to bottom addresses

[tschatzl]

Hi all,

can I get reviews for this change that completely fixes the possibility that during g1 full gc memory will not be slid to bottom addresses consistently in presence of threads.

The problem is that multiple thread compete for regions to compact into - it could happen that given live data somewhere "high up" the heap gets to be the bottom region for a particular thread, so all data will be compacted starting from that region.
The problem with that is region level fragmentation, i.e. that after gc there is not enough free contiguous space for a humongous object, leading to OOME.

The change splits the phase where determining the compaction point queue (the set of regions a particular thread compacts into) from the part of that phase where the new locations of the objects is determined (i.e. putting a forwarding pointer into the live objects in these regions) and other stuff best done in parallel.
This makes determining the compaction point queue deterministic (by distributing these regions we can compact into in a round-robin fashion) in a way that always slides live data consistently into the bottom heap area.

This change also makes it easier to, in the future, improve work distribution of the compaction phase (which directly uses the compaction point queues) by distributing them according to live data, and also incorporate last-ditch moves of humongous objects.

The most important thing about this split is probably the changes in the parallel part: every thread must make sure that some work is done on the compaction point queue (i.e. the forwarding), and other work on all regions (clearing metadata, updating the BOT of young regions that are not moved).

Testing: tier1-5, checking performance on some simple full gc benchmarks with no particular difference

Thanks,
Thomas

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed

Issue

• JDK-8279241: G1 Full GC does not always slide memory to bottom addresses

Reviewers

• Ivan Walulya (@walulyai - Reviewer) ⚠️ Review applies to 30ee707
• Albert Mingkun Yang (@albertnetymk - Reviewer) ⚠️ Review applies to 30ee707
• Stefan Johansson (@kstefanj - Reviewer) ⚠️ Review applies to 30ee707

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/7106/head:pull/7106
$ git checkout pull/7106

Update a local copy of the PR:
$ git checkout pull/7106
$ git pull https://git.openjdk.java.net/jdk pull/7106/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 7106

View PR using the GUI difftool:
$ git pr show -t 7106

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/7106.diff

[bridgekeeper]

👋 Welcome back tschatzl! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@tschatzl The following label will be automatically applied to this pull request:

• hotspot-gc

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 01: Full - Incremental (1500d6c6)
• 00: Full (30ee7079)

[walulyai]

Lgtm!

[openjdk]

@tschatzl This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8279241: G1 Full GC does not always slide memory to bottom addresses

Reviewed-by: iwalulya, ayang, sjohanss

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 193 new commits pushed to the master branch:

• d156911: 8280459: Suspicious integer division in Hashtable.readHashtable
• 9bf6ffa: 8279124: VM does not handle SIGQUIT during initialization
• 30cd47d: 8280499: runtime/cds/appcds/TestDumpClassListSource.java fails on platforms without AppCDS custom class loaders support
• 54c9de2: 8275918: Remove unused local variables in java.base security code
• b9ae779: 8279675: CDS cannot handle non-existent JAR file in bootclassapth
• c1e4f3d: 8279397: Update --release 18 symbol information for JDK 18 build 32
• 2920ce5: 8278036: Saving rscratch1 is optional in MacroAssembler::verify_heapbase
• 6287ae3: 8277531: Print actual default stacksize on Windows thread logging
• ab2c8d3: 8280393: Promote use of HtmlTree factory methods
• 47b1c51: 8277120: Use Optional.isEmpty instead of !Optional.isPresent in java.net.http
• ... and 183 more: https://git.openjdk.java.net/jdk/compare/3121898c33fa3cc5a049977f8677105a84c3e50c...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[albertnetymk]

Some minor comments/suggestions.

[kstefanj]

Looks good, just one small comment.

[tschatzl]

Thanks @albertnetymk @kstefanj @walulyai for your reviews
/integrate

[openjdk]

Going to push as commit 295b263.
Since your change was applied there have been 218 commits pushed to the master branch:

• b327746: 8280384: Parallel: Remove VMThread specific ParCompactionManager
• 5380472: 8280470: Confusing instanceof check in HijrahChronology.range
• a59d717: 8280289: Enhance debug pp() command with NMT info
• f35df5b: 8280422: thread_from_jni_environment can never return NULL
• e307655: 8280403: RegEx: String.split can fail with NPE in Pattern.CharPredicate::match
• 52ddbe2: Merge
• ead9fec: 8280441: Missing "classpath exception" in several files from jdk.httpserver
• 7d2ef9d: 8279179: Update nroff pages in JDK 18 before RC
• 8e82d00: 8280492: Use cross-module syntax for cross-module links
• a541666: 8280474: Garbage value passed to getLocaleInfoWrapper in HostLocaleProviderAdapter_md
• ... and 208 more: https://git.openjdk.java.net/jdk/compare/3121898c33fa3cc5a049977f8677105a84c3e50c...master

Your commit was automatically rebased without conflicts.

[openjdk]

@tschatzl Pushed as commit 295b263.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.