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

[Coleen Phillimore]

Coming up with ranges for these flags and verifying them is a 
significant body of work.  This looks really good.

I have a couple comments.

----

These options Shared*Size should probably not have a minimum size of 
zero.   What happens if you run -Xshare:dump -XX:+SharedMiscSize=0, as 
an example?

I think it would be good to have definitions of 
DEFAULT_SHARED_READ_WRITE_SIZE = *NOT_LP64(12*M) LP64_ONLY(16*M),* or as 
constant values above in the file and have the ranges be min = default - 
1M and max = default + 1M or something like that.

Also SharedBaseAddress = 0 shouldn't be allowed, it should be at least 
 >= HeapBaseMinAddress.

----

I don't see any callers of Arguments::get_default_heap_base_min_adderss 
in your patch.

----

I don't have a problem with TraceRedefineClasses max range being 
0x80000000 even though the value is unused.  It won't cause any problems 
if it is set.

Thanks,
Coleen


On 9/18/15 12:49 PM, gerard ziemski wrote:
>
> hi all,
>
> Please review this 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.
>
> 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
> SharedBaseAddress: Yumin Qi
> SharedMiscCodeSize: Yumin Qi
> SharedMiscDataSize: Yumin Qi
> SharedReadOnlySize: Yumin Qi
> SharedReadWriteSize: Yumin Qi
> 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_rev0/
>
> 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"
>
> hg_stat:
>
>  src/cpu/aarch64/vm/globals_aarch64.hpp |  10 ++-
>  src/cpu/sparc/vm/globals_sparc.hpp |  16 ++++--
>  src/cpu/x86/vm/globals_x86.hpp |  18 +++++-
>  src/cpu/zero/vm/globals_zero.hpp |  11 +++-
>  src/os/aix/vm/globals_aix.hpp |   7 +-
>  src/share/vm/runtime/arguments.cpp |  16 +-----
>  src/share/vm/runtime/arguments.hpp |   2 +
>  src/share/vm/runtime/commandLineFlagConstraintList.cpp |   2 +-
>  src/share/vm/runtime/commandLineFlagConstraintList.hpp |   4 +-
>  src/share/vm/runtime/commandLineFlagConstraintsRuntime.cpp |  73 
> ++++++++++++++++++++++++++++++-
>  src/share/vm/runtime/commandLineFlagConstraintsRuntime.hpp |   9 +++
>  src/share/vm/runtime/commandLineFlagRangeList.cpp |   3 +-
>  src/share/vm/runtime/globals.hpp |  67 +++++++++++++++++++++++++--
>  src/share/vm/runtime/vmThread.cpp |   2 +-
>  test/runtime/CommandLine/OptionsValidation/TestOptionsWithRanges.java 
> |   7 ++
>  15 files changed, 202 insertions(+), 45 deletions(-)
>
>
> Thank you.
>