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