RFR: 8156537: Tools using MonitoredVmUtil do not parse module in cmdline correctly
Dmitry Samersoff
dmitry.samersoff at oracle.com
Thu Jun 2 13:48:17 UTC 2016
Robbin,
The fix looks good for me.
-Dmitry
On 2016-06-02 09:41, Robbin Ehn 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
>>
More information about the serviceability-dev
mailing list