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 <yasue...@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-...@openjdk.java.net"
<hotspot-runtime-...@openjdk.java.net>
Cc: Gerard Ziemski <gerard.ziem...@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 <yasue...@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








Reply via email to