PING: RFR: 8220657: JFR.dump does not work when filename is set

[Yasumasa Suenaga]

PING: Do you have any opinion(s) about this fix?


Yasumasa


On 2019/04/01 21:46, Yasumasa Suenaga wrote:
> Hi,
> 
> Do you have concern(s) about this change?
> 
>>>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8220657/webrev.01/
> 
> As in the email below, I think it does not affect scenarios except snapshot cloning.
> Please tell me if you have any opinions about this.
> 
> 
> Thanks,
> 
> Yasumasa
> 
> 
> On 2019/03/20 10:37, Yasumasa Suenaga wrote:
>> Hi Erik,
>>
>> I think this change does not affect other scenarios.
>>
>> :jdk_jfr jtreg tests and submit repo have passed with this change.
>> newSnapshotClone() is called by PlatformRecording::dump and
>> DCmdDump::newSnapshot.
>> Both methods are related to user-requested dump.
>>
>> What scenarios do you think? I want to check them.
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> 2019年3月19日(火) 23:59 Erik Gahlin <erik.gahlin at oracle.com>:
>>>
>>> Yes, but then you would lose the original destination in other scenarios
>>>
>>> Maybe I can look at this next week.
>>>
>>> Erik
>>>
>>>> Hi Erik,
>>>>
>>>> Thank you for your reply.
>>>>
>>>> How about this changeset?
>>>>
>>>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8220657/webrev.01/
>>>>
>>>> I think PlatformRecording#newSnapshotClone() should not dump.
>>>> So I removed `setDestination()` from this.
>>>>
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>> On 2019/03/19 22:04, Erik Gahlin wrote:
>>>>> This looks a bit hackish.
>>>>>
>>>>> The real issue seems to be that the method
>>>>> PlatformRecording#newSnapshotClone incorrectly sets the destination in
>>>>> the clone, which then triggers a dump prematurely (when stop is called).
>>>>>
>>>>> Purpose of that method is just to take a snapshot of existing chunks in
>>>>> the disk repository (or from memory). and then let dumpStopped(path) do
>>>>> the actual dump (copy out the data from the disk repository).
>>>>>
>>>>> To make it work, some other changes would be needed as well, but I don't
>>>>> have time to investigate.
>>>>>
>>>>> Erik
>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> Please review this change.
>>>>>>
>>>>>>      JBS: https://bugs.openjdk.java.net/browse/JDK-8220657
>>>>>>      webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8220657/webrev.00/
>>>>>>
>>>>>> The user might not get flight record via JFR.dump if JFR is started
>>>>>> with filename option on target process.
>>>>>>
>>>>>> Please see JBS if you want to know how to reproduce.
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>