RFR rev1 (M): 8078556: Runtime: implement ranges (optionally constraints) for those flags that have them missing

Tue Oct 13 14:59:55 UTC 2015

Gerard,

This looks really good!  Reviewed.

Coleen

On 10/13/15 10:26 AM, gerard ziemski wrote:
> Thank you Yumin, Jiangli for the feedback.
>
> For the time being I'm going to leave AttachListenerTimeout range as 
> is. It was defined by Staffan Larsen and we can always tweak the value 
> later if we have a good reason. I agree that the value looks awkward. 
> The value is exactly 35 min, and why that is would be a question for 
> Staffan.
>
> I have updated the webrev 
> http://cr.openjdk.java.net/~gziemski/8078556_rev2 that only has 3 
> differences from rev1:
>
> #1 SharedSymbolTableBucketSize range min is now 2 [Jiangli]
>
> #2 Updated INITIAL_CONSTRAINTS_SIZE 45 [Dmitry, merged with GC flags 
> changes]
>
> #3 Updated INITIAL_RANGES_SIZES 204 [Dmitry, merged with GC flags 
> changes]
>
>
> Testing with the new value passes JPRT and RBT tests.
>
> Dmitry, Coleen, can I please have your final reviews?
>
>
> cheers
>
> On 10/12/2015 10:41 PM, Yumin Qi wrote:
>> Hi, Gerard
>>
>>    I am not the definer for ShareTableBucketSize --- Jangli first 
>> checked int code for its definition:
>>
>> changeset:   7561:05c08ab3cf65
>> parent:      7559:47ffd05828f9
>> user:        jiangli
>> date:        Wed Dec 17 23:34:52 2014 -0500
>> summary:     8059510: Compact symbol table layout inside shared archive.
>>
>> changeset:   7561:05c08ab3cf65
>> parent:      7559:47ffd05828f9
>> user:        jiangli
>> date:        Wed Dec 17 23:34:52 2014 -0500
>> summary:     8059510: Compact symbol table layout inside shared archive.
>>
>>
>> BTW, I only have one minor comment:
>>
>> *src/os/aix/vm/globals_aix.hpp: **product_pd(intx, 
>> AttachListenerTimeout, \*
>> *"Timeout in ms the attach listener waits for a request") \*
>> *+ range(0, 2147483) \ This is about 35 minutes. But the number looks 
>> awkward. I am ok if you keep it, better to change
>> to a reasonable reading symbol. Thanks Yumin *
>>
>>
>>
>> On 10/9/15, 11:47 AM, gerard ziemski wrote:
>>> (resending, fixed webrev link)
>>>
>>> hi all,
>>>
>>> Please review this rev1 change that implements ranges and constraint 
>>> for runtime flags.
>>>
>>> The purpose of the ranges is to define valid ranges, that are 
>>> guaranteed to be accepted and work when instantiating a
>>> VM. The VM is allowed to exit with an exception due to out of OS 
>>> resources (like memory), but it may not assert or
>>> prematurely exit with an error that produces hs_err and/or dump core.
>>>
>>> Summary of changes since rev0:
>>>
>>> #1 Punted the following flags:
>>>
>>> SharedBaseAddress
>>> SharedMiscCodeSize
>>> SharedMiscDataSize
>>> SharedReadOnlySize
>>> SharedReadWriteSize
>>>
>>> to task JDK-8138983 since their ranges are nontrivial [Coleen, 
>>> Jianghli]
>>>
>>> #2. Removed unused "Arguments::get_default_heap_base_min_address" 
>>> [Coleen]
>>>
>>> #3. Users may need to use "TraceRedefineClasses" with their own 
>>> custom events, so changed the max value to 0XFFFFFFFF
>>> [Dmitry, Dan]
>>>
>>> #4. All other feedback from Dmitry implemented, see below
>>>
>>> #5. PrintSafepointStatisticsTimeout and SafepointtimeoutDelat max 
>>> value is LP64_ONLY(max_intx/MICROUNITS)
>>> NOT_LP64(max_intx) because intx is 4bytes on 32bits architectures, 
>>> so dividing it by MICROUNITS would bring the value
>>> below default. I did, however, make sure to upcast the values to 
>>> jlong, so on 32bit they become 8byte long long, and
>>> so no overflow will occur when multiplying by MICROUNITS.
>>>
>>> #6. GuaranteedSafepointInterval range(0, max_intx) asserts 
>>> (overflows) on Windows64 (max_intx on 64bit is
>>> 0x7FFFFFFFFFFFFFFF), so I changed the range max to max_jint (ie. 
>>> 0x7FFFFFFF) which is roughly 25 days
>>>
>>>
>>> The following flags had their ranges defined by the the domain experts:
>>>
>>> AttachListenerTimeout: Staffan Larsen
>>> BiasedLockingBulkRebiasThreshold: Marcus Larsson
>>> BiasedLockingBulkRevokeThreshold: Marcus Larsson
>>> BiasedLockingDecayTime: Marcus Larsson
>>> BiasedLockingStartupDelay: Marcus Larsson
>>> SharedSymbolTableBucketSize: Yumin Qi
>>> StackShadowPages: Coleen Phillimore, Frederic Parain
>>>
>>> For the rest of the flags, I looked at the existing code that was 
>>> using them and also did local tests:
>>>
>>> DeferPollingPageLoopCount
>>> DeferThrSuspendLoopCount
>>> GuaranteedSafepointInterval
>>> HeapBaseMinAddress
>>> InitialBootClassLoaderMetaspaceSize
>>> JavaPriority10_To_OSPriority
>>> JavaPriority1_To_OSPriority
>>> JavaPriority2_To_OSPriority
>>> JavaPriority3_To_OSPriority
>>> JavaPriority4_To_OSPriority
>>> JavaPriority5_To_OSPriority
>>> JavaPriority6_To_OSPriority
>>> JavaPriority7_To_OSPriority
>>> JavaPriority8_To_OSPriority
>>> JavaPriority9_To_OSPriority
>>> MallocMaxTestWords
>>> MallocVerifyInterval
>>> MallocVerifyStart
>>> MaxDirectMemorySize
>>> MaxJNILocalCapacity
>>> MaxJavaStackTraceDepth
>>> MaxRAM
>>> MinPassesBeforeFlush
>>> PerfDataMemorySize
>>> PerfDataSamplingInterval
>>> PerfMaxStringConstLength
>>> PrintSafepointStatisticsCount
>>> PrintSafepointStatisticsTimeout
>>> ProfileIntervalsTicks
>>> RTMRetryCount
>>> SafepointSpinBeforeYield
>>> SafepointTimeoutDelay
>>> SelfDestructTimer
>>> SuspendRetryCount
>>> SuspendRetryDelay
>>> ThreadSafetyMargin
>>> ThreadStackSize
>>> TraceRedefineClasses
>>> VMThreadPriority
>>> VMThreadStackSize
>>>
>>> The change passes “JPRT -testset hotspot”, “RBT 
>>> hotspot/test/runtime/CommandLine” and “RBT testlist,noncolo.testlist
>>> --add-tonga-keyword quick”
>>>
>>> References:
>>>
>>>  bugid: https://bugs.openjdk.java.net/browse/JDK-8078556
>>> webrev: http://cr.openjdk.java.net/~gziemski/8078556_rev1
>>>
>>> Followup issues:
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8133564 "Runtime - 2nd 
>>> followup to JDK-8059557"
>>> https://bugs.openjdk.java.net/browse/JDK-8136766 "Enable 
>>> ThreadStackSize range test”
>>> https://bugs.openjdk.java.net/browse/JDK-8134691 
>>> "CommandLineFlagConstraint::AtParse is not useful and not what we need"
>>> https://bugs.openjdk.java.net/browse/JDK-8138983 "Runtime: implement 
>>> ranges for Shared*** flags"
>>>
>>> Thank you.
>>>
>>>