8256298: Shenandoah: Enable concurrent stack processing #2185

zhengyu123 · 2021-01-21T17:33:44Z

Please review this patch that enables concurrent stack processing for Shenandoah GC.

After this patch, all root processing is done concurrently for concurrent GC.

Test:

hotspot_gc_shenandoah Linux x86_64 and x86_32
Nightly
tier1 with -XX:+UseShenandoahGC on Linux x86_32

Progress

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

Issue

JDK-8256298: Shenandoah: Enable concurrent stack processing

Reviewers

Roman Kennke (@rkennke - Reviewer) ⚠️ Review applies to 77582b3
Aleksey Shipilev (@shipilev - Reviewer)

Download

$ git fetch https://git.openjdk.java.net/jdk pull/2185/head:pull/2185
$ git checkout pull/2185

…JDK-8255019-sh-mark

openjdk · 2021-01-21T17:36:42Z

⚠️ @zhengyu123 This pull request contains merges that bring in commits not present in the target repository. Since this is not a "merge style" pull request, these changes will be squashed when this pull request in integrated. If this is your intention, then please ignore this message. If you want to preserve the commit structure, you must change the title of this pull request to Merge <project>:<branch> where <project> is the name of another project in the OpenJDK organization (for example Merge jdk:master).

openjdk · 2021-01-21T17:41:00Z

@zhengyu123 The following labels will be automatically applied to this pull request:

hotspot-gc
shenandoah

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

mlbridge · 2021-01-24T01:34:56Z

Webrevs

rkennke

Wow that looks very nice and clean! I only have a few minor comments.

src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp

src/hotspot/share/gc/shenandoah/shenandoahStackWatermark.cpp

src/hotspot/share/gc/shenandoah/shenandoahStackWatermark.hpp

rkennke · 2021-01-25T12:49:01Z

/reviewers 2

openjdk · 2021-01-25T12:49:15Z

@rkennke
The number of required reviews for this PR is now set to 2 (with at least 1 of role reviewers).

rkennke

Better, thanks! Found a few more very minor complaints (sorry... ;-) )

src/hotspot/share/gc/shenandoah/shenandoahStackWatermark.cpp

src/hotspot/share/gc/shenandoah/shenandoahStackWatermark.hpp

rkennke

Looks good to me! Thank you!

shipilev

Cursory review.

src/hotspot/share/gc/shenandoah/shenandoahClosures.inline.hpp

src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp

src/hotspot/share/gc/shenandoah/shenandoahStackWatermark.cpp

shipilev

Good job, I have only a few minor comments.

src/hotspot/share/gc/shenandoah/shenandoahPhaseTimings.hpp

openjdk · 2021-01-26T15:33:22Z

@zhengyu123 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:

8256298: Shenandoah: Enable concurrent stack processing

Reviewed-by: rkennke, shade

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

b07797c: 8260391: Remove StringCoding::err
af8a08f: 8260378: [TESTBUG] DcmdMBeanTestCheckJni.java reports false positive
8d2f77f: 8260406: Do not copy pure java source code to gensrc
5e8e0ad: 8242456: PreviewFeature.Feature enum removal of TEXT_BLOCKS
e080ce9: 8252545: runtime/ErrorHandling/TestHeapDumpOnOutOfMemoryError.java timed out
edd2707: 8260408: Shenandoah: adjust inline hints after JDK-8255019
81a66df: 8259809: Remove PerfEvent class loading locking counters
b4ace3e: 8260042: G1 Post-cleanup liveness printing occurs too early
23edb6f: 8236490: Compiler bug relating to @nonnull annotation
abd9310: 8260222: remove unused _thread member SymbolTableLookup
... and 57 more: https://git.openjdk.java.net/jdk/compare/34eb8b344daabc9c52e7d9b575767dae1a3a2d51...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.

zhengyu123 · 2021-01-26T16:45:38Z

/integrate

openjdk · 2021-01-26T16:46:47Z

@zhengyu123 Since your change was applied there have been 67 commits pushed to the master branch:

b07797c: 8260391: Remove StringCoding::err
af8a08f: 8260378: [TESTBUG] DcmdMBeanTestCheckJni.java reports false positive
8d2f77f: 8260406: Do not copy pure java source code to gensrc
5e8e0ad: 8242456: PreviewFeature.Feature enum removal of TEXT_BLOCKS
e080ce9: 8252545: runtime/ErrorHandling/TestHeapDumpOnOutOfMemoryError.java timed out
edd2707: 8260408: Shenandoah: adjust inline hints after JDK-8255019
81a66df: 8259809: Remove PerfEvent class loading locking counters
b4ace3e: 8260042: G1 Post-cleanup liveness printing occurs too early
23edb6f: 8236490: Compiler bug relating to @nonnull annotation
abd9310: 8260222: remove unused _thread member SymbolTableLookup
... and 57 more: https://git.openjdk.java.net/jdk/compare/34eb8b344daabc9c52e7d9b575767dae1a3a2d51...master

Your commit was automatically rebased without conflicts.

Pushed as commit fd00ed7.

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

Zhengyu Gu added 30 commits October 30, 2020 15:13

8255019: Shenandoah: Split STW and concurrent mark into separate classes

4798cc2

Split finish mark and pre-evaculation

787e6ca

Make ShenandoahMarkCompact stack allocated

6b75cf0

Merge branch 'master' into JDK-8255019-sh-mark

e4113c3

Merge branch 'master' into JDK-8255019-sh-mark

a616a46

Merge

824165f

Merge branch 'JDK-8255019-sh-mark' of github.com:zhengyu123/jdk into …

b4efedb

