RFR(M): 8170655: [posix] Fix minimum stack size computations
Daniel D. Daugherty
daniel.daugherty at oracle.com
Tue Dec 13 16:48:35 UTC 2016
Goetz,
Does this latest webrev include the last of the code review changes that
you were holding onto?
Dan
On 12/13/16 9:22 AM, Lindenmaier, Goetz wrote:
> I messed up the link, the rebased webrev of 8170655 is
> http://cr.openjdk.java.net/~goetz/wr16/8170655-compilerGuardFix/webrev.03/
>
> Best regards,
> Goetz.
>
> (I'll resend the mail on the other mail thread anyways ...)
>
>> -----Original Message-----
>> From: Lindenmaier, Goetz
>> Sent: Dienstag, 13. Dezember 2016 17:16
>> To: 'daniel.daugherty at oracle.com' <daniel.daugherty at oracle.com>; David
>> Holmes <david.holmes at oracle.com>; Coleen Phillimore
>> <coleen.phillimore at oracle.com>; hotspot-runtime-dev at openjdk.java.net
>> Subject: RE: RFR(M): 8170655: [posix] Fix minimum stack size computations
>>
>> Hi,
>>
>> I rebased 8170655 to the recent changes in hotspot:
>> http://cr.openjdk.java.net/~goetz/wr16/8169373-ppc-stackFix/webrev.09/
>>
>> The patch is still in our test patch queue for the nightly tests, and there
>> were no failures I can attribute to this change.
>>
>> Best regards,
>> Goetz.
>>
>>
>>> -----Original Message-----
>>> From: Daniel D. Daugherty [mailto:daniel.daugherty at oracle.com]
>>> Sent: Dienstag, 13. Dezember 2016 16:45
>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; David Holmes
>>> <david.holmes at oracle.com>; Coleen Phillimore
>>> <coleen.phillimore at oracle.com>; hotspot-runtime-dev at openjdk.java.net
>>> Subject: Re: RFR(M): 8170655: [posix] Fix minimum stack size computations
>>>
>>> The current set of RBT test results looks bad. I suspect a mismatch
>>> between current test suite version(s) and the baseline that I used.
>>> I created the repo for 8169373 back on 2016.12.02 and the last
>>> changeset in hotspot at that time was dated 2016.11.30... so the
>>> baseline on which I applied your changes is almost 2 weeks out of
>>> date (pre-dates the latest Jigsaw refresh)...
>>>
>>> > Should I rebase the changes to the latest repo version?
>>>
>>> If you don't mind rebasing both patches (8169373 and 8170655) to the
>>> current JDK9-hs, that would be appreciated. Thanks!
>>>
>>> Dan
>>>
>>>
>>>
>>> On 12/13/16 8:17 AM, Daniel D. Daugherty wrote:
>>>> Both 8169373 and 8170655 are bug fixes so they do not have to be
>>>> in by the extended Feature Complete deadline. If they did, I would
>>>> be going through more Tums than I already am... :-)
>>>>
>>>> Dan
>>>>
>>>>
>>>>
>>>> On 12/13/16 8:02 AM, Lindenmaier, Goetz wrote:
>>>>> Hi Dan,
>>>>>
>>>>> that sounds bad ... thanks for your update! Yes, I also have the
>>>>> Feature
>>>>> Complete deadline in mind, that's why I would like to see the changes
>>>>> committed :) But obviously I currently can't help.
>>>>>
>>>>> Thanks,
>>>>> Goetz.
>>>>>
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Daniel D. Daugherty [mailto:daniel.daugherty at oracle.com]
>>>>>> Sent: Dienstag, 13. Dezember 2016 15:56
>>>>>> To: David Holmes <david.holmes at oracle.com>; Lindenmaier, Goetz
>>>>>> <goetz.lindenmaier at sap.com>; Coleen Phillimore
>>>>>> <coleen.phillimore at oracle.com>; hotspot-runtime-dev at openjdk.java.net
>>>>>> Subject: Re: RFR(M): 8170655: [posix] Fix minimum stack size
>>>>>> computations
>>>>>>
>>>>>> Hi Goetz,
>>>>>>
>>>>>> JDK9-hs is still closed for the run up to extended Feature Complete.
>>>>>> Should be open again sometime soon.
>>>>>>
>>>>>> I kicked off a "full RBT" run on 8169373 last Monday. It ran for four
>>>>>> days before it collided with a maintenance window on the Aurora DB.
>>>>>> I kicked off a "full RBT" run on 8170655 last Tuesday. It ran for three
>>>>>> days before it collided with the same brick wall.
>>>>>>
>>>>>> Neither RBT run is complete; in fact there are entire platforms with
>>>>>> zero results so they are pretty much useless. Normally RBT runs don't
>>>>>> take more than a couple of days, but our internal test systems are a
>>>>>> bit swamped due to the upcoming extended Feature Complete deadline.
>>>>>>
>>>>>> I did start another "full RBT" run on 8169373 on Sunday, but it is
>>>>>> not complete either. I was hoping to have all the testing done by
>>>>>> the time JDK9-hs reopened, but it looks like that is not going to
>>>>>> happen.
>>>>>>
>>>>>> Dan
>>>>>>
>>>>>>
>>>>>> On 12/13/16 1:41 AM, David Holmes wrote:
>>>>>>> On 13/12/2016 6:16 PM, Lindenmaier, Goetz wrote:
>>>>>>>> Hi Dan,
>>>>>>>>
>>>>>>>> what's the status of my changes? I think hs is open again, at least
>>>>>>>> I see changes popping up ... just a gentle reminder :)
>>>>>>> Actually it is closed (again?) other than for selective changes as we
>>>>>>> had to pull down the big jigsaw update from dev, and merge in with
>> AOT
>>>>>>> push, and a couple of other approved changes, and need things to
>>>>>>> stabilise so we can push up to dev.
>>>>>>>
>>>>>>> David
>>>>>>>
>>>>>>>> Should I rebase the changes to the latest repo version?
>>>>>>>>
>>>>>>>> Best regards,
>>>>>>>> Goetz.
>>>>>>>>
>>>>>>>>> -----Original Message-----
>>>>>>>>> From: Daniel D. Daugherty [mailto:daniel.daugherty at oracle.com]
>>>>>>>>> Sent: Dienstag, 6. Dezember 2016 22:15
>>>>>>>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Coleen
>>>>>>>>> Phillimore
>>>>>>>>> <coleen.phillimore at oracle.com>; hotspot-runtime-
>>> dev at openjdk.java.net
>>>>>>>>> Subject: Re: RFR(M): 8170655: [posix] Fix minimum stack size
>>>>>>>>> computations
>>>>>>>>>
>>>>>>>>> On 12/6/16 1:31 PM, Lindenmaier, Goetz wrote:
>>>>>>>>>> Hi Dan,
>>>>>>>>>>
>>>>>>>>>> I fixed the comment. I will post a new webrev once the other
>>>>>>>>>> change is pushed so that I can make a real change.
>>>>>>>>> I'll resync my repo to the webrev.02 version and restart my
>>>>>>>>> testing on this fix. I'm hoping we have only comment changes
>>>>>>>>> after the webrev.02 version...
>>>>>>>>>
>>>>>>>>> Also gotta check the test results on 8169373.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>>> Meta question: Now _java_thread_min_stack_allowed is
>>>>>>>>>>> specific to an
>>>>>>>>>>> OS/CPU platform (as are the other two). That OS/CPU
>>>>>>>>>>> platform
>>>>>>>>>>> enumeration
>>>>>>>>>>> doesn't take into account whether C1 or C2 is in play. We
>>>>>>>>>>> used to add
>>>>>>>>>>> just a little more space via the COMPILER2_PRESENT() macro,
>>>>>>>>>>> but now
>>>>>>>>> we
>>>>>>>>>>> don't. Yes, I know that you've done a bunch of testing and
>>>>>>>>>>> I'm running
>>>>>>>>>>> our usual batch of tests also, but this subtle change is
>>>>>>>>>>> bothering
>>>>>>>>>> Yep, I could have made sizing with C1, then C2, then
>>>>>>>>>> TieredCompilation,
>>>>>>>>>> but I don't think that would make a change.
>>>>>>>>>> This extra size was meant for the compiler thread. 83251780 moved
>>>>>>>>>> it to the
>>>>>>>>>> Java threads. I think it was there because C2 used to have
>>>>>>>>>> recursive
>>>>>>>>> algorithms.
>>>>>>>>>> But it only would have had an effect in rare situations because the
>>>>>>>>>> other MAX
>>>>>>>>>> should mostly be used on systems with large pages, where the
>>>>>>>>>> alignement
>>>>>>>>>> would bring more than these 4K. The compiler gained the
>> alignment
>>>>>>>>>> of the
>>>>>>>>>> vm_default_page_size() region plus the space gained by bigger
>>>>>>>>> StackShadowPages
>>>>>>>>>> which were huge at some point (20+ pages * 64K ...). Remember the
>>>>>>>>> compiler does
>>>>>>>>>> no stack banging and thus could use the shadow region.
>>>>>>>>>>
>>>>>>>>>> The original code looked like this:
>>>>>>>>>>
>>>>>>>>>>
>> http://hg.openjdk.java.net/jdk8/jdk8/hotspot/file/87ee5ee27509/src/os/linux/v
>>>>>>>>> m/os_linux.cpp
>>>>>>>>>> os::Linux::min_stack_allowed =
>>>>>>>>>> MAX2(os::Linux::min_stack_allowed,
>>>>>>>>>> (size_t)(StackYellowPages+StackRedPages+StackShadowPages) *
>>>>>>>>> Linux::page_size() +
>>>>>>>>>> (2*BytesPerWord COMPILER2_PRESENT(+1)) *
>>>>>>>>> Linux::vm_default_page_size());
>>>>>>>>>
>>>>>>>>> I'm good with this explanation. Coleen?
>>>>>>>>>
>>>>>>>>> Dan
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> Best regards,
>>>>>>>>>> Goetz.
>>>>>>>>>>
>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>> From: Daniel D. Daugherty [mailto:daniel.daugherty at oracle.com]
>>>>>>>>>>> Sent: Tuesday, December 06, 2016 8:32 PM
>>>>>>>>>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Coleen
>>>>>> Phillimore
>>>>>>>>>>> <coleen.phillimore at oracle.com>; hotspot-runtime-
>>>>>> dev at openjdk.java.net
>>>>>>>>>>> Subject: Re: RFR(M): 8170655: [posix] Fix minimum stack size
>>>>>>>>>>> computations
>>>>>>>>>>>
>>>>>>>>>>> On 12/6/16 2:35 AM, Lindenmaier, Goetz wrote:
>>>>>>>>>>>> Hi Coleen,
>>>>>>>>>>>>
>>>>>>>>>>>> thanks for looking at this change!
>>>>>>>>>>>>
>>>>>>>>>>>> New webrev:
>>>>>>>>>>>> http://cr.openjdk.java.net/~goetz/wr16/8170655-
>>>>>>>>>>> compilerGuardFix/webrev.02/
>>>>>>>>>>>
>>>>>>>>>>> src/cpu/ppc/vm/globals_ppc.hpp
>>>>>>>>>>> No comments.
>>>>>>>>>>>
>>>>>>>>>>> src/cpu/x86/vm/globals_x86.hpp
>>>>>>>>>>> No comments.
>>>>>>>>>>>
>>>>>>>>>>> src/os/posix/vm/os_posix.cpp
>>>>>>>>>>> L1112: // It dependes on word size, platform calling
>>>>>>>>>>> conventions, C
>>>>>>>>>>> frame layout and
>>>>>>>>>>> Typo: 'dependes' -> 'depends'
>>>>>>>>>>>
>>>>>>>>>>> If you put "It" at the end of the sentence on the
>>>>>>>>>>> previous
>>>>>>>>>>> line, then the formatting will look better. :-)
>>>>>>>>>>>
>>>>>>>>>>> Meta question: Now _java_thread_min_stack_allowed is
>>>>>>>>>>> specific to an
>>>>>>>>>>> OS/CPU platform (as are the other two). That OS/CPU
>>>>>>>>>>> platform
>>>>>>>>>>> enumeration
>>>>>>>>>>> doesn't take into account whether C1 or C2 is in play. We
>>>>>>>>>>> used to add
>>>>>>>>>>> just a little more space via the COMPILER2_PRESENT() macro,
>>>>>>>>>>> but now
>>>>>>>>> we
>>>>>>>>>>> don't. Yes, I know that you've done a bunch of testing and
>>>>>>>>>>> I'm running
>>>>>>>>>>> our usual batch of tests also, but this subtle change is
>>>>>>>>>>> bothering
>>>>>>>>>>> me...
>>>>>>>>>>>
>>>>>>>>>>> src/os/posix/vm/os_posix.hpp
>>>>>>>>>>> No comments.
>>>>>>>>>>>
>>>>>>>>>>> src/os_cpu/aix_ppc/vm/os_aix_ppc.cpp
>>>>>>>>>>> No comments.
>>>>>>>>>>>
>>>>>>>>>>> src/os_cpu/bsd_x86/vm/os_bsd_x86.cpp
>>>>>>>>>>> No comments.
>>>>>>>>>>>
>>>>>>>>>>> src/os_cpu/linux_aarch64/vm/os_linux_aarch64.cpp
>>>>>>>>>>> No comments.
>>>>>>>>>>>
>>>>>>>>>>> src/os_cpu/linux_ppc/vm/os_linux_ppc.cpp
>>>>>>>>>>> No comments.
>>>>>>>>>>>
>>>>>>>>>>> src/os_cpu/linux_s390/vm/os_linux_s390.cpp
>>>>>>>>>>> No comments.
>>>>>>>>>>>
>>>>>>>>>>> src/os_cpu/linux_sparc/vm/os_linux_sparc.cpp
>>>>>>>>>>> No comments.
>>>>>>>>>>>
>>>>>>>>>>> src/os_cpu/linux_x86/vm/os_linux_x86.cpp
>>>>>>>>>>> No comments.
>>>>>>>>>>>
>>>>>>>>>>> src/os_cpu/solaris_sparc/vm/os_solaris_sparc.cpp
>>>>>>>>>>> No comments.
>>>>>>>>>>>
>>>>>>>>>>> src/os_cpu/solaris_x86/vm/os_solaris_x86.cpp
>>>>>>>>>>> No comments.
>>>>>>>>>>>
>>>>>>>>>>> src/share/vm/runtime/os.hpp
>>>>>>>>>>> No comments.
>>>>>>>>>>>
>>>>>>>>>>> Dan
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>> See my comments inline.
>>>>>>>>>>>>
>>>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>>>> From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-
>>>>>>>>>>>>> bounces at openjdk.java.net] On Behalf Of Coleen Phillimore
>>>>>>>>>>>>> Sent: Dienstag, 6. Dezember 2016 01:00
>>>>>>>>>>>>> To: hotspot-runtime-dev at openjdk.java.net
>>>>>>>>>>>>> Subject: Re: RFR(M): 8170655: [posix] Fix minimum stack size
>>>>>>>>>>> computations
>>>>>>>>>>>>> On 12/5/16 6:27 PM, Daniel D. Daugherty wrote:
>>>>>>>>>>>>>> On 12/3/16 11:17 AM, Lindenmaier, Goetz wrote:
>>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I would like to fix two issues of minimum stack size
>>>>>>>>>>>>>>> computation:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~goetz/wr16/8170655-
>>>>>>>>>>>>> compilerGuardFix/webrev.01/
>>>>>>>>>>>>>>> Please review and sponsor.
>>>>>>>>>>>>>> I'm sponsoring the related bug:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 8169373 Work around linux NPTL stack guard error
>>>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8169373
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> so I guess I should sponsor this one also... For obvious
>>>>>>>>>>>>>> reasons, this
>>>>>>>>>>>>>> fix will also need a "full RBT" run...
>>>>>>>>>>>>> Thank you, Dan for sponsoring this.
>>>>>>>>>>>>>> http://cr.openjdk.java.net/~goetz/wr16/8170655-
>>>>>>>>>>>>> compilerGuardFix/webrev.01/
>>>>>>>>>>>>>> src/cpu/ppc/vm/globals_ppc.hpp
>>>>>>>>>>>>>> No comments.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> src/cpu/x86/vm/globals_x86.hpp
>>>>>>>>>>>>>> No comments.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> src/os/posix/vm/os_posix.cpp
>>>>>>>>>>>>>> My brain read right past where you took out "MAX2" and
>>>>>>>>>>>>>> changed
>>>>>>>>> the
>>>>>>>>>>>>>> math to "cur = cur + guard_size + shadow_size". I think
>>>>>>>>>>>>>> I've been
>>>>>>>>>>>>>> staring at that particular line of code for way too long
>>>>>>>>>>>>>> (in a couple
>>>>>>>>>>>>>> of different bug fixes)...
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I think with your fix that Solaris specific block on
>>>>>>>>>>>>>> L1132 - L1148 can go away (see JDK-8161093). Hopefully
>>>>>>>>>>>>>> Coleen will chime in on just this part.
>>>>>>>>>>>>> Yes, I think with refactoring, this is dead code. I think you
>>>>>>>>>>>>> should
>>>>>>>>>>>>> remove it, and make 8161093 a duplicate so it can be closed
>> with
>>>>>>>>>>>>> this fix.
>>>>>>>>>>>> I removed the code and closed the bug as duplicate.
>>>>>>>>>>>>
>>>>>>>>>>>>> The changes in this file (adding the guard_size and shadow_size)
>>>>>>>>>>>>> are a
>>>>>>>>>>>>> lot cleaner than the max of some random number of pages
>>> someone
>>>>>>>>>>>>> calculated a long time ago.
>>>>>>>>>>>> Thanks!
>>>>>>>>>>>>
>>>>>>>>>>>>> There's a comment above this function that should be
>>>>>>>>>>>>> rewritten also
>>>>>>>>>>> though:
>>>>>>>>>>>>> // Check minimum allowable stack sizes for thread creation
>>>>>>>>>>>>> and to
>>>>>>>>>>> initialize
>>>>>>>>>>>>> // the java system classes, including StackOverflowError -
>>>>>>>>>>>>> depends on
>>>>>>>>>>> page
>>>>>>>>>>>>> // size. Add two 4K pages for compiler2 recursion in main
>>>>>>>>>>>>> thread.
>>>>>>>>>>>>> // Add in 4*BytesPerWord 4K pages to account for VM stack
>>> during
>>>>>>>>>>>>> // class initialization depending on 32 or 64 bit VM.
>>>>>>>>>>>> Dan saw this too. I wrote a summary of the text in the bug.
>>>>>>>>>>>>
>>>>>>>>>>>>> http://cr.openjdk.java.net/~goetz/wr16/8170655-
>>>>>>>>>>>>>
>>> compilerGuardFix/webrev.01/src/os_cpu/linux_sparc/vm/os_linux_sparc.cpp
>>>>>>>>>>> .u
>>>>>>>>>>>>> diff.html
>>>>>>>>>>>>> This seems small. I don't think it should be reduced because I
>>>>>>>>>>>>> don't
>>>>>>>>>>>>> think we have a lot of testing for this platform.
>>>>>>>>>>>> I changed it to 64K. I can't test on this platform, so this
>>>>>>>>>>>> probably really is
>>>>>>>>>>> better.
>>>>>>>>>>>>> http://cr.openjdk.java.net/~goetz/wr16/8170655-
>>>>>>>>>>>>>
>>> compilerGuardFix/webrev.01/src/os_cpu/linux_x86/vm/os_linux_x86.cpp.ud
>>>>>>>>>>> iff.
>>>>>>>>>>>>> html
>>>>>>>>>>>>> Why is DEBUG_ONLY( + 4) removed? We do run native JVM
>> code
>>>>>> with
>>>>>>>>>>>>> debugging on in Java threads also, although the +4 seems
>>>>>>>>>>>>> arbitrary.
>>>>>>>>>>>> I checked startup and ran jvm98 and did not find any
>>>>>>>>>>>> differences in
>>>>>>>>>>> slowdebug/opt
>>>>>>>>>>>> variants. So I thought I should get rid of this. Also, this kind
>>>>>>>>>>>> of accounts for
>>>>>>>>>>> the
>>>>>>>>>>>> extra space of shadow pages in dbg builds, which now is no more
>>>>>>>>>>>> included
>>>>>>>>>>> in that
>>>>>>>>>>>> number.
>>>>>>>>>>>>
>>>>>>>>>>>>> If you're removing the concept of DEBUG_ONLY to these sizes, it
>>>>>>>>>>>>> still
>>>>>>>>>>>>> remains in this change:
>>>>>>>>>>>>> http://cr.openjdk.java.net/~goetz/wr16/8170655-
>>>>>>>>>>>>>
>>> compilerGuardFix/webrev.01/src/os_cpu/linux_s390/vm/os_linux_s390.cpp.
>>>>>>>>>>> udi
>>>>>>>>>>>>> ff.html
>>>>>>>>>>>> On s390, I quickly ran into stack overflows with the debug build.
>>>>>>>>>>>> Therefore
>>>>>>>>>>>> I increased the sizes.
>>>>>>>>>>>>
>>>>>>>>>>>>> I assume that all these minimum stack sizes have been tested in
>>>>>>>>>>>>> your lab.
>>>>>>>>>>>> Yes, the change is running with our nighly tests on the platforms
>>>>>>>>>>>> we have
>>>>>>>>>>> available.
>>>>>>>>>>>> This includes jck tests and hotspot jtreg tests.
>>>>>>>>>>>>
>>>>>>>>>>>>> This seems like a good change but I hope this is the last of
>>>>>>>>>>>>> these until
>>>>>>>>>>>>> JDK10 opens.
>>>>>>>>>>>> :) Yes, stack changes kind of got my special hobby ... I
>>>>>>>>>>>> still have
>>>>>>>>>>> implementing
>>>>>>>>>>>> StackReservedPages support on s390 on my list, but that should be
>>>>>>>>>>> platform-only.
>>>>>>>>>>>> Also I'm not sure whether I'll find time for that.
>>>>>>>>>>>>
>>>>>>>>>>>> Best regards,
>>>>>>>>>>>> Goetz.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>> Coleen
>>>>>>>>>>>>>
>>>>>>>>>>>>>> L1154: _compiler_thread_min_stack_allowed =
>>>>>>>>>>>>>> _compiler_thread_min_stack_allowed +
>>>>>>>>>>>>>> Please add a comment above this line:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> // Reminder: a compiler thread is-a Java thread.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> src/os/posix/vm/os_posix.hpp
>>>>>>>>>>>>>> L48: // Set_minimum_stack_sizes() ...
>>>>>>>>>>>>>> Please change 'S' -> 's' since it is the function's
>>>>>>>>>>>>>> name.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> src/os_cpu/aix_ppc/vm/os_aix_ppc.cpp
>>>>>>>>>>>>>> L539: // HotSpotguard pages is added later.
>>>>>>>>>>>>>> Typo: space before 'guard'
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> L542: size_t
>>>>>>>>>>>>>> os::Posix::_vm_internal_thread_min_stack_allowed = 64
>>>>>>>>>>>>>> * K;
>>>>>>>>>>>>>> VM internal thread size lowered from 128K to 64K
>>>>>>>>>>>>>> without any
>>>>>>>>>>>>>> changes to how
>> _vm_internal_thread_min_stack_allowed
>>>>>>>>>>>>>> is set.
>>>>>>>>>>>>>> Any particular reason for this change?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> src/os_cpu/bsd_x86/vm/os_bsd_x86.cpp
>>>>>>>>>>>>>> No comments.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> src/os_cpu/linux_aarch64/vm/os_linux_aarch64.cpp
>>>>>>>>>>>>>> No comments.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> src/os_cpu/linux_ppc/vm/os_linux_ppc.cpp
>>>>>>>>>>>>>> L542: size_t
>>>>>>>>>>>>>> os::Posix::_vm_internal_thread_min_stack_allowed = 64
>>>>>>>>>>>>>> * K;
>>>>>>>>>>>>>> VM internal thread size lowered from 128K to 64K
>>>>>>>>>>>>>> without any
>>>>>>>>>>>>>> changes to how
>> _vm_internal_thread_min_stack_allowed
>>>>>>>>>>>>>> is set.
>>>>>>>>>>>>>> Any particular reason for this change?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> src/os_cpu/linux_s390/vm/os_linux_s390.cpp
>>>>>>>>>>>>>> L478: size_t
>>>>>>>>>>>>>> os::Posix::_compiler_thread_min_stack_allowed = (52
>>>>>>>>>>>>>> DEBUG_ONLY(+32)) * K;
>>>>>>>>>>>>>> L479: size_t
>>>>>>>>>>>>>> os::Posix::_java_thread_min_stack_allowed = (32
>>>>>>>>>>>>>> DEBUG_ONLY(+8)) * K;
>>>>>>>>>>>>>> consistency - trying to put space around
>>>>>>>>>>>>>> operators so...
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> '+32' -> '+ 32'
>>>>>>>>>>>>>> '+8' -> '+ 8'
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> L480: size_t
>>>>>>>>>>>>>> os::Posix::_vm_internal_thread_min_stack_allowed = 32
>>>>>>>>>>>>>> * K;
>>>>>>>>>>>>>> VM internal thread size lowered from 128K to 32K
>>>>>>>>>>>>>> without any
>>>>>>>>>>>>>> changes to how
>> _vm_internal_thread_min_stack_allowed
>>>>>>>>>>>>>> is set.
>>>>>>>>>>>>>> Any particular reason for this change?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> src/os_cpu/linux_sparc/vm/os_linux_sparc.cpp
>>>>>>>>>>>>>> No comments.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> src/os_cpu/linux_x86/vm/os_linux_x86.cpp
>>>>>>>>>>>>>> No comments.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> src/os_cpu/solaris_sparc/vm/os_solaris_sparc.cpp
>>>>>>>>>>>>>> No comments.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> src/os_cpu/solaris_x86/vm/os_solaris_x86.cpp
>>>>>>>>>>>>>> No comments.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> src/share/vm/runtime/os.hpp
>>>>>>>>>>>>>> L439: java_thread, // Java, CocdeCacheSweeper,
>>>>>>>>>>>>>> JVMTIAgent and Service threads.
>>>>>>>>>>>>>> Typo: 'CocdeCacheSweeper' -> 'CodeCacheSweeper'
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Dan
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> This are the issues I excluded from the "8169373: Work
>> around
>>>>>>>>>>>>>>> linux
>>>>>>>>>>>>>>> NPTL stack guard error."
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I wrote a lengthy explanation in the bug, trying to comment
>> on
>>>>>>>>>>>>>>> what
>>>>>>>>>>> was
>>>>>>>>>>>>>>> said in the other thread. I'll repeat it here, I think that's
>>>>>>>>>>>>>>> better
>>>>>>>>>>>>>>> for discussion.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Dan, thanks for improving the text, I use the improved variant
>>>>>>>>>>>>>>> here:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> HotSpot has three cmd line options to set stack sizes (besides
>>>>>>>>>>>>>>> -Xss):
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> -XX:ThreadStackSize for threads executing Java code.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> -XX:CompilerThreadStackSize for threads used by the JIT
>>>>>>>>>>>>>>> compilers.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> -XX:VMThreadStackSize for threads executing VM internal
>>>>>>>>>>>>>>> tasks as
>>>>>>>>> gc.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> All these flags should not be set to a value that leads to a
>>>>>>>>>>>>>>> stack
>>>>>>>>>>>>>>> overflow before user code can be executed. As the VM
>> executes
>>>>>>>>>>>>>>> a lot
>>>>>>>>>>>>>>> of code for initialization and also the JIT already compiles
>>>>>>>>>>>>>>> methods,
>>>>>>>>>>>>>>> considerable amounts of stack can be used during startup. We
>>>>>>>>>>>>>>> must try
>>>>>>>>>>>>>>> to avoid stack overflows before startup is complete as error
>>>>>>>>>>>>>>> handling
>>>>>>>>>>>>>>> might not be properly in place yet.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Required minimum stack sizes depend on frame sizes and
>>> program
>>>>>>>>>>>>>>> execution paths. Frame sizes again depend on the C compiler
>>>>>>>>>>>>>>> used, the
>>>>>>>>>>>>>>> platform compiled for, and design decisions in interpreter, C1
>>>>>>>>>>>>>>> and C2.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Required stack sizes also depend on option settings, e.g. with
>>>>>>>>>>>>>>> JVM/TI
>>>>>>>>>>>>>>> enabled, frames can get bigger. With inlining increased JIT
>>>>>>>>>>>>>>> compilers
>>>>>>>>>>>>>>> might do more optimizations leading to deeper call chains,
>>>>>>>>>>>>>>> etc.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> While the minimum stack sizes should reflect differences in
>>>>>>>>>>>>>>> Platform
>>>>>>>>>>>>>>> and compiler, they must not, and cannot, cover all possible
>>>>>>>>>>>>>>> option
>>>>>>>>>>>>>>> settings.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> This change addresses two issues:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> 1. Fixed minimum stack size configuration
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Currently, the minimum Java thread size is given in a constant
>>>>>>>>>>>>>>> per
>>>>>>>>>>>>>>> os/cpu platform for each of the three stack kinds. This number
>>>>>>>>>>>>>>> includes the size required for guard pages.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> The guard pages are used for stack overflow detection. They
>>>>>>>>>>>>>>> make up 4
>>>>>>>>>>>>>>> zones on the stack: Red, Yellow, Reserved and Shadow pages.
>>>>>>>>>>>>>>> The Red,
>>>>>>>>>>>>>>> Yellow and Reserved pages are protected to detect stack
>>>>>>>>>>>>>>> overflows.
>>>>>>>>>>>>>>> The Shadow pages are just some extra space to allow methods
>>>>>>>>>>>>>>> that
>>>>>>>>>>>>>>> don't do a stack bang to execute.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Unfortunately, the size required for guard pages is not
>>>>>>>>>>>>>>> fixed at
>>>>>>>>>>>>>>> compile time. It depends on the concrete system the VM is
>>>>>>>>>>>>>>> started on.
>>>>>>>>>>>>>>> Thus the minimum sizes given can be too small to hold the
>>>>>>>>>>>>>>> guard
>>>>>>>>>>>>>>> pages. This lead to errors in the past that were solved by
>>>>>>>>>>>>>>> introducing code that overruled the per-platform minimum
>>> stack
>>>>>>>>>>>>>>> size.
>>>>>>>>>>>>>>> This code nowadays is the MAX2() in os_posix.cpp:1114 and
>> the
>>>>>>>>>>> SOLARIS
>>>>>>>>>>>>>>> special case further down. It uses the value (4 * BytesPerWord
>>>>>>>>>>>>>>> COMPILER2_PRESENT(+ 2)) * 4 * K) (os_posix.cpp:1117) as
>>>>>> minimum
>>>>>>>>>>>>>>> required space for frames. Thereby it effectively overrules
>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>> minimum stack size settings given in the os/cpu constants, and
>>>>>>>>>>>>>>> there
>>>>>>>>>>>>>>> is currently no way to specify this size per platform.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> This change proposes to fix this issue by specifying the space
>>>>>>>>>>>>>>> needed
>>>>>>>>>>>>>>> for stack frames in the os/cpu constants. During startup, this
>>>>>>>>>>>>>>> number
>>>>>>>>>>>>>>> is increased by the space required for the guard pages. Thus,
>>>>>>>>>>>>>>> this
>>>>>>>>>>>>>>> takes into account the page size of the concrete system the
>> VM
>>>>>>>>>>>>>>> runs
>>>>>>>>>>>>>>> on, and also eventual changes to the guard pages by the flags
>>>>>>>>>>>>>>> StackRed/Yellow/Reserved/Shadow/Pages. This gives the
>>>>>> opportunity
>>>>>>>>>>> to
>>>>>>>>>>>>>>> reduce the minimum stack sizes on systems with small pages.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Minimum stack size configuration is more simple with this
>>>>>>>>>>>>>>> change and
>>>>>>>>>>>>>>> valid for systems with any page size.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> 2. Stack guard pages not considered for compiler thread
>> stacks
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Compiler threads are Java threads. The C++ class
>>>>>>>>>>>>>>> CompilerThread is a
>>>>>>>>>>>>>>> subclass of JavaThread. When a compiler thread is started,
>>>>>>>>>>>>>>> JavaThread::run() is executed which protects the red,
>>>>>>>>>>>>>>> yellow and
>>>>>>>>>>>>>>> reserved pages.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Since 8140520 the minimum stack sizes for Compiler and VM
>>>>>>>>>>>>>>> internal
>>>>>>>>>>>>>>> threads no longer include the space for the guard pages.
>>>>>>>>>>>>>>> This is
>>>>>>>>>>>>>>> correct for the VM internal threads, but not for the Compiler
>>>>>>>>>>>>>>> thread.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> For the HotSpot C1 and C2 compilers it would be fine to
>>>>>>>>>>>>>>> reserve space
>>>>>>>>>>>>>>> for the Red/Yellow/Reserved pages, as they don't stack bang
>>>>>>>>>>>>>>> with the
>>>>>>>>>>>>>>> shadow page size. But since introducing JVMCI, compilers
>>>>>>>>>>>>>>> written in
>>>>>>>>>>>>>>> Java can be running on the compiler threads. Therefore the
>>>>>>>>>>>>>>> shadow
>>>>>>>>>>>>>>> pages are needed, too.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> As for the Java thread, this change uses a os/cpu constant for
>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>> required minimum space for compiler frames and then adds
>> the
>>>>>> zone
>>>>>>>>>>>>>>> sizes to the minimum stack sizes during startup.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> New sizing:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> The constants of the os/cpu minimum thread sizes are
>> reduced
>>>>>>>>>>>>>>> by the
>>>>>>>>>>>>>>> default guard page sizes and then verified by starting the
>>>>>>>>>>>>>>> VM to
>>>>>>>>>>>>>>> assure the stack still suffices to get through startup.
>>>>>>>>>>>>>>> Hotspot jtreg
>>>>>>>>>>>>>>> tests are passing. The overall sizes required (after adding
>>>>>>>>>>>>>>> guard
>>>>>>>>>>>>>>> pages) on the systems I have available get a bit smaller.
>>>>>>>>>>>>>>> In most
>>>>>>>>>>>>>>> cases the sizes even suffice to run simple programs as
>>>>>>>>>>>>>>> SpecJvm98. The
>>>>>>>>>>>>>>> table below gives the systems I tested and the required sizes
>>>>>>>>>>>>>>> reported when started with too small stacks.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> <pre>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Thread kind: Java Compiler VM
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> old new old new old new
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> bsd x86_64 dbg: 240 232 64 64 64 64
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> opt: 240 232 64 64 64 64
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> linux x86_64 dbg: 240 144 64 152 64 64
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> opt: 232 136 64 144 64 64
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> solaris sparc dbg:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> opt: 240 192 128 128 128 128
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> aix ppc64 dbg: 512 512 384 512 128 128
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> opt: 512 512 384 512 128 128
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> linux ppc64 dbg: 512 384 128 384 128 64
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> opt: 512 384 128 384 128 64
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> linux ppc64le dbg: 512 384 128 384 128 64
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> opt: 512 384 128 384 128 64
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> linux s390 dbg: 236 140 128 184 128 32
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> opt: 236 124 128 144 128 32
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> </pre>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>
More information about the hotspot-runtime-dev
mailing list