RFR: 8246134: ZGC: Restructure hs_err sections
Per Liden
per.liden at oracle.com
Mon Jun 1 14:00:19 UTC 2020
Looks good!
/Per
> On 1 Jun 2020, at 15:18, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
>
> Updated webrev after Per's comments:
> https://cr.openjdk.java.net/~stefank/8246134/webrev.02
>
> StefanK
>
>> On 2020-06-01 10:24, Per Liden wrote:
>>> On 5/29/20 12:13 PM, Stefan Karlsson wrote:
>>> Hi all,
>>>
>>> Please review this small patch to restructure and cleanup the information ZGC prints to hs_err files (and jcmd VM.info).
>>>
>>> https://cr.openjdk.java.net/~stefank/8246134/webrev.01/
>>> https://bugs.openjdk.java.net/browse/JDK-8246134
>>>
>>> The patch:
>>> - Moves the Page Table dumping later, to make it easier to find the other sections
>>> - Pretty print some info
>>> - Add barrier set print (mostly to get rid of awkward double new lines)
>>> - Update titles and cleanup newlines
>>
>> Looks good. Some nits:
>>
>> 1) How about we move z_global_phase_string() to zGlobal.hpp/cpp and call it e.g. ZGlobalPhaseToString()?
>>
>>
>> 2) I find code like this unnecessarily hard to read:
>>
>> + switch (ZGlobalPhase) {
>> + case ZPhaseMark: return "Mark";
>> + case ZPhaseMarkCompleted: return "MarkCompleted";
>> + case ZPhaseRelocate: return "Relocate";
>> + default: assert(false, "Unknown ZGlobalPhase"); return "Unknown";
>>
>> How about:
>>
>> switch (ZGlobalPhase) {
>> case ZPhaseMark:
>> return "Mark";
>>
>> case ZPhaseMarkCompleted:
>> return "MarkCompleted";
>>
>> case ZPhaseRelocate:
>> return "Relocate";
>>
>> default:
>> assert(false, "Unknown ZGlobalPhase");
>> return "Unknown";
>> }
>>
>>
>> 3) I see it was like this before your change, but how about removing the extra space on all print_cr-lines, for example:
>>
>> 317 st->print_cr( "ZGC Globals:");
>>
>> to:
>>
>> 317 st->print_cr("ZGC Globals:");
>>
>> It also looks like the argument indentation here is off by one:
>>
>> 320 st->print_cr( " Offset Max: " SIZE_FORMAT "%s (" PTR_FORMAT ")",
>> 321 byte_size_in_exact_unit(ZAddressOffsetMax),
>> 322 exact_unit_for_byte_size(ZAddressOffsetMax),
>> 323 ZAddressOffsetMax);
>> 321 byte_size_in_exact_unit(ZAddressOffsetMax),
>> 322 exact_unit_for_byte_size(ZAddressOffsetMax),
>> 323 ZAddressOffsetMax);
>>
>> cheers,
>> Per
>>
>>>
>>> Thanks,
>>> StefanK
>
More information about the hotspot-gc-dev
mailing list