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

[sangheon.kim at oracle.com]

Hi Dmitry,  

thank you again for looking at this!

Thanks,
Sangheon


> On Nov 26, 2015, at 2:18 AM, Dmitry Dmitriev <dmitry.dmitriev at oracle.com> wrote:
> 
> Hi Sangheon,
> 
> Updated version also looks good to me!
> 
> Thanks,
> Dmitry
> 
>> On 26.11.2015 2:00, sangheon.kim wrote:
>> 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
>