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

[Chihiro Ito]

Hi Erik and Yasumasa,

I am considering what is the best test for this fix. please give me a 
advice for following question.

Do we need to verify that two objects are same one ?

This test verifies that it will not change when the same path is 
specified in JFR.configure. As long as the same path is specified, even 
if the results of Repository.getRepository().getRepositoryPath() are 
different objects, these objects should show the same path. We will not 
need to verify that it is the same object.

Regards,
Chihiro


On 2019/01/15 22:08, Yasumasa Suenaga wrote:
> 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
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>