[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Re: RFR (M): 8142510: rev2: -XX:+PrintFlagsRanges should print default range value for those flags t
From:       Gerard Ziemski <gerard.ziemski () oracle ! com>
Date:       2016-03-30 15:58:11
Message-ID: 848A518F-0391-4D7D-AE18-175DD4EB68A0 () oracle ! com
[Download RAW message or body]

Thank you Coleen for the review.

> On Mar 29, 2016, at 5:33 PM, Coleen Phillimore <coleen.phillimore@oracle.com> \
> wrote: 
> 
> Hi, I think this looks good.
> 
> http://cr.openjdk.java.net/~gziemski/8142510_rev2/src/share/vm/runtime/globals.cpp.udiff.html
>  
> The only thing I'd change is to use mtInternal for the C_HEAP allocation rather \
> than mtLogging. I think all of the arguments NMT tracking should be mtCommand or \
> mtArg, rather than mtInternal but this should be consistent with that.  I'll file \
> an RFE that someone might want to pick up.

I thought mtLogging would apply here since the code is used to log the errors, but we \
can follow up on this as you suggested in a separate RFE.


> 
> thanks,
> Coleen
> 
> 
> On 3/17/16 9:52 AM, Gerard Ziemski wrote:
> > Thank you very much Dmitry!
> > 
> > > On Mar 17, 2016, at 8:29 AM, Dmitry Dmitriev <dmitry.dmitriev@oracle.com> \
> > > wrote: 
> > > Hi Gerard,
> > > 
> > > Looks good, except copyrights years in the following modules(forgot about that \
> > > in the first round): commandLineFlagConstraintList.cpp
> > > commandLineFlagConstraintList.hpp
> > > commandLineFlagRangeList.hpp
> > > 
> > > Not need a new webrev for that.
> > > 
> > > Thanks,
> > > Dmitry
> > > 
> > > On 16.03.2016 19:55, Gerard Ziemski wrote:
> > > > hi all,
> > > > 
> > > > This is rev2 of the fix incorporating feedback from Dmitry:
> > > > 
> > > > - re-use "CommandLineFlagConstraintList::find()"
> > > > 
> > > > Please review this enhancement to Command Line Options Validation JEP-245, \
> > > > which prints default ranges for those flags, that only have constraints (ie. \
> > > > no range, but a constraint, implies default range) 
> > > > With this fix we'll be able to include more flags in \
> > > > test/runtime/CommandLine/OptionsValidation test. 
> > > > 
> > > > bug https://bugs.openjdk.java.net/browse/JDK-8142510
> > > > webrev http://cr.openjdk.java.net/~gziemski/8142510_rev2
> > > > 
> > > > tested with JPRT hotspot, RBT hotspot/test/runtime and local \
> > > > test/runtime/CommandLine/OptionsValidation 
> > > > 
> > > > cheers
> 


[prev in list] [next in list] [prev in thread] [next in thread]