RFR: 8156537: Tools using MonitoredVmUtil do not parse module in cmdline correctly

Staffan Larsen staffan.larsen at oracle.com
Fri Jun 3 07:20:59 UTC 2016


Looks good!

Thanks,
/Staffan

> On 2 juni 2016, at 11:41, Robbin Ehn <robbin.ehn at 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
>> 



More information about the serviceability-dev mailing list