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