8273614: Shenandoah: intermittent timeout with ConcurrentGCBreakpoint tests #5868

zhengyu123 · 2021-10-08T16:47:07Z

I were able to reproduce with aggressive heuristics.

A _wb_breakpoint GC request , while a concurrent GC is started. ShenandoahBreakpoint::_start_gc may not get set, as we bypass it if it is not a _wb_breakpoint GC, then we are forever blocked in ShenandoahBreakpoint::at_before_gc(), waiting for the flag to be set.

The solution is only run breakpoints during _wb_breakpoint GC.

Also, enforced _requested_gc_cause and _gc_requested order, ensure that read side sees latest _requested_gc_cause.

Test:

tier1 with Shenandoah

Progress

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

Issue

JDK-8273614: Shenandoah: intermittent timeout with ConcurrentGCBreakpoint tests

Reviewers

Aleksey Shipilev (@shipilev - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 5868

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

Using diff file

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

bridgekeeper · 2021-10-08T16:47:31Z

👋 Welcome back zgu! 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 · 2021-10-08T16:49:22Z

@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-10-08T16:55:26Z

Webrevs

01: Full - Incremental (e34cd1bc)
00: Full (175811c7)

shipilev

Looks good with minor nits.

shipilev · 2021-10-11T10:40:52Z

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

@@ -50,24 +50,37 @@

 // Breakpoint support
 class ShenandoahBreakpointGCScope : public StackObj {
+private:
+  GCCause::Cause _cause;


Better be const?

shipilev · 2021-10-11T10:41:03Z

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

  }
 };

 class ShenandoahBreakpointMarkScope : public StackObj {
+private:
+  GCCause::Cause _cause;


Also const?

shipilev · 2021-10-11T10:43:43Z

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

    _requested_gc_cause = cause;
+    _gc_requested.set();


Ah. This whole thing is under _gc_waiters_lock. But there is a raw read of _requested_gc_cause and _gc_requested in ShenandoahControlThread::run_service. Pity. Could you please fork it out as separate (more backportable) bug?

openjdk · 2021-10-11T10:45:28Z

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

8273614: Shenandoah: intermittent timeout with ConcurrentGCBreakpoint tests

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

b870468: 8274347: Passing a nested switch expression as a parameter causes an NPE during compile
110e38d: 8274753: ZGC: SEGV in MetaspaceShared::link_shared_classes
b7af890: 8274430: Remove some debug error printing code added in JDK-8017163
aaf2401: 8274927: Remove unnecessary G1ArchiveAllocator code
c55dd36: 8275008: gtest build failure due to stringop-overflow warning with gcc11
3edee1e: 8272723: Don't use Access API to access primitive fields
49f8ce6: 8274773: [TESTBUG] UnsafeIntrinsicsTest intermittently fails on weak memory model platform
c032186: 8272968: AArch64: Remove redundant matching rules for commutative ops
a05873a: 8274952: jdk/jfr/api/consumer/TestRecordedFrameType.java failed when c1 disabled
5ecc99b: 8274620: resourcehogs/serviceability/sa/TestHeapDumpForLargeArray.java is timing out
... and 13 more: https://git.openjdk.java.net/jdk/compare/6364719cd1c57220769ea580d958da8dc2fdf7f9...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-10-11T15:09:53Z

/integrate

openjdk · 2021-10-11T15:11:04Z

Going to push as commit 3f07337.
Since your change was applied there have been 24 commits pushed to the master branch:

0d80f6c: 8274379: Allow process of unsafe access errors in check_special_condition_for_native_trans
b870468: 8274347: Passing a nested switch expression as a parameter causes an NPE during compile
110e38d: 8274753: ZGC: SEGV in MetaspaceShared::link_shared_classes
b7af890: 8274430: Remove some debug error printing code added in JDK-8017163
aaf2401: 8274927: Remove unnecessary G1ArchiveAllocator code
c55dd36: 8275008: gtest build failure due to stringop-overflow warning with gcc11
3edee1e: 8272723: Don't use Access API to access primitive fields
49f8ce6: 8274773: [TESTBUG] UnsafeIntrinsicsTest intermittently fails on weak memory model platform
c032186: 8272968: AArch64: Remove redundant matching rules for commutative ops
a05873a: 8274952: jdk/jfr/api/consumer/TestRecordedFrameType.java failed when c1 disabled
... and 14 more: https://git.openjdk.java.net/jdk/compare/6364719cd1c57220769ea580d958da8dc2fdf7f9...master

Your commit was automatically rebased without conflicts.

openjdk · 2021-10-11T15:11:20Z

@zhengyu123 Pushed as commit 3f07337.

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

Zhengyu Gu added 9 commits August 17, 2021 14:57

v0

c989922

Merge branch 'master' into TestJNIWeak-timeout

5723dfc

Merge branch 'master' into JDK-8273614-conbkptr-timeout

a1b64e9

use wait

70a68b5

Merge branch 'master' into JDK-8273614-conbkptr-timeout

f2b4a53

Merge branch 'master' into JDK-8273614-conbkptr-timeout

750a175

v1

ce7aca7

Merge branch 'master' into JDK-8273614-conbkptr-timeout

cfbec84

Order requested gc cause and gc requested flag

175811c

openjdk bot added the rfr Pull request is ready for review label Oct 8, 2021

openjdk bot added hotspot-gc hotspot-gc-dev@openjdk.org shenandoah shenandoah-dev@openjdk.org labels Oct 8, 2021

shipilev approved these changes Oct 11, 2021

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Oct 11, 2021

Aleksey's comment

e34cd1b

shipilev approved these changes Oct 11, 2021

View reviewed changes

openjdk bot closed this Oct 11, 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 Oct 11, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

8273614: Shenandoah: intermittent timeout with ConcurrentGCBreakpoint tests #5868

8273614: Shenandoah: intermittent timeout with ConcurrentGCBreakpoint tests #5868

Uh oh!

zhengyu123 commented Oct 8, 2021 •

edited by openjdk bot

Loading

Uh oh!

bridgekeeper bot commented Oct 8, 2021

Uh oh!

openjdk bot commented Oct 8, 2021

Uh oh!

mlbridge bot commented Oct 8, 2021 •

edited

Loading

Uh oh!

shipilev left a comment

Uh oh!

shipilev Oct 11, 2021

Uh oh!

shipilev Oct 11, 2021

Uh oh!

shipilev Oct 11, 2021

Uh oh!

openjdk bot commented Oct 11, 2021 •

edited

Loading

Uh oh!

zhengyu123 commented Oct 11, 2021

Uh oh!

openjdk bot commented Oct 11, 2021

Uh oh!

openjdk bot commented Oct 11, 2021

Uh oh!

Uh oh!

8273614: Shenandoah: intermittent timeout with ConcurrentGCBreakpoint tests #5868

8273614: Shenandoah: intermittent timeout with ConcurrentGCBreakpoint tests #5868

Uh oh!

Conversation

zhengyu123 commented Oct 8, 2021 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Oct 8, 2021

Uh oh!

openjdk bot commented Oct 8, 2021

Uh oh!

mlbridge bot commented Oct 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

shipilev left a comment

Choose a reason for hiding this comment

Uh oh!

shipilev Oct 11, 2021

Choose a reason for hiding this comment

Uh oh!

shipilev Oct 11, 2021

Choose a reason for hiding this comment

Uh oh!

shipilev Oct 11, 2021

Choose a reason for hiding this comment

Uh oh!

openjdk bot commented Oct 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zhengyu123 commented Oct 11, 2021

Uh oh!

openjdk bot commented Oct 11, 2021

Uh oh!

openjdk bot commented Oct 11, 2021

Uh oh!

Uh oh!

zhengyu123 commented Oct 8, 2021 •

edited by openjdk bot

Loading

mlbridge bot commented Oct 8, 2021 •

edited

Loading

openjdk bot commented Oct 11, 2021 •

edited

Loading