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

[Erik Gahlin]

Hi Chihiro,,
It would better (and safer) to just compare the references than using System.identityHashCode. 

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
>>>>> 
>>>>> 
>>>>> 
>>>> 
>>> 
>>> 
>> 
>