PING: RFR: 8213015: Inconsistent settings between JFR.configure and -XX:FlightRecorderOptions

[Markus Gronlund]

Looks good.

Thanks
Markus

-----Original Message-----
From: Yasumasa Suenaga <yasuenag at gmail.com> 
Sent: den 22 november 2018 14:34
To: Erik Gahlin <erik.gahlin at oracle.com>
Cc: hotspot-jfr-dev at openjdk.java.net
Subject: Re: PING: RFR: 8213015: Inconsistent settings between JFR.configure and -XX:FlightRecorderOptions

Thanks Erik!

I will push this change after I get Reviewer(s).


Yasumasa


On 2018/11/21 21:54, Erik Gahlin wrote:
> Hi Yasumasa,
> 
> Regarding the test fix. By making options to JFR.configure typed 
> (MEMORY_SIZE), out of range values are caught in native and the 
> process exits with 1. We should probably change so the process exit 
> with 1 if incorrect values are found in Java as well, but it may be 
> outside the scope for this fix.
> 
> Looks good!
> 
> Thanks
> Erik
> 
> 
>> Hi,
>>
>> I changed TestJcmdConfigure.java in new webrev. Could you check it again?
>>
>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8213015/webrev.01/
>>
>> All test/jdk/jdk/jfr tests work fine on my environment, and tests on 
>> submit repo also works fine.
>> (mach5-one-ysuenaga-JDK-8213015-20181121-0533-11201)
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> On 2018/11/20 23:27, Yasumasa Suenaga wrote:
>>> Hi Markus,
>>>
>>> Thank you for your review.
>>> I guess you are saying about TestJcmdConfigure.
>>>
>>> ```
>>> Running DCMD 'JFR.configure maxchunksize=-500' through 'PidJcmdExecutor'
>>> Executing command
>>> '[/home/ysuenaga/OpenJDK/jdk/build/linux-x86_64-server-fastdebug/ima
>>> ges/ jdk/bin/jcmd, 82431, JFR.configure maxchunksize=-500]'
>>> Command returned with exit code 1
>>> ---------------- stdout ----------------
>>> 82431:
>>>
>>> ---------------- stderr ----------------
>>> java.lang.IllegalArgumentException: Parsing error memory size value:
>>> negative values not a
>>> llowed
>>>
>>> ----------------------------------------
>>>
>>> Error: Expected to get exit value of [0] ```
>>>
>>> I think this test shouldn't expect exit value 0.
>>> May I change this testcase?
>>>
>>>
>>> Yasumasa
>>>
>>>
>>>
>>> On 2018/11/20 18:48, Markus Gronlund wrote:
>>>> Looks OK but this will cause tests to fail.
>>>>
>>>> Please ensure you pass all tests in test/jdk/jdk/jfr.
>>>>
>>>> Thanks
>>>> Markus
>>>>
>>>> -----Original Message-----
>>>> From: Yasumasa Suenaga <yasuenag at gmail.com>
>>>> Sent: den 19 november 2018 14:32
>>>> To: hotspot-jfr-dev at openjdk.java.net
>>>> Subject: PING: RFR: 8213015: Inconsistent settings between 
>>>> JFR.configure and -XX:FlightRecorderOptions
>>>>
>>>> PING: Could you review it?
>>>>
>>>>>         JBS: https://bugs.openjdk.java.net/browse/JDK-8213015
>>>>>      webrev:
>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8213015/webrev.00/
>>>>
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>> On 2018/11/01 22:58, Yasumasa Suenaga wrote:
>>>>> Hi all,
>>>>>
>>>>>         JBS: https://bugs.openjdk.java.net/browse/JDK-8213015
>>>>>      webrev:
>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8213015/webrev.00/
>>>>>
>>>>> globalbuffersize, thread_buffer_size, memorysize, maxchunksize in 
>>>>> -XX:FlightRecorderOptions can be set as "MEMORY SIZE" type. OTOH 
>>>>> them in JFR.configure cannot be set so because they are defined as 
>>>>> JLONG.
>>>>>
>>>>> Also default values between JFR.configure and 
>>>>> -XX:FlightRecorderOptions are inconsistent - globalbuffercount and 
>>>>> memorysize. They should be consistent.
>>>>>
>>>>>
>>>>> Could you review it?
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Yasumasa (ysuenaga)
> 
>