RFR: 8263387: G1GarbageCollection JFR event gets gc phase, not gc type [v2]

[Albert Mingkun Yang]

On Mon, 15 Mar 2021 12:17:29 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Hi all,
>> 
>>   can I get reviews for this change that fixes some mess-up with JFR events, where the "G1YoungCollection" event should give the name of the current (G1) garbage collection. Currently the code adds a new type that returns the GC phase similar to shown in the tuning guide in [Fig 7-2](https://docs.oracle.com/en/java/javase/15/gctuning/garbage-first-g1-garbage-collector1.html#GUID-F1BE86FA-3EDC-4D4F-BDB4-4B044AD83180). Which might not be as expected, but due to when it's called, the user strings are the usual "Normal" / "Mixed" / "Concurrent Start" that are well-known for pauses.
>> 
>> It also showed that the phase enum we carry around internally has only been used for this purpose, so it can be deleted. While at it, the JFR event can now show the "new" pauses ("Prepare Mixed" and the "Concurrent Start" variants).
>> 
>> There are three commits in this series:
>> - Initial commit: implements the changes in the current coding style
>> - Rename file: I renamed `g1GCTypes.hpp` to the now more fitting `g1GCPauseType.hpp`
>> - enum class changes: where I attempted to turn the C enum `G1GCPauseTypes` into an enum class to make things a bit more c++'y. Not sure if it is a good change, it does not give a lot (apart from less polluting the global namespace a bit), but I left it here to see the opinions of others.
>> 
>> Testing: tier1-5, checked some JFR recordings for the event and its description
>> 
>> Thanks,
>>   Thomas
>
> Thomas Schatzl has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - sjohanss review
>  - iwalulya albert review

Marked as reviewed by ayang (Committer).

src/hotspot/share/gc/g1/g1GCPauseType.hpp line 85:

> 83:                                            "Mixed",
> 84:                                            "Full",
> 85:                                            "Invalid"};

`pause_strings[]` should not include `"Invalid"` any more, right?

-------------

PR: https://git.openjdk.java.net/jdk/pull/2960