RFR 8134995(M): [REDO] GC: implement ranges (optionally constraints) for those flags that have them missing

[Kim Barrett]

On Sep 10, 2015, at 8:01 PM, sangheon.kim <sangheon.kim at oracle.com> wrote:
> 
> Hi all,
> 
> Please review this patch for command-line validation for GC flags.
> This REDO patch is adding ranges and implementing constraint functions for GC flags.
> 
> Original CR of JDK-8078555 was backout as it made a test failure from 'TestOptionsWithRanges.java'.
> And also there were some discussion of OOM handling.
> 
> Most parts are same as JDK-8078555 except below:
> 1. Changed 'range' for some flags.
> 2. Excluded 3 flags for TestOptionsWithRanges.java test. These flags make this test unstable as it tries to allocate huge amount of memory.
> 
> And below are the suggestion note for JDK-8078555:
> 1. Exponential notation for 'double' type variable parse: Previously there were some discussion for maximum value for double type flags from code review of JDK-8059557 and JDK-8112746. And Kim and I decided not to add upper limit unless there are problems with DBL_MAX. And as 255 is the maximum length that can be passed via command-line, we introduced exponential notation to avoid this limit. ( arguments.cpp )
> 2. These GC flags ranges are not ideal ranges but ranges which don't make problem with current source code.
>    If one flag makes some problem but hard to find good range, I added some ranges.
> 3. There are some constraint functions to avoid overflow.
> 4. Test applications are changed: as some of them assumed to be ParallelGC or to check it's output messages.
> 5. Includes cleanup of JDK-8133565: GC -2nd followup to JDK-8059557.
> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8134995
> 
> Webrev:
> http://cr.openjdk.java.net/~sangheki/8134995/webrev.00/
> http://cr.openjdk.java.net/~sangheki/8134995/webrev.00_to_8078555
> 
> Testing:
> JPRT, UTE(vm.quick-pcl) and test/runtime/CommandLine/OptionsValidation/TestOptionsWithRanges.java.

------------------------------------------------------------------------------ 
src/share/vm/gc/g1/g1_globals.hpp 
 142   product(intx, G1ConcRefinementServiceIntervalMillis, 300,                 \
...
 145           range(0, max_jint)                                                \

Why is this being changed from max_intx to max_jint?

------------------------------------------------------------------------------  
src/share/vm/gc/g1/g1_globals.hpp 
 169   develop(intx, G1RSetRegionEntriesBase, 256,                               \
 173   product(intx, G1RSetRegionEntries, 0,                                     \
 179   develop(intx, G1RSetSparseRegionEntriesBase, 4,                           \
 184   product(intx, G1RSetSparseRegionEntries, 0,                               \
 240   product(uintx, G1ConcRefinementThreads, 0,                                \

All of these have their max range value changed to be divided by 4.
What's this about?  What is this magic "4"?

------------------------------------------------------------------------------  
src/share/vm/gc/g1/g1_globals.hpp 
 240   product(uintx, G1ConcRefinementThreads, 0,                                \
...
 243           range(0, (max_jint-1)/4)                                          \

I know we discussed the problem of thread counts.  I'd suggested
perhaps basing the upper bound on the number of processors.  (Some
care might be needed for uniprocessor systems.) I couldn't find any
followup on that suggestion though.  Such an approach would probably
let us remove this change:

test/runtime/CommandLine/OptionsValidation/TestOptionsWithRanges.java 
  58         allOptionsAsMap.remove("G1ConcRefinementThreads");

Note also that this
test/runtime/CommandLine/OptionsValidation/TestOptionsWithRanges.java 
  52         allOptionsAsMap.remove("CICompilerCount");
is another example of an impossibly huge thread count.
 
------------------------------------------------------------------------------