RFR: 8240532: heap inspection prints trailing @ after name of module without version

[Stefan Karlsson]

Thanks for reviewing, Lois.

StefanK

On 2020-03-04 18:34, Lois Foltan wrote:
> On 3/4/2020 12:12 PM, Stefan Karlsson wrote:
>> Hi all,
>>
>> Please review this patch to remove a trailing @ after module names of 
>> modules without a version.
>>
>> https://cr.openjdk.java.net/~stefank/8240532/webrev.01/
>> https://bugs.openjdk.java.net/browse/JDK-8240532
>>
>> The heap inspection functionality generates and prints a class 
>> histogram.
>>
>> Full image builds show module name and version as 'name at version':
>> [3,640s][trace][gc,classhisto      ] GC(1)    4: 5067 162144 
>> java.lang.String (java.base at 15-internal)
>>
>> Exploded builds don't have a version and prints 'name@'
>> [4,361s][trace][gc,classhisto      ] GC(1)    3: 7144 228608 
>> java.lang.String (java.base@)
>>
>> This looks weird and I propose that we drop the trailing @ when 
>> there's no version string:
>> [4,361s][trace][gc,classhisto      ] GC(1)    3: 7144 228608 
>> java.lang.String (java.base)
>>
>> This affects the output of:
>> * -Xlog:classhisto*=trace
>> * jcmd <pid> GC.class_histogram
>> * inspectheap attach operation
>> * jmap -histo
>>
>> I've run the tests I could find that were using these operations. 
>> Other than that I intend to run tier1-3.
>>
>> Do others agree with this change?
>
> Yes, I think this change looks good and is consistent with how it is 
> currently handled in javaClasses.cpp/print_stack_element_to_stream() 
> and frame.cpp/frame::print_on_error() for example.  However, all of 
> these places should really be calling 
> ModuleEntry::should_show_version() instead of directly checking if the 
> version() is NULL.  But that could be addressed in a differing RFE.
>
> Thanks,
> Lois
>
>>
>> Thanks,
>> StefanK
>