PING: RFR: 8233373: hs_err should report the location of JFR files

[coleen.phillimore at oracle.com]

This code looks fine to me.

http://cr.openjdk.java.net/~ysuenaga/JDK-8233373/webrev.01/src/hotspot/share/utilities/vmError.cpp.udiff.html

Small nit.  Can you change:

+ const char *path = NULL;
+
+ path = Jfr::get_emergency_dump_path();

to one line:

+ const char *path =Jfr::get_emergency_dump_path();


Thanks,
Coleen




On 1/18/20 9:19 PM, Yasumasa Suenaga wrote:
> PING: Could you review it?
>
>>    JBS: https://bugs.openjdk.java.net/browse/JDK-8233373
>>    webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8233373/webrev.01/
>
> I believe this enhancement helps troubleshooters for postmortem 
> analysis with JFR.
>
>
> Yasumasa
>
>
> On 2019/12/13 11:10, Yasumasa Suenaga wrote:
>> (Add hotspot-runtime-dev)
>>
>> PING: Could you review it?
>>
>>    JBS: https://bugs.openjdk.java.net/browse/JDK-8233373
>>    webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8233373/webrev.01/
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> On 2019/11/29 23:37, Yasumasa Suenaga wrote:
>>> Hi Markus,
>>>
>>> How about this?
>>>
>>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8233373/webrev.01/
>>>
>>> I do no agree to pass outputStream instance to JFR implementation in 
>>> HotSpot because we need to consider
>>> '#' and indents.
>>> I think it might be hard to maintain if "#" and indents would be 
>>> printed in different place.
>>>
>>> So I want to add getters for emergency dump path and repository path 
>>> to Jfr class.
>>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>> On 2019/11/29 19:26, Markus Gronlund wrote:
>>>> Hi,
>>>>
>>>> I have not looked at all the details yet, but from a high level:
>>>>
>>>> You should try to keep JFR specific logic encapsulated in JFR code 
>>>> as much as possible. jfrRepository.hpp and jfrEmergencyDump.hpp are 
>>>> internal modules, so they should not leak out into vmError.cpp.
>>>> The jfr.hpp is the external interface for the VM to interact 
>>>> directly with JFR. I would hope that there is only one invocation 
>>>> needed in vmError.cpp, something like 
>>>> JFR_ONLY(Jfr::on_error_reporting(outputStream* out);).
>>>> If there are multiple callouts needed to coordinate the layout, 
>>>> then you might need multiple entries in jfr.hpp, but please try to 
>>>> keep them to a minimum.  Pure textual output, such as aligning 
>>>> spaces and # can be printed directly in vmError.cpp of course.
>>>>
>>>> Thanks
>>>> Markus
>>>>
>>>>
>>>> -----Original Message-----
>>>> From: Yasumasa Suenaga <suenaga at oss.nttdata.com>
>>>> Sent: den 19 november 2019 02:32
>>>> To: hotspot-jfr-dev at openjdk.java.net
>>>> Cc: yasuenag at gmail.com
>>>> Subject: PING: RFR: 8233373: hs_err should report the location of 
>>>> JFR files
>>>>
>>>> PING: Could you review?
>>>>
>>>>>     JBS: https://bugs.openjdk.java.net/browse/JDK-8233373
>>>>>     webrev: 
>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8233373/webrev.00/
>>>>
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>> On 2019/11/01 13:38, Yasumasa Suenaga wrote:
>>>>> Hi all,
>>>>>
>>>>> Please review this change:
>>>>>
>>>>>     JBS: https://bugs.openjdk.java.net/browse/JDK-8233373
>>>>>     webrev: 
>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8233373/webrev.00/
>>>>>
>>>>> Since JFR Event Streaming (JEP 349), JFR file in the repository 
>>>>> would keep most recently data.
>>>>> It is very useful for postmortem analysis. So it helps JVM 
>>>>> engineer to acquire them from the user if hs_err log points it.
>>>>>
>>>>>
>>>>> Examples:
>>>>>
>>>>> * Show JFR repository path:
>>>>> ```
>>>>> # JFR files might be saved in the repository:
>>>>> # /tmp/2019_11_01_11_09_30_3529
>>>>> ```
>>>>>
>>>>> * Show emergency dump path
>>>>> ```
>>>>> # JFR data is saved as:
>>>>> # /home/ysuenaga/github/garakuta/NativeSEGV/hs_err_pid3807.jfr
>>>>> ```
>>>>>
>>>>>
>>>>> This webrev passed all tests on submit repo 
>>>>> (mach5-one-ysuenaga-JDK-8233373-20191101-0248-6329106)
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Yasumasa