8302122: Parallelize TLAB retirement in prologue in G1

[tschatzl]

Hi all,

can I have reviews for this change that parallelizes TLAB retirement and log buffer flushing (which JDK-8297611 suggests?

• the parallelization has been split into java and non-java threads: parallelization of the latter does not make sense given how they are organized in a linked list. However non-java threads are typically very few anyway, and parallelization only starts making sense with low hundreds of threads anyway.
• G1BarrierSet::write_region added a new parameter that describes the JavaThread the write_region/invalidation (not sure why write_region isn't just an overload of invalidate) is made for. The reason is previously when BarrierSet::make_parsable() (not sure why this is called this way either) is called to flush out deferred card marks, the card marks generated by that were added to the calling thread's refinement queue. This worked because the calling thread (the worker thread/vm thread) has always been processed after all java threads (which are the only ones that can have deferred card marks). So these deferred card marks' refinement entries were put into the worker thread's refinement queue. After parallelization this is not possible any more unless we deferred non java thread log flushing until after all java threads - I did not want to do that, so the change passes that explicit java thread through to G1BarrierSet::write_region. I think this is clearer anyway, and corresponds to how G1DirtyCardQueueSet::concatenate_log_and_stats works.

Testing: tier1-5

Thanks,
Thomas

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issues

• JDK-8302122: Parallelize TLAB retirement in prologue in G1
• JDK-8297611: G1: Merge tlab and per-thread dirty card log flushing

Reviewers

• Kim Barrett (@kimbarrett - Reviewer)
• Albert Mingkun Yang (@albertnetymk - Reviewer) ⚠️ Review applies to 165085a3

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/12497/head:pull/12497
$ git checkout pull/12497

Update a local copy of the PR:
$ git checkout pull/12497
$ git pull https://git.openjdk.org/jdk pull/12497/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 12497

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/12497.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

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.

[tschatzl]

/label add hotspot-gc

[openjdk]

@tschatzl
The hotspot-gc label was successfully added.

[tschatzl]

/issue add JDK-8297611

[openjdk]

@tschatzl
Adding additional issue to issue list: 8297611: G1: Merge tlab and per-thread dirty card log flushing.

[mlbridge]

Webrevs

• 04: Full - Incremental (1e37161d)
• 03: Full - Incremental (53f887b9)
• 02: Full - Incremental (165085a3)
• 01: Full - Incremental (950b849d)
• 00: Full (ad492777)

[kimbarrett]

A few comments, but overall looks quite nice.

[albertnetymk]

Minor subjective suggestion.

[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:

8302122: Parallelize TLAB retirement in prologue in G1
8297611: G1: Merge tlab and per-thread dirty card log flushing

Reviewed-by: kbarrett, ayang

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 145 new commits pushed to the master branch:

• eaae0ba: 8302215: G1: Last-ditch Full GC should do serial compaction for tail regions in per thread compaction points.
• 7e08275: 8302668: [TESTBUG] Tests require feature sse4_1 which does not exist, should be sse4.1
• 5c0f50b: 8295979: [IR Framework] Improve IR matching warning
• 743a85d: 8302656: Missing spaces in output of -XX:+CIPrintMethodCodes
• 432cf68: 6753661: JFileChooser font not reset after Look & Feel change
• 2009dc2: 8302462: [REDO] 8297487: G1 Remark: no need to keep alive oop constants of nmethods on stack
• 7abe269: 8302781: CDS archive heap not reproducible after JDK-8296344
• d6716d2: 8302315: Examine cost of clone of primitive arrays compared to arraycopy
• 53be5dc: 8302812: JDK-8302455 broke ClassLoaderStatsTest on 32-bit
• 78f71b4: 8301873: Avoid string decoding in ZipFile.Source.getEntryPos
• ... and 135 more: https://git.openjdk.org/jdk/compare/2caa56a66b318a8ac6dac90126ba77f0dc7ed549...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.

[kimbarrett]

Looks good.

[tschatzl]

Thanks for your review @kimbarrett @albertnetymk

/integrate

[openjdk]

Going to push as commit 593bec6.
Since your change was applied there have been 146 commits pushed to the master branch:

• e971f90: 8302206: Factor out duplicate G1VerificationClosure
• eaae0ba: 8302215: G1: Last-ditch Full GC should do serial compaction for tail regions in per thread compaction points.
• 7e08275: 8302668: [TESTBUG] Tests require feature sse4_1 which does not exist, should be sse4.1
• 5c0f50b: 8295979: [IR Framework] Improve IR matching warning
• 743a85d: 8302656: Missing spaces in output of -XX:+CIPrintMethodCodes
• 432cf68: 6753661: JFileChooser font not reset after Look & Feel change
• 2009dc2: 8302462: [REDO] 8297487: G1 Remark: no need to keep alive oop constants of nmethods on stack
• 7abe269: 8302781: CDS archive heap not reproducible after JDK-8296344
• d6716d2: 8302315: Examine cost of clone of primitive arrays compared to arraycopy
• 53be5dc: 8302812: JDK-8302455 broke ClassLoaderStatsTest on 32-bit
• ... and 136 more: https://git.openjdk.org/jdk/compare/2caa56a66b318a8ac6dac90126ba77f0dc7ed549...master

Your commit was automatically rebased without conflicts.

[openjdk]

@tschatzl Pushed as commit 593bec6.

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