[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: PING: RFR: JDK-8155936: Boolean value should be set 1/0 or true/false via VM.set_flag jcmd
From:       Yasumasa Suenaga <yasuenag () gmail ! com>
Date:       2016-05-31 13:58:55
Message-ID: 9dd7f531-fdcc-4834-88cd-8f2c0b681659 () gmail ! com
[Download RAW message or body]

Hi all,

JDK-8155936 has been reviewed.
But Gerard and I think we should add more test for this feature
whether setting a boolean flag to "true" and "false" works
as expected.

So I uploaded new webrev again.
Changes from webrev.01 (reviewed) is for SetVMFlagTest.java only.

Could you review again?

   http://cr.openjdk.java.net/~ysuenaga/JDK-8155936/webrev.02/


Thanks,

Yasumasa


On 2016/05/18 2:36, Dmitry Samersoff wrote:
> Yasumasa,
>
> Looks good for me.
>
> -Dmitry
>
> On 2016-05-17 17:31, Yasumasa Suenaga wrote:
>> Hi Dmitry,
>>
>> Thank you for your comment.
>> I've fixed them in new webrev. Could you review again?
>>
>>   http://cr.openjdk.java.net/~ysuenaga/JDK-8155936/webrev.01/
>>
>>
>> Yasumasa
>>
>>
>> On 2016/05/17 19:25, Dmitry Samersoff wrote:
>>> Yasumasa,
>>>
>>> 1. Please use strcasecmp for true/false
>>>
>>> 2. You may save one strcmp call by replacing it to
>>>    (*arg == '0' && *(arg+1) == 0)
>>>
>>> -Dmitry
>>>
>>>
>>> On 2016-05-17 13:15, Yasumasa Suenaga wrote:
>>>> PING: Could you review it?
>>>> We need a reviewer.
>>>>
>>>>> bug id: https://bugs.openjdk.java.net/browse/JDK-8155936
>>>>> webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8155936/webrev.00/
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>> On 2016/05/06 1:18, Gerard Ziemski wrote:
>>>>> I'm including serviceability mailing list.
>>>>>
>>>>> bug id: https://bugs.openjdk.java.net/browse/JDK-8155936v
>>>>> webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8155936/webrev.00/
>>>>>
>>>>>
>>>>> cheers
>>>>>
>>>>>> Begin forwarded message:
>>>>>>
>>>>>> From: Yasumasa Suenaga <yasuenag@gmail.com>
>>>>>> Subject: Re: RFR: JDK-8155936: Boolean value should be set 1/0 or
>>>>>> true/false via VM.set_flag jcmd
>>>>>> Date: May 4, 2016 at 9:34:15 AM CDT
>>>>>> To: "hotspot-runtime-dev@openjdk.java.net"
>>>>>> <hotspot-runtime-dev@openjdk.java.net>
>>>>>> Cc: Gerard Ziemski <gerard.ziemski@oracle.com>
>>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> We still need a second Reviewer.
>>>>>> Could you review?
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>> On 2016/05/04 9:32, Yasumasa Suenaga wrote:
>>>>>>> Hi Gerard,
>>>>>>>
>>>>>>>> Reviewed and I will sponsor it.
>>>>>>>
>>>>>>> Thanks!
>>>>>>>
>>>>>>>> Just one question: there is no existing JDK issue covering this
>>>>>>>> yet, is there? Can you file one please if none exists yet?
>>>>>>>
>>>>>>> I do not change in jdk repos.
>>>>>>> My change affects jinfo, however jtreg test for jinfo passed.
>>>>>>>
>>>>>>> I ran jtreg with two directories:
>>>>>>>
>>>>>>>  - hotspot/test/serviceability/dcmd/vm
>>>>>>>  - jdk/test/sun/tools/jinfo
>>>>>>>
>>>>>>> They work fine.
>>>>>>> (JInfoRunningProcessFlagTest is failed. But it is listed in
>>>>>>> ProblemList.)
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Yasumasa
>>>>>>>
>>>>>>>
>>>>>>> On 2016/05/04 3:18, Gerard Ziemski wrote:
>>>>>>>> hi Yasumasa,
>>>>>>>>
>>>>>>>> Thank you for the fix, I like it - the very first time I tried
>>>>>>>> using jcmd to set a boolean value I tried using "true", so this
>>>>>>>> will make jcmd easier to use.
>>>>>>>>
>>>>>>>> Reviewed and I will sponsor it.
>>>>>>>>
>>>>>>>> Just one question: there is no existing JDK issue covering this
>>>>>>>> yet, is there? Can you file one please if none exists yet?
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> cheers
>>>>>>>>
>>>>>>>>> On May 3, 2016, at 9:22 AM, Yasumasa Suenaga <yasuenag@gmail.com>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> Hi all,
>>>>>>>>>
>>>>>>>>> This review request relates to [1].
>>>>>>>>>
>>>>>>>>> We can change a part of -XX option via VM.set_flag jcmd.
>>>>>>>>> This jcmd requires 1 or 0 as boolean value.
>>>>>>>>> However, we can set 0 or not (NOT 1).
>>>>>>>>>
>>>>>>>>> In jinfo, we can set boolean value with 1/0 or +/-.
>>>>>>>>> So I think it is useful if VM.set_flag accept boolean value in
>>>>>>>>> true/false.
>>>>>>>>>
>>>>>>>>> I uploaded a webrev for this issue.
>>>>>>>>> Could you review it?
>>>>>>>>>
>>>>>>>>>  http://cr.openjdk.java.net/~ysuenaga/JDK-8155936/webrev.00/
>>>>>>>>>
>>>>>>>>> I cannot access JPRT.
>>>>>>>>> So I need a sponsor.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> Yasumasa
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> [1]
>>>>>>>>> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2016-April/019192.html
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>
>>>
>>>
>
>
[prev in list] [next in list] [prev in thread] [next in thread]