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

[Erik Gahlin]

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/images/
>> 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)