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

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8156537: Tools using MonitoredVmUtil do not parse module in cmdline correctly
From:       Robbin Ehn <robbin.ehn () oracle ! com>
Date:       2016-06-03 7:36:38
Message-ID: 3dd4a76a-f771-de82-d581-892d9a2fcab8 () oracle ! com
[Download RAW message or body]

Thanks Staffan!

/Robbin

On 06/03/2016 09:20 AM, Staffan Larsen wrote:
> Looks good!
>
> Thanks,
> /Staffan
>
>> On 2 juni 2016, at 11:41, Robbin Ehn <robbin.ehn@oracle.com> wrote:
>>
>> Hi Dmitry, thanks for looking at this.
>>
>> On 06/01/2016 07:59 PM, Dmitry Samersoff wrote:
>>> Robbin,
>>>
>>> ProcessArgumentMatcher.java:61
>>>
>>>    What is the reason of keeping singlePid as a string. It might be
>>> better to convert it to Long at the argument processing time and then
>>> deal with Long.
>>
>> VirtualMachineDescriptor returns pid as string, see ProcessArgumentMatcher.java:116
>> So I'll prefer to keep it as a string.
>>
>> Rest should be fixed, also contains fixes after input from Staffan (thanks).
>>
>> Inc: http://cr.openjdk.java.net/~rehn/8156537/02-01/webrev/
>> Full: http://cr.openjdk.java.net/~rehn/8156537/02/webrev/
>>
>> /Robbin
>>
>>>
>>> Arguments.java:
>>>
>>>   null seems more natural to me than "0"
>>>
>>> JInfo.java:
>>>
>>>   54 value of flag is not obvious to me, because optionCount do exactly
>>> the same.
>>>
>>>    Update: it's the open question how we should proceed multiple -flag
>>> and -flags arguments.
>>>
>>> e.g. command line
>>>
>>>    jinfo -flags -flag MinHeapFreeRatio=1 pidA pidA
>>>
>>> results different output for the same process.
>>>
>>> I didn't find any documentation specified it, So I would prefer to
>>> disable the combination like one above. At least for now.
>>>
>>>   74 missed space
>>>
>>> MonitoredVmUtil.java:
>>>
>>>   119  Could you change tmp to something more descriptive? Or ever
>>> better move the code guessing lastFileSparator to a separate function
>>> for better readability.
>>>
>>> -Dmitry
>>>
>>> On 2016-06-01 10:16, Robbin Ehn wrote:
>>>> Hi all,
>>>>
>>>> There were some ripple effects, so to get everything working as intended
>>>> the patch grow slightly.
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~rehn/8156537/01/webrev/
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8156537
>>>>
>>>> Both a lot of manual testing, the new tests and
>>>> jdk/test/sun/tools/,jdk/test/tools/ and some other which uses e.g. jcmd.
>>>>
>>>> Thanks!
>>>>
>>>> /Robbin
>>>
>
[prev in list] [next in list] [prev in thread] [next in thread]