(RFR)(S)(10): 8176768: hotspot ignores PTHREAD_STACK_MIN when creating new threads

[David Holmes]

Hi Chris,

On 16/03/2017 3:03 PM, Chris Plummer wrote:
> Hello,
>
> Please review the following:
>
> https://bugs.openjdk.java.net/browse/JDK-8176768
> http://cr.openjdk.java.net/~cjplummer/8176768/webrev.00/webrev.hotspot

Change looks good.

> While working on 8175342 I noticed our stack size on xgene was 8mb even
> though I was specifying -Xss72k. It turns out the following code was
> failing:
>
>       pthread_attr_setstacksize(&attr, stack_size);

So these really should be checking return values, at least in debug 
builds. But we can leave that until we refactor the thread startup code 
into os_posix.cpp.

Thanks,
David
-----

> Although we computed a minimum stack size of 72k, so -Xss72k should be
> fine, pthreads on this platform requires the stack be at least 128k, so
> it failed the pthread_attr_setstacksize() call. The end result is
> pthread_attr_setstacksize() had no impact on the thread's stack size,
> and we ended up with the platform default of 8mb. The fix is to round up
> the following variables to PTHREAD_STACK_MIN after computing their new
> values:
>
>       _java_thread_min_stack_allowed
>       _compiler_thread_min_stack_allowed
>       _vm_internal_thread_min_stack_allowed
>
> For solaris, there was an issue using PTHREAD_STACK_MIN. You need to
> #define _POSIX_C_SOURCE >= 199506L in order to get PTHREAD_STACK_MIN
> #defined, and this needs to be done before including OS header files. I
> noticed that on solaris we were using thr_min_stack() elsewhere instead
> of PTHREAD_STACK_MIN, so I decided to do the same with this fix. Either
> way is ugly (the #define or using thr_min_stack()).
>
> And speaking of the existing use of thr_min_stack(), I deleted it. It
> was being applied before any adjustments to the stack sizes had been
> made (rounding and adding red, yellow, and shadow zones). This mean the
> stack ended up being larger than necessary. With the above fix in place,
> we are now applying thr_min_stack() after recomputing the minimum stack
> sizes. If for any reason one of those stack sizes is now too small, the
> correct fix is to adjust the initial stack sizes, not apply
> thr_min_stack() to the initial stack sizes. However, it looks like no
> adjustment is needed. I did something close to our nightly testing on
> all affect platforms, and no new problems turned up.
>
> thanks,
>
> Chris