RFR(S): 8142341: GC: current flags need ranges to be implemented

[sangheon.kim]

Hi all,

Here's updated webrev which reflects changes by "JDK-8143038: [TESTBUG] 
TestOptionsWithRanges: allow excluding only a subset of tested values 
specified for a flag".

The only updated file is 
test/runtime/CommandLine/OptionsValidation/TestOptionsWithRanges.java.
Changed from "allOptionsAsMap.remove("flag name")" to 
"excludeTestMaxRange("flag name")".

FYI,  JDK-8143038 introduced separated exclude methods of "excludeTest, 
excludeTestMaxRange and excludeTestMinRange".

webrev:
http://cr.openjdk.java.net/~sangheki/8142341/webrev.01/
http://cr.openjdk.java.net/~sangheki/8142341/webrev.01_to_00/

Thanks,
Sangheon


On 11/24/2015 06:37 AM, sangheon.kim wrote:
> Hi Dmitry,
>
> Thank you for the review!
> Sure I will update my patch related to your enhancement.
>
> Thanks,
> Sangheon
>
>
> On 11/24/2015 06:30 AM, Dmitry Dmitriev wrote:
>> Hi Sangheon,
>>
>> Looks good to me! Thank you for fixing that. I hope that enhancement 
>> will be pushed today(currently in JPRT queue), so please update your 
>> patch after that!
>>
>> Thanks,
>> Dmitry
>>
>> On 21.11.2015 0:04, sangheon.kim wrote:
>>> Hi all,
>>>
>>> Could I have a couple of reviews for this change to add explicit 
>>> 'range' for flags?
>>>
>>> Previously, we added 'range' when it is needed to avoid assert/crash 
>>> but now explicit 'range' are needed for all flags.
>>> All flags should have 'range' or 'constraint' at least.
>>>
>>> CR: https://bugs.openjdk.java.net/browse/JDK-8142341
>>> Webrev: http://cr.openjdk.java.net/~sangheki/8142341/webrev.00
>>> Testing: JPRT, RBT 
>>> (hotspot/test/:hotspot_all,testlist,noncolo.testlist 
>>> --add-tonga-keyword quick), hotspot/test/runtime/CommandLine for 
>>> embedded
>>>
>>> * Summary
>>> 1. Added 3 new constraint functions.
>>>    1) HeapBaseMinAddress: Added to avoid an overflow after align up 
>>> and this flag makes hang up without constraint function. (newly 
>>> added as a part of GC work)
>>>    2) TLABWasteIncrement: Without this constraint function we don't 
>>> have problems (even many GCs happen). But added to avoid an overflow 
>>> at ThreadLocalAllocBuffer::record_slow_allocation(). There are also 
>>> separate CR for this overflow ( JDK-8143352 ).
>>>    3) NUMAInterleaveGranularity: Added to avoid an overflow after 
>>> align up.
>>>
>>> 2. Some flags have narrower range than their type.
>>>    1) Here's the reason why some flags should have different range. 
>>> (same order from globals.hpp)
>>> {flag type} {flag name}: (range), {comment}
>>> size_t NUMAInterleaveGranularity: (os::vm_allocation_granularity(), 
>>> max_uintx), there is a comment at numa_interleaving_init() that this 
>>> flag should be larger than vm_allocation_granularity().
>>> uintx CMSOldPLABReactivityFactor: (1,max_uintx), couldn't be zero as 
>>> used for multiplication
>>> uintx CMS_FLSPadding: (0, max_juint), used to set a function which 
>>> has 'unsigned int' type input parameter.
>>> uintx CMS_SweepPadding: (0, max_juint), used to set a function which 
>>> has 'unsigned int' type input parameter.
>>> intx CMSWaitDuration: (min_jint, max_jint), used to set a function 
>>> which has 'long' type input parameter.
>>> uintx PausePadding: (0, max_juint), used to set a function which has 
>>> 'unsigned int' type input parameter.
>>> uintx PromotedPadding: (0, max_juint), used to set a function which 
>>> has 'unsigned int' type input parameter.
>>> uintx SurvivorPadding: (0, max_juint), used to set a function which 
>>> has 'unsigned int' type input parameter.
>>> uintx AdaptiveSizePolicyCollectionCostMargin: (0, 100), as this flag 
>>> is divided by 100 I assume this is percentage.
>>> uintx GCTimeRatio: (0, max_juint), used to set a function which has 
>>> 'unsigned int' type input parameter.
>>> intx PrefetchCopyIntervalInBytes: (-1, max_jint)
>>> intx PrefetchScanIntervalInBytes:  (-1, max_jint)
>>> intx PrefetchFieldsAhead:  (-1, max_jint), I think these 3 Prefetch* 
>>> flags should have same upper limit and looking at their related 
>>> codes 'max_jint' seems appropriate ( no problem with 'max_jint' 
>>> during testing). However, as Prefetch::read/write() needs 'intx', 
>>> 'intx' also seems good but we have to fix some codes (maybe 
>>> including generated codes).
>>> uintx CPUForCMSThread: (0, max_juint), used to set a function which 
>>> has 'unsigned int' type input parameter.
>>> uintx ProcessDistributionStride: (0, max_juint), used to set uint 
>>> variable and used 'for loop' which has uint increment. i.e. for 
>>> (uint i=0; i<ProcessDistributionStride; i++)
>>> uintx CMSCoordinatorYieldSleepCount: (0, max_juint), used at 
>>> 'increment for loop()' as max value and the increment is uint.
>>> uintx CMSYieldSleepCount: (0, max_juint),  used at 'increment for 
>>> loop()' as max value and the increment is uint.
>>> uintx TLABRefillWasteFraction: (1, max_juint), used as a return 
>>> value of uint type function and for division. i.e. uint 
>>> GCTLABConfiguration::tlab_refill_waste_limit()() { return 
>>> TLABRefillWasteFraction; }
>>> uintx TLABWasteIncrement: (0, max_jint), type cast to (int)
>>> intx PrintCMSStatistics: (0,2), flag to choose print mode and we 
>>> only use " if (a !=0, >0, >1)".
>>> intx PrintFLSStatistics: (0,2), flag to choose print mode and we 
>>> only use " if (a !=0, >0, >1)".
>>> intx PrintFLSCensus: (0,1), flag to choose print mode and we only 
>>> use " if (a !=0, >0)".
>>> uintx GCDrainStackTargetSize: (0, max_juint), type cast to 'unsigned 
>>> int'
>>>
>>> (g1_globals.hpp)
>>> intx G1ConcRefinementThresholdStep: (0, max_jint), used to set (int) 
>>> type variable.
>>>
>>> 3. Excluded flags from TestOptionsWithRanges.java
>>> 1) Memory and thread related flags: tests for these flags will 
>>> consume too many resources from the system.
>>> 2) VMThreadStackSize: range work for this flag is not included in 
>>> this patch but I faced OOME several times, so excluded.
>>>
>>> * I will reflect Dmitry's enhancement(JDK-8143038: [TESTBUG] 
>>> TestOptionsWithRanges: allow excluding only a subset of tested 
>>> values specified for a flag) next time.
>>>
>>> Thanks,
>>> Sangheon
>>>
>>>
>>>
>>
>