[9] RFR(S): 8172731: runtime/Thread/TooSmallStackSize.java fails on solaris-x64 with product build

Lindenmaier, Goetz goetz.lindenmaier at sap.com
Tue Jan 17 06:17:57 UTC 2017 

That's good, thanks a lot!

Best regards,
  Goetz

> -----Original Message-----
> From: Tobias Hartmann [mailto:tobias.hartmann at oracle.com]
> Sent: Montag, 16. Januar 2017 16:26
> To: David Holmes <david.holmes at oracle.com>; Lindenmaier, Goetz
> <goetz.lindenmaier at sap.com>; daniel.daugherty at oracle.com; hotspot-
> dev at openjdk.java.net Developers <hotspot-dev at openjdk.java.net>
> Subject: Re: [9] RFR(S): 8172731: runtime/Thread/TooSmallStackSize.java fails
> on solaris-x64 with product build
> 
> Hi,
> 
> Goetz suggested (off-thread) to add a comment to the patch stating the
> reason for the large stack size:
> http://cr.openjdk.java.net/~thartmann/8172731/webrev.01/
> 
> Are you fine with that?
> 
> Thanks,
> Tobias
> 
> On 16.01.2017 13:38, Tobias Hartmann wrote:
> > Hi David,
> >
> > On 16.01.2017 13:33, David Holmes wrote:
> >> Sorry I missed answering the "should we file a studio compiler bug"
> question. Don't we generate the MachNodeGenerator code using adlc? What
> does the thing we generate actually look like?
> >
> > Yes, the C(++) code is generated using adlc. It looks like this:
> >
> > //------------------------- MachNode Generator ---------------
> > // A switch statement on the dense-packed user-defined type system
> > // that invokes 'new' on the corresponding class constructor.
> >
> > MachNode *State::MachNodeGenerator(int opcode){
> >   switch(opcode) {
> >   case loadB_rule: {
> >       loadBNode *node = new loadBNode();
> >       return node;
> >     }
> >   case loadB2L_rule: {
> >       loadB2LNode *node = new loadB2LNode();
> >       return node;
> >     }
> >   case loadUB_rule: {
> >       loadUBNode *node = new loadUBNode();
> >       return node;
> >     }
> >   case loadUB2L_rule: {
> >
> > [...]
> >
> >>
> >> David
> >>
> >> On 16/01/2017 10:21 PM, David Holmes wrote:
> >>> On 16/01/2017 10:15 PM, Tobias Hartmann wrote:
> >>>> Hi,
> >>>>
> >>>> thanks to everyone for looking at this.
> >>>>
> >>>>> My concern with this series of changes is that we're claiming to be
> >>>>> setting shared posix values when it seems in fact the requirements
> >>>>> are different across "posix" platforms. We now penalize all posix
> >>>>> platforms with a larger minimum stack due to this Solaris x64 issue.
> >>>>
> >>>> Why do we penalize all platforms with this fix? The changes are to
> >>>> 'os_solaris_x86.cpp' and only affect a Solaris x86 build.
> >>>
> >>> Sorry I was mis-remembering that despite being called
> >>> os::Posix::_compiler_thread_min_stack_allowed it is actually a platform
> >>> specific value.
> >
> > Okay, no problem.
> >
> > Thanks,
> > Tobias
> >
> >>>
> >>> David
> >>>
> >>>>> I'm also very curious as to why Solaris x64 needs to be different?
> >>>>
> >>>> [see below]
> >>>>
> >>>>>> Anyways, this size looks strange. Other platforms get along with 32K
> >>>>>> and 48K
> >>>>>> compiler thread size!  Why does this platform need ten times more
> >>>>>> than others?
> >>>>>> Is it because C2 optimizations are configured more aggressive,
> >>>>>> leading to bigger
> >>>>>> compile tasks during startup?
> >>>>>> But solaris_sparc also needs a lot (102K) as well as aix (192K). At
> >>>>>> some point
> >>>>>> I'll have a look at why aix needs that much.
> >>>>
> >>>> I don't think it's due to C2 optimizations. They shouldn't differ much
> >>>> on Solaris x86 compared to Linux x86.
> >>>>
> >>>> I had a closer look at the failures and we always fail in the C2
> >>>> matcher in the generated method State::MachNodeGenerator(int)
> which
> >>>> basically consists of a *huge* switch statement matching opcodes.
> >>>> Turns out that with a product build we reserve lots of stack space on
> >>>> method entry:
> >>>>
> >>>> 0xffff80ff92537cf0<MachNode*State::MachNodeGenerator(int)>:
> push
> >>>> %rbp
> >>>> 0xffff80ff92537cf1<MachNode*State::MachNodeGenerator(int)+1>:
> mov
> >>>> %rsp,%rbp
> >>>> 0xffff80ff92537cf4<MachNode*State::MachNodeGenerator(int)+4>:
> push
> >>>> %r15
> >>>> 0xffff80ff92537cf6<MachNode*State::MachNodeGenerator(int)+6>:
> sub
> >>>> $0x53ba8,%rsp
> >>>>
> >>>> This requires ~335k of stack space whereas a fastdebug build only
> >>>> requires ~15k for the same method:
> >>>>
> >>>> 0xffff80ff7c8a0460<MachNode*State::MachNodeGenerator(int)>:
> push
> >>>> %rbp
> >>>> 0xffff80ff7c8a0461<MachNode*State::MachNodeGenerator(int)+1>:
> mov
> >>>> %rsp,%rbp
> >>>> 0xffff80ff7c8a0464<MachNode*State::MachNodeGenerator(int)+4>:
> sub
> >>>> $0x9430,%rsp
> >>>>
> >>>> This is consistent with my findings that a fastdebug VM requires a
> >>>> CompilerThreadStackSize of 155k whereas a product build requires
> 384k.
> >>>>
> >>>> I guess that this difference is due to more aggressive optimizations
> >>>> done by the Sun Studio Compiler for product builds but from looking at
> >>>> the assembly code, it's not obvious what all that space is actually
> >>>> used for.
> >>>>
> >>>> Do you think we should file a Sun Studio Compiler bug for this?
> >>>>
> >>>>>> Basically this is the right fix for the immediate problem, though.
> >>>>>> Looks good.
> >>>>
> >>>> Thanks, if there are no objections I would like to push this fix.
> >>>>
> >>>> Thanks,
> >>>> Tobias
> >>>>
> >>>>>>
> >>>>>> Best regards,
> >>>>>>   Goetz
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>> -----Original Message-----
> >>>>>>> From: Daniel D. Daugherty [mailto:daniel.daugherty at oracle.com]
> >>>>>>> Sent: Samstag, 14. Januar 2017 01:04
> >>>>>>> To: Tobias Hartmann <tobias.hartmann at oracle.com>; hotspot-
> >>>>>>> dev at openjdk.java.net Developers <hotspot-
> dev at openjdk.java.net>;
> >>>>>>> Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
> >>>>>>> Subject: Re: [9] RFR(S): 8172731:
> >>>>>>> runtime/Thread/TooSmallStackSize.java fails
> >>>>>>> on solaris-x64 with product build
> >>>>>>>
> >>>>>>> Adding Goetz to this email thread directly.
> >>>>>>>
> >>>>>>>
> >>>>>>> On 1/13/17 10:37 AM, Tobias Hartmann wrote:
> >>>>>>>> Hi,
> >>>>>>>>
> >>>>>>>> please review the following patch:
> >>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8172731
> >>>>>>>> http://cr.openjdk.java.net/~thartmann/8172731/webrev.00/
> >>>>>>>
> >>>>>>> src/os_cpu/solaris_x86/vm/os_solaris_x86.cpp
> >>>>>>>      No comments.
> >>>>>>>
> >>>>>>> Thumbs up! Thanks for fixing this issue.
> >>>>>>>
> >>>>>>> Goetz, can you please chime in on this thread since you were the
> >>>>>>> author for:
> >>>>>>>
> >>>>>>>      JDK-8170655: [posix] Fix minimum stack size computations
> >>>>>>>      https://bugs.openjdk.java.net/browse/JDK-8170655
> >>>>>>>
> >>>>>>> My in-house testing as sponsor didn't catch this issue and now
> >>>>>>> I look more closely at your testing comment in the bug report,
> >>>>>>> I don't see Solaris X64 listed.
> >>>>>>>
> >>>>>>> Dan
> >>>>>>>
> >>>>>>>
> >>>>>>>>
> >>>>>>>> The test fails because the VM crashes with a
> >>>>>>>> CompilerThreadStackSize of
> >>>>>>> 300k. The problem is caused by the fix for JDK-8170655 [1] which
> >>>>>>> now allows
> >>>>>>> a minimum CompilerThreadStackSize of 300k, before it was 396k.
> 300k
> >>>>>>> is not
> >>>>>>> enough to start the VM on Solaris x86. This didn't show up when
> the
> >>>>>>> fix was
> >>>>>>> integrated because it was only tested with debug builds (with a
> >>>>>>> debug build
> >>>>>>> the number of StackShadowPages is increased by 2 and therefore
> the
> >>>>>>> minimal CompilerThreadStackSize is larger).
> >>>>>>>>
> >>>>>>>> Setting the guard pages to the minimum size via "-
> >>>>>>> XX:StackShadowPages=10 -XX:StackRedPages=1 -
> XX:StackYellowPages=2"
> >>>>>>> requires a CompilerThreadStackSize of at least 381k to not crash at
> >>>>>>> startup
> >>>>>>> with a product build but the VM only checks for >= 260k. I
> >>>>>>> increased the
> >>>>>>> value of os::Posix::_compiler_thread_min_stack_allowed by 123 to
> >>>>>>> 325 such
> >>>>>>> that the minimum allowed CompilerThreadStackSize is 384k (due to
> the
> >>>>>>> additional space allocated for guard pages and rounding to the
> page
> >>>>>>> size).
> >>>>>>>>
> >>>>>>>> Tested with failing test, manually and with RBT (running).
> >>>>>>>>
> >>>>>>>> Thanks,
> >>>>>>>> Tobias
> >>>>>>>>
> >>>>>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8170655
> >>>>>>

More information about the hotspot-dev mailing list