RFR 8130459(M): Add additional validation after heap creation
gerard.ziemski at oracle.com
Wed Jul 15 17:34:24 UTC 2015
I like how you map String constraint types into enums, so we don't have
to do string comparisons later.
I only have two comments:
line 93: I don't like the "find_if_validated()" name for the API. If I
understand the intend of the API correctly it should be something like
"find_if_needs_check()", or better yet
line 96, 98: We seem to be mixing up the API name convention used here.
I used something_foo(), but you are using somethingFoo(), can you please
change validateAfterParse() and setValidatingType() to
validate_after_parse() and set_validating_type() respectively?
On 07/15/2015 02:33 AM, sangheon.kim wrote:
> Hi all,
> Please review this change of adding additional validation on
> command-line flag checking framework.
> Recently we introduced Command-line flag checking framework
> (JDK-8059557 <https://bugs.openjdk.java.net/browse/JDK-8059557>, JEP
> 245: Validate JVM Command-Line Flag Arguments ) for ranges and
> And most cases this works great.
> Unfortunately there are some flags which can be checked after heap
> creation and these flags are hard to be checked under current framework.
> In this regard, I suggest to have an additional constraint checking.
> This suggestion adds one more constraint checking stage after memory
> And if we need more validations we can extend it.
> There are 2 big changes.
> 1. Flag declaration.
> 'constraint(FUNC)' -> 'constraint(FUNC,TYPE)'
> This webrev has 3 types, Anytime, AfterParse and AfterMemoryInit.
> - Anytime: current constraint functions which don't use
> - AfterParse: constraint functions which use
> - AfterMemoryInit: I added only YoungPLABSize as an example.
> * All GC related flags will be covered by JDK-8078555
> <https://bugs.openjdk.java.net/browse/JDK-8078555>: GC: implement
> ranges (optionally constraints) for those flags that have them missing.
> 2. Previous constraint functions have to use
> 'CommandLineFlags::finishedInitializing()' if the flag needs to be
> checked after Argument::parse().
> However, with this suggestion there's no need to call the function.
> Instead framework will decide when the constraint function is needed
> to be called.
> And previously constraint functions are called twice while this change
> will call each constraint function only one time.
> CR: https://bugs.openjdk.java.net/browse/JDK-8130459
> webrev: http://cr.openjdk.java.net/~sangheki/8130459/webrev.00/
> Testing: JPRT, UTE(vm.quick-pcl) and tests at test/runtime/CommandLine.
More information about the hotspot-dev