RFR: 8223438: add VirtualizationInformation JFR event

[Baesken, Matthias]

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.openjdk.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.openjdk.java.net/%7Embaesken/webrevs/8223438.1/>


Thanks, Matthias