RFR: 8246134: ZGC: Restructure hs_err sections
Stefan Karlsson
stefan.karlsson at oracle.com
Mon Jun 1 13:18:30 UTC 2020
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