…JDK-8255019-sh-mark

init update

f1222c6

Move weak reference processing out of STWMark and fix its timings

bb72dea

Merge branch 'JDK-8255019-sh-mark' into fix_phase_timings

b9b39fb

update

7b4ae25

Merge

9c5d0d3

Merge branch 'master' into JDK-8255019-sh-mark

75e6a85

Merge branch 'JDK-8255019-sh-mark' into fix_phase_timings

69e6f0e

init update

da64890

update

c6a26a1

Revert is_marked -> is_marked_strong changes

a3d25ad

Merge branch 'master' into JDK-8255019-sh-mark

fe93d95

Merge branch 'JDK-8255019-sh-mark' into fix_phase_timings

6042402

Merge branch 'fix_phase_timings' into JDK-8255765-isolate-gcs

1fd8124

Merge branch 'master' into JDK-8255019-sh-mark

6896e06

Merge branch 'JDK-8255019-sh-mark' into fix_phase_timings

33d512c

Merge branch 'fix_phase_timings' into JDK-8255765-isolate-gcs

f0eb0d3

Revert access kind port

80c8408

Update

6da85c6

Merge branch 'master' into JDK-8255019-sh-mark

71b38b4

Removed obsoleted class

91886ed

Merge branch 'JDK-8255019-sh-mark' into fix_phase_timings

b2a1c04

Merge branch 'fix_phase_timings' into JDK-8255765-isolate-gcs

bcda942

Fix a compilation error

764d709

openjdk bot added hotspot-gc hotspot-gc-dev@openjdk.org shenandoah shenandoah-dev@openjdk.org labels Jan 21, 2021

zhengyu123 marked this pull request as ready for review January 24, 2021 01:30

openjdk bot added the rfr Pull request is ready for review label Jan 24, 2021

Minor cleanup

6217060

rkennke suggested changes Jan 25, 2021

View reviewed changes

src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp Show resolved Hide resolved

src/hotspot/share/gc/shenandoah/shenandoahStackWatermark.cpp Outdated Show resolved Hide resolved

src/hotspot/share/gc/shenandoah/shenandoahStackWatermark.hpp Outdated Show resolved Hide resolved

Roman's comments

61b3dcb

rkennke suggested changes Jan 25, 2021

View reviewed changes

src/hotspot/share/gc/shenandoah/shenandoahStackWatermark.cpp Outdated Show resolved Hide resolved

src/hotspot/share/gc/shenandoah/shenandoahStackWatermark.hpp Outdated Show resolved Hide resolved

src/hotspot/share/gc/shenandoah/shenandoahStackWatermark.hpp Outdated Show resolved Hide resolved

More of Roman's comments

77582b3

rkennke approved these changes Jan 25, 2021

View reviewed changes

shipilev reviewed Jan 25, 2021

View reviewed changes

src/hotspot/share/gc/shenandoah/shenandoahClosures.inline.hpp Show resolved Hide resolved

src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp Outdated Show resolved Hide resolved

src/hotspot/share/gc/shenandoah/shenandoahStackWatermark.cpp Outdated Show resolved Hide resolved

Zhengyu Gu added 2 commits January 25, 2021 13:17

Aleksey's comments

3cab64e

Reverted override

6ade41a

shipilev suggested changes Jan 26, 2021

View reviewed changes

src/hotspot/share/gc/shenandoah/shenandoahPhaseTimings.hpp Outdated Show resolved Hide resolved

src/hotspot/share/gc/shenandoah/shenandoahPhaseTimings.hpp Outdated Show resolved Hide resolved

More Aleksey's comments

ef78719

shipilev approved these changes Jan 26, 2021

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jan 26, 2021

openjdk bot closed this Jan 26, 2021

openjdk bot added integrated Pull request has been integrated and removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jan 26, 2021

zhengyu123 deleted the JDK-8256298-conc-stack-proc branch May 2, 2022 18:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8256298: Shenandoah: Enable concurrent stack processing #2185

8256298: Shenandoah: Enable concurrent stack processing #2185

zhengyu123 commented Jan 21, 2021 •

edited by openjdk bot

Loading

openjdk bot commented Jan 21, 2021

openjdk bot commented Jan 21, 2021

mlbridge bot commented Jan 24, 2021 •

edited

Loading

rkennke left a comment

rkennke commented Jan 25, 2021

openjdk bot commented Jan 25, 2021

rkennke left a comment

rkennke left a comment

shipilev left a comment

shipilev left a comment

openjdk bot commented Jan 26, 2021

zhengyu123 commented Jan 26, 2021

openjdk bot commented Jan 26, 2021

8256298: Shenandoah: Enable concurrent stack processing #2185

8256298: Shenandoah: Enable concurrent stack processing #2185

Conversation

zhengyu123 commented Jan 21, 2021 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Download

openjdk bot commented Jan 21, 2021

openjdk bot commented Jan 21, 2021

mlbridge bot commented Jan 24, 2021 • edited Loading

Webrevs

rkennke left a comment

Choose a reason for hiding this comment

rkennke commented Jan 25, 2021

openjdk bot commented Jan 25, 2021

rkennke left a comment

Choose a reason for hiding this comment

rkennke left a comment

Choose a reason for hiding this comment

shipilev left a comment

Choose a reason for hiding this comment

shipilev left a comment

Choose a reason for hiding this comment

openjdk bot commented Jan 26, 2021

zhengyu123 commented Jan 26, 2021

openjdk bot commented Jan 26, 2021

zhengyu123 commented Jan 21, 2021 •

edited by openjdk bot

Loading

mlbridge bot commented Jan 24, 2021 •

edited

Loading