Thank you for review. Leonid
> On Jun 7, 2019, at 11:29 AM, serguei.spit...@oracle.com wrote: > > On 6/7/19 11:23, Leonid Mesnik wrote: >> 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. > > I see now. > Agreed, this dos not require a CSR. > > Thanks, > Serguei > > >> >> Any advise is welcome. >> >> Leonid >> >>> On Jun 7, 2019, at 9:48 AM, serguei.spit...@oracle.com >>> <mailto:serguei.spit...@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.spit...@oracle.com >>>>> <mailto:serguei.spit...@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 >>>>>> >>>>>> >>>>> >>>> >>> >> >
