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

Wed Sep 30 16:33:40 UTC 2015

hi Coleen,

Thank you for the review!

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

# java -Xshare:dump -XX:SharedMiscDataSize=0 -version
Java HotSpot(TM) 64-Bit Server VM warning:
The shared miscellaneous data space is not large enough
to preload requested classes. Use -XX:SharedMiscDataSize=<size>
to increase the initial size of shared miscellaneous data space.

The VM prints out the warning and exits with error code 2, so we are in fact good, but see below.

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

I like the approach, but how about we widen up the ranges a bit like so:

#define DEFAULT_SHARED_READ_WRITE_SIZE  (NOT_LP64(12*M) LP64_ONLY(16*M))
#define MIN_SHARED_READ_WRITE_SIZE      (DEFAULT_SHARED_READ_WRITE_SIZE/32)
#define MAX_SHARED_READ_WRITE_SIZE      (DEFAULT_SHARED_READ_WRITE_SIZE*32)

#define DEFAULT_SHARED_READ_ONLY_SIZE   (NOT_LP64(12*M) LP64_ONLY(16*M))
#define MIN_SHARED_READ_ONLY_SIZE       (DEFAULT_SHARED_READ_ONLY_SIZE/32)
#define MAX_SHARED_READ_ONLY_SIZE       (DEFAULT_SHARED_READ_ONLY_SIZE*32)

#define DEFAULT_SHARED_MISC_DATA_SIZE   (NOT_LP64(2*M) LP64_ONLY(4*M))
#define MIN_SHARED_MISC_DATA_SIZE       (DEFAULT_SHARED_MISC_DATA_SIZE/16)
#define MAX_SHARED_MISC_DATA_SIZE       (DEFAULT_SHARED_MISC_DATA_SIZE*16)

#define DEFAULT_SHARED_MISC_CODE_SIZE   (120*K)
#define MIN_SHARED_MISC_CODE_SIZE       (DEFAULT_SHARED_MISC_CODE_SIZE/8)
#define MAX_SHARED_MISC_CODE_SIZE       (DEFAULT_SHARED_MISC_CODE_SIZE*8)

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

Fixed.

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

Good catch - a leftover from a prototype. Fixed.

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

It was pointed out to me that the “unused” (which should be probably renamed to “reserved”) can be used for custom 
events. It’s no big deal to set it to 0xffffffff in my opinion, I just originally thought it was cleaner to go with the 
highest defined mask.

cheers

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