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

Lindenmaier, Goetz goetz.lindenmaier at sap.com
Wed Oct 14 10:47:25 UTC 2015


Hi Gerald, 

I think the code below would be a good fix for ThreadStackSize.
There should be at least 4 pages, red, yellow, shadow, and some
space to be used by the thread.  And the values should be divided by
1K.

   product_pd(intx, ThreadStackSize,                                         \
           "Thread Stack Size (in Kbytes)")                                  \
-          range(0, max_intx-os::vm_page_size())                             \
+          range((4*os::vm_page_size())/(1 * K),                             \
+                (max_intx-os::vm_page_size())/(1 * K))                      \
                                                                             \
   product_pd(intx, VMThreadStackSize,                                       \
-          "Non-Java Thread Stack Size (in Kbytes)")                         \
+          "Non-Java Thread Stack Size (in Kbytes). 0: an ergonomic default" \
+          " is used.")                                                      \
           range(0, max_intx/(1 * K))                                        \
                                                                             \
   product_pd(intx, CompilerThreadStackSize,                                 \

I also extended the comment for VMThreadStackSize.

With this, the range test passes on the ppc machines with 64K page
size.  It seems to ignore the crashes because of too few memory,
but I still find all the hs_err files.
Your change resolves my issue 
https://bugs.openjdk.java.net/browse/JDK-8134396
thanks for that!

Best regards,
  Goetz.




-----Original Message-----
From: Lindenmaier, Goetz 
Sent: Mittwoch, 14. Oktober 2015 11:01
To: 'gerard ziemski'; 'Dmitry Dmitriev'; 'hotspot-runtime-dev at openjdk.java.net'; 'Coleen Phillimore'
Subject: RE: RFR rev1 (M): 8078556: Runtime: implement ranges (optionally constraints) for those flags that have them missing

Hi Gerald, 

I had a closer look at your change now, especially on the settings of the 
stack protection zone sizes.

You set the lower bounds of the stack protection pages > 1. Because of that 
the VM on PPC does not start up, it immediately fails with:
  intx StackYellowPages=1 is outside the allowed range [ 6 ... 11 ]
  intx StackShadowPages=1 is outside the allowed range [ 8 ... 38 ]
  Error: Could not create the Java Virtual Machine.
  Error: A fatal exception has occurred. Program will exit.

The lower bounds of these flags must be '1'.  If vm_page_size() > vm_default_page_size(),
the numbers of pages are reduced.  This is necessary 
on systems with page size 64K, else stacks get too big.  We have such 
systems on ppc.  See also os_linux.cpp:4649.

http://cr.openjdk.java.net/~gziemski/8078556_rev2/src/cpu/sparc/vm/globals_sparc.hpp.udiff.html
Why do you need to special-case for !LP64 on sparc?  As I understand, 
this is no more supported in jdk9.

in globals.hpp:
http://cr.openjdk.java.net/~gziemski/8078556_rev2/src/share/vm/runtime/globals.hpp.udiff.html

You might want to use SIZE_MAX for MaxDirectMemorySize:
   product(size_t, MaxDirectMemorySize, 0,                                   \
+           range(0, (size_t)max_uintx)                                      \

This lower bound setting crashes the VM on linuxx86_64 and seems pointless:
    product_pd(intx, ThreadStackSize,                                         \
+           range(0, max_intx-os::vm_page_size())                             \
bin/java -XX:ThreadStackSize=0
#  Internal Error (.../src/os/linux/vm/os_linux.cpp:720), pid=47259, tid=47260
#  assert(JavaThread::stack_size_at_create() > 0) failed: this should be set

More crashes I see on linuxx86_64:
bin/java -XX:MallocMaxTestWords=1
# There is insufficient memory for the Java Runtime Environment to continue.
# Native memory allocation (malloc) failed to allocate 18 bytes for AllocateHeap

bin/java -XX:VMThreadStackSize=9007199254740991 / 4294967296 / 2147483648
# There is insufficient memory for the Java Runtime Environment to continue.
# Cannot create worker GC thread. Out of system resources.

and on linuxppc64:
/sapmnt/home/d045726/oJ/vms/ppc_9-1/bin/java -XX:MaxRAM=18446744073709551615
OpenJDK 64-Bit Server VM warning: INFO: os::commit_memory(0x0000000082000000, 32178700288, 0) failed; error='Cannot allocate memory' (errno=12)
# There is insufficient memory for the Java Runtime Environment to continue.
# Native memory allocation (mmap) failed to map 32178700288 bytes for committing reserved memory.

I saw these errors trying to run 
runtime/CommandLine/OptionsValidation/TestOptionsWithRanges.java
with a debug build.

If you need any more input, especially on ppc, I'm happy to help out!

Best regards,
  Goetz.


-----Original Message-----
From: Lindenmaier, Goetz 
Sent: Mittwoch, 14. Oktober 2015 08:34
To: 'gerard ziemski'; Dmitry Dmitriev; hotspot-runtime-dev at openjdk.java.net; Coleen Phillimore
Subject: RE: RFR rev1 (M): 8078556: Runtime: implement ranges (optionally constraints) for those flags that have them missing

Hi Gerald, 

sorry I only look at this now.  
The PPC implementation is missing, the build fails right away.
I will send you a patch to apply on top of your change off-list
today.

Best regards,
  Goetz.




-----Original Message-----
From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-bounces at openjdk.java.net] On Behalf Of gerard ziemski
Sent: Dienstag, 13. Oktober 2015 19:19
To: Dmitry Dmitriev; hotspot-runtime-dev at openjdk.java.net; Coleen Phillimore
Subject: Re: RFR rev1 (M): 8078556: Runtime: implement ranges (optionally constraints) for those flags that have them missing

hi Dmitry,

Excellent findings. I lost a few changes due to the sync-up to TOT to get the GC flag fixes.


On 10/13/2015 11:03 AM, Dmitry Dmitriev wrote:
> Hi Gerard,
>
> Thank you for fixing issues from revision 0, overall looks good, but I found that some of these issues are still exist.
> Here are few small issues:
> 1) BiasedLockingBulkRevokeThresholdFunc constraint function still have(line 97, commandLineFlagConstraintsRuntime.cpp)
> "(double)value/(double)BiasedLockingDecayTime >= 0.1" instead of > 0.1 as in corrected BiasedLockingDecayTimeFunc
> constraint function.

Redone.

>
> 2) Following code still exist in hotspot/src/share/vm/runtime/safepoint.cpp module(lines 1055-1057):
>    if (PrintSafepointStatisticsCount <= 0) {
>      fatal("Wrong PrintSafepointStatisticsCount");
>    }
>
> PrintSafepointStatisticsCount flag have range (1, max_intx), thus as I remember we agreed that this code is redundant.
> Can you please remove them?
>

Redone.


> 3) You correct MaxJavaStackTraceDepth max range to max_intx/2 according to my comment from previous review, but not
> change max_intx to max_jint.  This flag is assigned to int variables and using max_intx can lead to overflow. Thus, I
> think that max_jint should be instead of max_intx.

This I think I missed, but you are right, on 64bit intx is 8 bytes, but int is 4, so we need max_jint (max_int) here.


> 4) I don't see changes to the TestOptionsWithRanges.java in rev2. It is possible that these changes are lost in sync
> with GC ranges or these changes are not more needed?

Redone.


I will need to re-submit RBT and JPRT to make 100% sure all is right here.


Thank you for the review!

> I don't need to see another webrev for that.
>
> Thank you,
> Dmitry
>
> On 13.10.2015 17:26, 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.
>>>>
>>>>
>
>


More information about the hotspot-runtime-dev mailing list