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

[gerard ziemski]

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.
>>
>>