RFR: 8225035: Thread stack size issue caused by large TLS size
Thomas Stüfe
thomas.stuefe at gmail.com
Wed Jun 12 20:45:51 UTC 2019
Hi Jiangli,
after learning a lot of interesting things today about TLS :-) I took a
look at your change.
First another question, just to be sure I understand this: From what think
I understood about TLS, and from the way you wrote your test (writing an
own launcher and all) I assume the problem only happens for TLS of modules
which are loaded at program start, right? So Libjvm.so and dynamically
loaded JNI libs can allocate as many __thread space as they like, they
would not go on the thread stacks? And this is only a problem when you
embed the libjvm into other launchers which use a lot of TLS?
---
I do not understand why the complicated dynamic lookup of
__pthread_get_minstack. What is the advantage of this two-level approach vs
just dlsym'ing the pointer like we do for all other dynamically loaded
functions? Also, would this not cause problems when building for other libc
implementations, e.g. muslc?
And why RTLD_NEXT, why not RTLD_DEFAULT? Any special reason? Since that
seems to be how we normally load dynamic symbols from the libc.
About the fkt pointer, in contrast to David I would prefer to see it is a
pointer, not a function, so your first name which I assume was "p_.." I
would have liked more.
---
+ size_t min_stack_size = align_up(pthread_get_minstack(&attr),
vm_page_size());
Why the align up? The only condition we have to meet is for the total stack
size to be page aligned. On some platforms page size can be large (I think
we have 64K pages on ppc) and TLS size could be smaller than that. Could
you move the align up do right before the call to
pthread_attr_setstacksize() (and probably remove the assert there)?
---
What is the logic behind giving attr to pthread_get_minstack? If Florian is
right and this:
http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2019-May/034593.html
is the implementation, that parameter seems not to be used. In that case,
just to be sure, I would feed it a dummy attr.
---
I am not sure, but is adding the TLS sized buffer to just small thread
stacks the right thing to do? Why it is okay for thread stacks slightly
larger than the cut-off point to live without this additional space? Will
they not as well be hurt by the large TLS subtraction from their stack
sizes?
Thank you!
Cheers, Thomas
On Tue, Jun 11, 2019 at 12:30 AM Jiangli Zhou <jianglizhou at google.com>
wrote:
> Hi David and Florian,
>
> This the a follow up of the discussion regarding the TLS (Thread Local
> Storage) issue:
>
> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2019-May/034459.html
> .
> The following is an updated patch with your suggestions incorporated:
>
> 1) Use __pthread_get_minstack
> 2) By default only adjust the thread stack size if the
> minstack-to-stacksize ratio is >= 10%
> 3) Introduce a new command-line option (Linux only),
> AdjustWithMinStackAt, which can be used to change the default 10%
> ratio for triggering stack size adjustment
>
> CSR: https://bugs.openjdk.java.net/browse/JDK-8225498
> webrev: http://cr.openjdk.java.net/~jiangli/8225035/webrev.00/
> bug: https://bugs.openjdk.java.net/browse/JDK-8225035
>
> Best regards,
>
> Jiangli
>
More information about the hotspot-runtime-dev
mailing list