RFR: 8223438: add VirtualizationInformation JFR event

[Langer, Christoph]

Hi Matthias,

the change looks good to me.

In src/hotspot/cpu/ppc/vm_version_ppc.cpp, the correct SAP copyright header entry would be:
* Copyright (c) 2012, 2019 SAP SE. All rights reserved.
...without comma after 2019. Should look like in  src/hotspot/cpu/ppc/vm_version_ppc.hpp. Can you please fix that when pushing?

Also, in src/hotspot/cpu/ppc/vm_version_ppc.cpp, line 393, the call to perfstat_partition_total: Is 1 the success return code and if it returns 1, one can assume that PowerVM virtualization is available? If you can confirm that, it's fine ��

Best regards
Christoph

> -----Original Message-----
> From: hotspot-dev <hotspot-dev-bounces at openjdk.java.net> On Behalf Of
> Baesken, Matthias
> Sent: Dienstag, 14. Mai 2019 10:02
> To: Erik Gahlin <erik.gahlin at oracle.com>; 'hotspot-dev at openjdk.java.net'
> <hotspot-dev at openjdk.java.net>
> Subject: RE: RFR: 8223438: add VirtualizationInformation JFR event
> 
> Thanks Erik , good to hear !
> May I get a second review ?
> 
> Best regards, Matthias
> 
> 
> From: Erik Gahlin <erik.gahlin at oracle.com>
> Sent: Freitag, 10. Mai 2019 15:15
> To: Baesken, Matthias <matthias.baesken at sap.com>; 'hotspot-
> dev at openjdk.java.net' <hotspot-dev at openjdk.java.net>
> Subject: Re: RFR: 8223438: add VirtualizationInformation JFR event
> 
> Looks good.
> 
> Erik
> 
> Hello, here is   the next webrev  with  adjusted coding following your
> suggestion :
> 
> http://cr.openjdk.java.net/~mbaesken/webrevs/8223438.2/<http://cr.open
> jdk.java.net/%7Embaesken/webrevs/8223438.2/>
> 
> 
> Best regards, Matthias
> 
> 
> From: Baesken, Matthias
> Sent: Donnerstag, 9. Mai 2019 09:03
> To: Erik Gahlin <erik.gahlin at oracle.com><mailto:erik.gahlin at oracle.com>;
> 'hotspot-dev at openjdk.java.net<mailto:hotspot-dev at openjdk.java.net>'
> <hotspot-dev at openjdk.java.net><mailto:hotspot-dev at openjdk.java.net>
> Subject: RE: RFR: 8223438: add VirtualizationInformation JFR event
> 
> Hi   Erik,  the virtualization names  are a set of fix names ,  so I think we can do
> what you suggest .
> 
> I just "borrowed" a bit  from  the code above ...
> 
> TRACE_REQUEST_FUNC(OSInformation) {
>   ResourceMark rm;
>   char* os_name = NEW_RESOURCE_ARRAY(char, 2048);
>   JfrOSInterface::os_version(&os_name);
>   EventOSInformation event;
>   event.set_osVersion(os_name);
>   event.commit();
> }
> 
>    ... where a ResourceMark + allocation is used (but  in jfrOSInterface I
> wonder about the  second allocation in  string.stream.as_string , do we really
> need two here ?
> 
> int JfrOSInterface::JfrOSInterfaceImpl::os_version(char** os_version) const
> {
>   assert(os_version != NULL, "os_version pointer is NULL!");
>   stringStream os_ver_info;
>   os::print_os_info_brief(&os_ver_info);
>   *os_version = os_ver_info.as_string();       <--- second alloc
> 
> Best regards, Matthias
> 
> 
> 
> From: Erik Gahlin <erik.gahlin at oracle.com<mailto:erik.gahlin at oracle.com>>
> Sent: Mittwoch, 8. Mai 2019 18:55
> To: Baesken, Matthias
> <matthias.baesken at sap.com<mailto:matthias.baesken at sap.com>>;
> 'hotspot-dev at openjdk.java.net<mailto:hotspot-dev at openjdk.java.net>'
> <hotspot-dev at openjdk.java.net<mailto:hotspot-dev at openjdk.java.net>>
> Subject: Re: RFR: 8223438: add VirtualizationInformation JFR event
> 
> Hello,
> 
> Looks good, but do we really need to allocate the name every time an event
> is emitted in jfrPeriodic.cpp?
> 
> It will not matter much from a performance perspective, but the code would
> look cleaner if we would have.
> 
> EventVirtualizationInformation event;
> event.set_name(JfrOSInterface::virtualization_name());
> event.commit();
> 
> Thanks
> Erik
> Hello, please review the following change .
> It adds  a  simple VirtualizationInformation   event  to JFR .
> 
> The addition  to JFR  has been previously  proposed by Erik  when  we
> discussed the enhancement of hs_error file output  with virtualization
> related information .
> 
> Bug/webrev :
> 
> https://bugs.openjdk.java.net/browse/JDK-8223438
> 
> http://cr.openjdk.java.net/~mbaesken/webrevs/8223438.1/<http://cr.open
> jdk.java.net/%7Embaesken/webrevs/8223438.1/>
> 
> 
> Thanks, Matthias
>