RFR(M): 8170655: [posix] Fix minimum stack size computations
Lindenmaier, Goetz
goetz.lindenmaier at sap.com
Tue Dec 13 16:22:59 UTC 2016
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