8309346: Extend hs_err logging for all VM operations deriving from VM_GC_Operation

[quadhier]

This patch implements cause() for VM_GC_Operation and its derived classes. It follows JDK-8309210 to provide more detailed info for VM_GC_Operations in hs_err log file.

---

Progress

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

Issue

• JDK-8309346: Extend hs_err logging for all VM operations deriving from VM_GC_Operation

Reviewers

• Thomas Schatzl (@tschatzl - Reviewer)
• Stefan Karlsson (@stefank - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 14287

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/14287.diff

Webrev

Link to Webrev Comment

[bridgekeeper]

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

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

• 03: Full - Incremental (8d4d729f)
• 02: Full - Incremental (62c1da2c)
• 01: Full - Incremental (63d5edf4)
• 00: Full (6424880e)

[lkorinth]

(this is not a review) How about using override instead of virtual? I think it better shows how the method is used, and also create compile time errors if we miss to rename the method during refactoring.

[quadhier]

(this is not a review) How about using override instead of virtual? I think it better shows how the method is used, and also create compile time errors if we miss to rename the method during refactoring.

Good advice, but only using override here will raise many other errors like error: 'doit_prologue' overrides a member function but is not marked 'override' [-Werror,-Winconsistent-missing-override]. We'd better leave it for further cleaning up.

[tschatzl]

Lgtm.

Would it be possible to also add some description string for VM_Verify and VM_PopulateDynamicDumpSharedSpace (separately)? I'll file an RFE which you could pick up if you want.

Edit: filed https://bugs.openjdk.org/browse/JDK-8309482

[openjdk]

⚠️ @quadhier the full name on your profile does not match the author name in this pull requests' HEAD commit. If this pull request gets integrated then the author name from this pull requests' HEAD commit will be used for the resulting commit. If you wish to push a new commit with a different author name, then please run the following commands in a local repository of your personal fork:

$ git checkout fix-8309346
$ git commit --author='Preferred Full Name <you@example.com>' --allow-empty -m 'Update full name'
$ git push

[openjdk]

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

8309346: Extend hs_err logging for all VM operations deriving from VM_GC_Operation

Reviewed-by: tschatzl, stefank

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

• 3b85f84: 8309340: Provide sctpHandleSocketErrorWithMessage
• 2e9eff5: 8309406: Change jdk.trackAllThreads to default to true
• 6d155a4: 8308167: SequencedMap::firstEntry throws NPE when first entry has null key or value
• 4b15349: 8304438: jcmd JVMTI.agent_load should obey EnableDynamicAgentLoading
• b3c9d67: 8309136: [JVMCI] add -XX:+UseGraalJIT flag
• 98b53c0: 8306112: Implementation of JEP 445: Unnamed Classes and Instance Main Methods (Preview)
• e970ddb: 8309170: CDS archive heap is always relocated for larger heap
• 4b8922f: 8308842: Consolidate exceptions thrown from Class-File API
• 2b38343: 8309416: Misstatement in semantics of methods in javax.lang.model.ElementFilter
• 73352b6: 8280994: [XWayland] Drag and Drop does not work in java -> wayland app direction
• ... and 23 more: https://git.openjdk.org/jdk/compare/931913fbb299fbed7485ab8229100e6e56d8bada...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.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@tschatzl, @stefank) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

[quadhier]

Would it be possible to also add some description string for VM_Verify and VM_PopulateDynamicDumpSharedSpace (separately)? I'll file an RFE which you could pick up if you want.

Edit: filed https://bugs.openjdk.org/browse/JDK-8309482

Thanks for your review @tschatzl ! I'll take it.

[quadhier]

/integrate

[openjdk]

@quadhier
Your change (at version 62c1da2) is now ready to be sponsored by a Committer.

[stefank]

Could you move the implementation over to the .cpp file? We should strive to not add non-trivial implementation in .hpp files.

[quadhier]

Could you move the implementation over to the .cpp file? We should strive to not add non-trivial implementation in .hpp files.

Make sense, thanks for your review @stefank !

[quadhier]

/reviewers 2

[openjdk]

@quadhier
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

[stefank]

Looks good.

[quadhier]

/integrate

[openjdk]

@quadhier
Your change (at version 8d4d729) is now ready to be sponsored by a Committer.

[tschatzl]

Still good.

[tschatzl]

/sponsor

[openjdk]

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

• 3b85f84: 8309340: Provide sctpHandleSocketErrorWithMessage
• 2e9eff5: 8309406: Change jdk.trackAllThreads to default to true
• 6d155a4: 8308167: SequencedMap::firstEntry throws NPE when first entry has null key or value
• 4b15349: 8304438: jcmd JVMTI.agent_load should obey EnableDynamicAgentLoading
• b3c9d67: 8309136: [JVMCI] add -XX:+UseGraalJIT flag
• 98b53c0: 8306112: Implementation of JEP 445: Unnamed Classes and Instance Main Methods (Preview)
• e970ddb: 8309170: CDS archive heap is always relocated for larger heap
• 4b8922f: 8308842: Consolidate exceptions thrown from Class-File API
• 2b38343: 8309416: Misstatement in semantics of methods in javax.lang.model.ElementFilter
• 73352b6: 8280994: [XWayland] Drag and Drop does not work in java -> wayland app direction
• ... and 23 more: https://git.openjdk.org/jdk/compare/931913fbb299fbed7485ab8229100e6e56d8bada...master

Your commit was automatically rebased without conflicts.

[openjdk]

@tschatzl @quadhier Pushed as commit a7a0913.

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

[quadhier]

Thanks again for your comments and reviews @lkorinth @tschatzl @stefank !