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

[David Holmes]

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.

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
>>>