RFR(S): 8225388: Running jcmd Compiler.CodeHeap_Analytics all 0 cause crash.

[Leonid Mesnik]

I read documentation about CRS on following wiki https://wiki.openjdk.java.net/display/csr <https://wiki.openjdk.java.net/display/csr>. It says that CRS requires if command-line options are changes.
However I believe that fix don't change actual behavior of 'granularity' option so no CSR is needed for this small help update.

Any advise is welcome.

Leonid 

> On Jun 7, 2019, at 9:48 AM, serguei.spitsyn at oracle.com wrote:
> 
> Okay then.
> I wonder now, if a CSR needs to be filed for this change.
> 
> Thanks,
> Serguei
> 
> 
> On 6/7/19 09:10, Leonid Mesnik wrote:
>> Hi
>> 
>> Currently  DCmdArgument<jlong> is used to parse any numeric values in dcmd framework including positive integer values for port, ttl etc.
>> 
>> Adding new type DCmdArgument<size_t> requires adding more specialized methods like
>> template <> void DCmdArgument<jlong>::parse_value(const char* str,
>>                                                   size_t len, TRAPS) {
>> 
>> and other to parse and validate any *integer* parameters. See
>> http://hg.openjdk.java.net/jdk/jdk/file/47ee6c00d27c/src/hotspot/share/services/diagnosticArgument.cpp#l112 <http://hg.openjdk.java.net/jdk/jdk/file/47ee6c00d27c/src/hotspot/share/services/diagnosticArgument.cpp#l112>
>> 
>> I think that it is easier to use single jlong for all integer like it is done now rather than adding more types.
>> 
>> One might said that it would be better to add size_t type and use it for all non-negative integers. It would be good improvement for all dcmd args parsing. As well as overall improvement of parameters validation. (dcmd often don't throw exception and just return in the case of incorrect arguments). But sees like separate effort for whole dcmd framework.
>>  
>> 
>> Leonid
>>> On Jun 7, 2019, at 1:37 AM, serguei.spitsyn at oracle.com <mailto:serguei.spitsyn at oracle.com> wrote:
>>> 
>>> Hi Leonid,
>>> 
>>> It looks good to me.
>>> One minor comment on the src/hotspot/share/services/diagnosticCommand.?pp
>>> +  DCmdArgument<jlong> _granularity;
>>> 
>>> I'm curios if using size_t instead of jlong as in other files would be more unified.
>>> 
>>> Thanks,
>>> Serguei
>>> 
>>> 
>>> 
>>> On 6/6/19 17:36, Leonid Mesnik wrote:
>>>> Hi 
>>>> 
>>>> Could you please review following fix which verify parameter for Compiler.CodeHeap_Analytics command. So jcmd just exits instead of crashing target VM. Regression test was added, hs-tier1/2 passed. 
>>>> 
>>>> webrev: http://cr.openjdk.java.net/~lmesnik/8225388/webrev.00/ <http://cr.openjdk.java.net/~lmesnik/8225388/webrev.00/> 
>>>> 
>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8225388 <https://bugs.openjdk.java.net/browse/JDK-8225388> 
>>>> 
>>>> Leonid 
>>>> 
>>>> 
>>> 
>> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20190607/b3b5f117/attachment.html>