RFR: [JDK-8216565] Specifying the same path creates a new directory in JFR.configure

[Yasumasa Suenaga]

Hi,

On 2019/01/15 2:29, Erik Gahlin wrote:
> Hi Chihiro,,
> It would better (and safer) to just compare the references than using System.identityHashCode.

Does "safer" means two paths points different path?
If so, I think you can add sleep between two JFR.configure calls.

The resolution of time in Repository::REPO_DATE_FORMAT is seconds.
IMHO you can test this change "safely" with sleep 1 sec or longer.


Thanks,

Yasumasa


> Asserts.assertTrue(initialPath == samePath));
> …
> Asserts.assertFalse(initialPath == changedPath));
> 
> Thanks
> Erik
> 
> 
>> On 13 Jan 2019, at 10:59, Chihiro Ito <chihiro.ito at oracle.com> wrote:
>>
>> Hi,
>>
>> Thank you for reviewing.
>>
>> I fix test case and if-statement. Could you review this again ?
>>
>> webrev:
>> http://cr.openjdk.java.net/~cito/JDK-8216565/webrev.01/
>>
>> JBS:
>> https://bugs.openjdk.java.net/browse/JDK-8216565
>>
>> Regards,
>> Chihiro
>>
>> On 2019/01/12 11:05, Yasumasa Suenaga wrote:
>>> Hi Chihiro,
>>>
>>> The condition in if-statement is a little verbosely.
>>> I think it becomes more simple if you implement SafePath::equals and use it.
>>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>> On 2019/01/12 2:11, Chihiro Ito wrote:
>>> Hi Erik,
>>>
>>> Thank you for your review. I didn't have this idea.
>>>
>>> I'll fix it soon and request a review again.
>>>
>>> Regards,
>>> Chihiro
>>>
>>> On 2019/01/12 1:24, Erik Gahlin wrote:
>>>> Hi,
>>>>
>>>> Code looks good, but the 2 s sleep in the test unfortunate.
>>>>
>>>> Perhaps you could compare object identity instead? If it hasn't change, it's fine
>>>>
>>>> Erik
>>>>
>>>>> Hi all,
>>>>>
>>>>> I reported a following behabior as a bug. [JDK-8216565]
>>>>>
>>>>> Could I have a reviews for small patch, please ?
>>>>>
>>>>> -- 
>>>>> Even if you specify the same path as repositorypath in JFR.configure, a new subdirectory is created every time.
>>>>>
>>>>> $ ${JAVA_HOME}/bin/jcmd 17535 JFR.configure repositorypath=/tmp
>>>>> 17535:
>>>>> Repository path: /tmp/2019_01_12_00_28_53_17535
>>>>>
>>>>> $ ${JAVA_HOME}/bin/jcmd 17535 JFR.configure repositorypath=/tmp
>>>>> 17535:
>>>>> Repository path: /tmp/2019_01_12_00_29_03_17535
>>>>>
>>>>> If the same directory is specified, you should not create a new .
>>>>> -- 
>>>>>
>>>>> Regards,
>>>>> Chihiro
>>>>>
>>>>>
>>>>> On 2019/01/10 1:12, Chihiro Ito wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> Could I have a reviews for this fix of "jcmd JFR.configure", please ?
>>>>>> This patch passed all tests in test-tier1.
>>>>>>
>>>>>> http://cr.openjdk.java.net/~cito/webrev.00/
>>>>>>
>>>>>> In current implementation even if user specify the same repository base path in jcmd JFR.configure, hotspot create a new directory (base path + timestamp) as repository.
>>>>>>
>>>>>> This patch is hotspot don't create new directory and hotspot keep to use current one.
>>>>>>
>>>>>> Regards,
>>>>>> Chihiro
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>