RFR (M): 8078556: Runtime: implement ranges (optionally constraints) for those flags that have them missing
Dmitry Dmitriev
dmitry.dmitriev at oracle.com
Fri Sep 25 13:57:51 UTC 2015
Hi Gerard,
Overall looks good, but I have several comments & questions:
1) BiasedLockingBulkRevokeThreshold and BiasedLockingBulkRebiasThreshold
Constraints for these flags states that
"BiasedLockingBulkRevokeThreshold >= BiasedLockingBulkRebiasThreshold",
thus I think that min range for BiasedLockingBulkRevokeThreshold should
be >= than min range for BiasedLockingBulkRebiasThreshold. Currenty
BiasedLockingBulkRevokeThreshold min range(0) is less than
BiasedLockingBulkRebiasThreshold min range(1).
2) Typo in src/share/vm/runtime/commandLineFlagConstraintsRuntime.cpp on
line 112: "BiasedLockingBulkRebiasThreshold" should be instead of
"BiasedLockingBulkRevokeThreshold".
3) Error message in BiasedLockingDecayTimeFunc and
BiasedLockingBulkRevokeThresholdFunc
functions(commandLineFlagConstraintsRuntime.cpp module) states that
ratio should be less or equal to 0.1, but "if" statement verifies that
">=0.1", i.e. 0.1 is not acceptable value.
4) I think that max range for "TraceRedefineClasses" should be
"max_juint" instead of "0x80000000". Each bit in TraceRedefineClasses
defines event to trace, here a part of the
hotspot/src/share/vm/prims/jvmtiRedefineClassesTrace.hpp(lines 38-74):
// 0x00000000 | 0 - default; no tracing messages
// 0x00000001 | 1 - name each target class before loading, after
// loading and after redefinition is completed
...
// 0x00000010 | 16 - unused
// 0x00000020 | 32 - unused
// 0x00000040 | 64 - unused
// 0x00000080 | 128 - unused
// 0x00000100 | 256 - previous class weak reference addition
// 0x00000200 | 512 - previous class weak reference mgmt during
// class unloading checks (GC)
...
// 0x40000000 | 1073741824 - unused
// 0x80000000 | 2147483648 - unused
Thus "0x80000000 == unused" mean that this bit just currently not used.
5) PrintSafepointStatisticsCount flag have range (1, max_intx), thus do
we need following code in
SafepointSynchronize::deferred_initialize_stat()
function(hotspot/src/share/vm/runtime/safepoint.cpp)?
if (PrintSafepointStatisticsCount <= 0) {
fatal("Wrong PrintSafepointStatisticsCount");
}
6) PrintSafepointStatisticsTimeout max range currently is max_intx, but
I think it should be divided by MICROUNITS, because
PrintSafepointStatisticsTimeout is multiplied on MICROUNITS(lines 1171
and 1237 hotspot/src/share/vm/runtime/safepoint.cpp) and this can lead
to the overflow if PrintSafepointStatisticsTimeout is greater than
(max_intx / MICROUNITS).
7) MaxJavaStackTraceDepth have max range equal to max_intx. This flag is
assigned to int variables and sometimes multiplied by 2 and then
assigned to the int variables. Thus, I think max range for this flags
should be (max_jint / 2) to avoid overflow.
8) GuaranteedSafepointInterval have max range equal to 0x7FFFFFFF(lines
3117, 3122), but I believe that max_jint have the same value and can be
used instead of 0x7FFFFFFF.
9) SafepointTimeoutDelay max range currently is max_intx, but I think it
should be divided by MICROUNITS, because SafepointTimeoutDelay is
multiplied on MICROUNITS(line 204
hotspot/src/share/vm/runtime/safepoint.cpp) and this can lead to the
overflow if SafepointTimeoutDelay is greater than (max_intx / MICROUNITS).
10) VMThreadStackSize have max range equal to "max_intx".
VMThreadStackSize is a size in Kbytes and code which use this flag
multiply this flag on a "K" value, thus I think that max range should be
"max_intx / (1 * K)".
11) PerfDataSamplingInterval have max range equal to "max_intx". It
passed as constructor argument StatSamplerTask object, but
StatSamplerTask object accepts "int" parameter, thus I think that max
range for this flag should be "max_jint".
Thanks,
Dmitry
On 18.09.2015 19:49, 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.
>
More information about the hotspot-runtime-dev
mailing list