8353559: Restructure CollectedHeap error printing

[jsikstro]

Calling Universe::heap()->print_on_error() gets dispatched to the most specific implementation, which for some GCs is their own implementation instead of the default in CollectedHeap. Each GC-specific implementation calls back to CollectedHeap::print_on_error(), which then dispatches back into the specific implementation of print_on(). This is kind of awkward and creates a call-chain that's not straightforward to wrap your head around, jumping back and forth via CollectedHeap and the specific implementation.

To make the call-chain cleaner, I have made print_on_error() a pure virtual method in CollectedHeap, and implemented print_on_error() in each GC's implementation of CollectedHeap. In addition, I have removed print_extended_on() from CollectedHeap and implemented that for the GCs that actually need/use it.

Removing the usage of the common print_on_error() also means that GCs that do not print anything interesting for their barrier set can omit this. So, I've removed it from ZGC and Shenandoah.

To make print_on_error() consistent with print_on(), I have moved the printing of "Heap:" to the caller(s) of print_on_error() (only inside vmError.cpp). This is a trivial change for all GCs except ZGC, which requires some restructuring in its error printing.

The old and new printing orders are shown below for ZGC:

# Old
<precious log>
<zgc globals>
<heap>
<zgc page table>
<barrier set>
<polling page>

# New
<precious log>
<heap>
<zgc globals>
<zgc page table>
<polling page>

Testing:

• GHA
• Tiers 1 & 2
• Manually verified that printing still works and outputs the intended information via running the following commands and comparing the output.

../fastdebug-old/jdk/bin/java -XX:ErrorHandlerTest=14 -XX:+ErrorFileToStdout -XX:+Use${gc}GC --version > ${gc}_old.txt
../fastdebug-new/jdk/bin/java -XX:ErrorHandlerTest=14 -XX:+ErrorFileToStdout -XX:+Use${gc}GC --version > ${gc}_new.txt

---

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

Issue

• JDK-8353559: Restructure CollectedHeap error printing (Enhancement - P4)

Reviewers

• Stefan Karlsson (@stefank - Reviewer)
• Erik Österlund (@fisk - Reviewer)
• Albert Mingkun Yang (@albertnetymk - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 24387

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

Using diff file

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

Using Webrev

Link to Webrev Comment

[bridgekeeper]

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

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

8353559: Restructure CollectedHeap error printing

Reviewed-by: stefank, eosterlund, 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 47 new commits pushed to the master branch:

• a13e34d: 8353274: [PPC64] Bug related to -XX:+UseCompactObjectHeaders -XX:-UseSIGTRAP in JDK-8305895
• 41d4a0d: 8352392: AIX: implement attach API v2 and streaming output
• 1c2a553: 8327858: Improve spliterator and forEach for single-element immutable collections
• a449aee: 8350704: Create tests to ensure the failure behavior of core reflection APIs
• 57df89c: 8353684: [BACKOUT] j.u.l.Handler classes create deadlock risk via synchronized publish() method
• ebcb9a8: 8349206: j.u.l.Handler classes create deadlock risk via synchronized publish() method
• d894b78: 8353588: [REDO] DaCapo xalan performance with -XX:+UseObjectMonitorTable
• db08726: 8352966: Opensource Several Font related tests - Batch 2
• 6b7b324: 8351431: Type annotations on new class creation expressions can't be retrieved
• 64b691a: 8271870: G1: Add objArray splitting when scanning object with evacuation failure
• ... and 37 more: https://git.openjdk.org/jdk/compare/6891490892cc0405882658e067d587ffe5401a6d...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.

[openjdk]

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

• hotspot
• 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.

[jsikstro]

/label remove hotspot
/label add hotspot-gc

[openjdk]

@jsikstro
The hotspot label was successfully removed.

[openjdk]

@jsikstro
The hotspot-gc label was successfully added.

[mlbridge]

Webrevs

• 00: Full (d7ec5133)