8273482: Remove "foreground work" concept from WorkGang

[pliden]

JDK-8237354 introduced the concept of "foreground work" in WorkGang, as a special case for use by the HeapDumper. I propose that we remove this code, since this special use case can be solved without the need for the concept of "foreground work" in WorkGang.

As far as I can tell, there's no reason why it must be the VM thread that takes on the task of writing the header, iterating over roots, etc. So, in VM_HeapDumper::work(), instead of checking if the current thread is the VM thread we can just check if the worker_id is non-zero. That way, a single worker thread will take on the task of writing the header, iterating over roots, etc, and all other worker threads will continue to call worker_loop().

Testing:

• Passed Tier1-3
• Passed multiple runs of test/hotspot/jtreg/serviceability/dcmd/gc/
• Manually ran jcmd GC.heap_dump with various GCs enabled

---

Progress

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

Issue

• JDK-8273482: Remove "foreground work" concept from WorkGang

Reviewers

• Thomas Schatzl (@tschatzl - Reviewer) ⚠️ Review applies to 74ab4a0
• Kim Barrett (@kimbarrett - Reviewer) ⚠️ Review applies to 74ab4a0

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 5410

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

Using diff file

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

[bridgekeeper]

👋 Welcome back pliden! 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]

@pliden 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.

[pliden]

/label add hotspot-gc,serviceability
/label remove hotspot

[openjdk]

@pliden
The hotspot-gc label was successfully added.

The serviceability label was successfully added.

[openjdk]

@pliden
The hotspot label was successfully removed.

[mlbridge]

Webrevs

• 01: Full - Incremental (7a9abf0c)
• 00: Full (74ab4a07)

[tschatzl]

Lgtm. Thanks a lot for looking into this.

[openjdk]

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

8273482: Remove "foreground work" concept from WorkGang

Reviewed-by: tschatzl, kbarrett

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

• 54dee13: 8273188: java/lang/instrument/BootClassPath/BootClassPathTest.sh fails with "FATAL ERROR in native method: processing of -javaagent failed, processJavaStart failed"
• 4020a60: 8273550: Replace os::cgc_thread/pgc_thread with os::gc_thread
• 185eaca: 8273545: Remove Thread::is_GC_task_thread()
• dd1209e: 8268885: duplicate checkcast when destination type is not first type of intersection type
• ef4a532: 8263773: Reenable German localization for builds at Oracle
• 96614da: 8272903: Missing license header in ArenaAllocator.java
• e3bda63: 8273489: Zero: Handle UseHeavyMonitors on all monitorenter paths
• c81690d: 8273498: compiler/c2/Test7179138_1.java timed out
• 4866eaa: 8273409: Receiver type narrowed by CCP does not always trigger post-parse call devirtualization
• 5ca26cb: 8273433: Enable parallelism in vmTestbase_nsk_sysdict tests
• ... and 28 more: https://git.openjdk.java.net/jdk/compare/a66629a464b97176bcdc2ca1150d12df6241dc1c...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.

[pliden]

Patch updated include the removal of a few asserts, as discussed here: http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2021-September/036807.html

[schmelter-sap]

The reason I've added this was, that for the shenandoah GC the heap iteration would run into assertions if not called from the VM thread. Maybe this has changed in the meantime.

[pliden]

The reason I've added this was, that for the shenandoah GC the heap iteration would run into assertions if not called from the VM thread. Maybe this has changed in the meantime.

As mentioned in my previous comment. This PR removes those asserts since they were overly strict.

[pliden]

Thanks all for reviewing!

/integrate

[openjdk]

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

• 2eaf374: 8272698: LoadNode::pin is unused
• 792281d: 8264517: C2: make MachCallNode::return_value_is_used() only available for x86
• 70c9e02: 8272148: JDesktopPane:getComponentCount() returns one extra than expected with GTKLookAndFeel
• 08f0be8: 8272608: java_lang_System::allow_security_manager() doesn't set its initialization flag
• 2e321dc: 8273528: Avoid ByteArrayOutputStream.toByteArray when converting stream to String
• 54dee13: 8273188: java/lang/instrument/BootClassPath/BootClassPathTest.sh fails with "FATAL ERROR in native method: processing of -javaagent failed, processJavaStart failed"
• 4020a60: 8273550: Replace os::cgc_thread/pgc_thread with os::gc_thread
• 185eaca: 8273545: Remove Thread::is_GC_task_thread()
• dd1209e: 8268885: duplicate checkcast when destination type is not first type of intersection type
• ef4a532: 8263773: Reenable German localization for builds at Oracle
• ... and 33 more: https://git.openjdk.java.net/jdk/compare/a66629a464b97176bcdc2ca1150d12df6241dc1c...master

Your commit was automatically rebased without conflicts.

[openjdk]

@pliden Pushed as commit c1e39fa.

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