We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

link to musl's definition of default stack size
http://git.musl-libc.org/cgit/musl/tree/src/internal/pthread_impl.h#n122

addendum to the above:
the problem with glibc's huge stack size is not only the resource waste going on, it's that everyone using it (probably more than 99% of linux users) gets the wrong impression that pthread_create works perfectly well for everything without having to care about the stack size, which is of course wrong since we're relying on POSIX semantics when dealing with pthreads, not on GLIBC implementation details.

this can be important.

Maybe the best option (non-breaking) would be to add a new function, SDL_CreateThreadEx, that explicitly sets the stack size (a stack size of -1 could mean "use implementation default", and systems that only have a fixed stack for a new thread could simply fail unless -1 is passed)

Would a hint work? SDL_THREAD_STACK_SIZE or whatever.

A hint is probably the right approach here.

Mmm, except that's not something we want affected by users setting environment variables...

If the hint is set with SDL_HINT_OVERRIDE it can't be affected by environment variables.

We can detect if glibc is *not* being linked in, and in that case set a hint by default with SDL_HINT_OVERRIDE priority indicating a stack of 128K, 256K or whatever seems best, thus preventing users from messing with the setting with environment variables.

Then, on creation of the thread, we obey that hint if we are not linking against glibc (I suggest that to preserve the previous behavior, at least until 2.1)

How does that sound?

That sounds great. :)

It's worth noting that glibc doesn't actually devour 8 megabytes for each thread you spin; the stack memory is committed page by page as you touch it. So unless you have a deep recursion or a massive amount of stack data (both of which benefit from having 8 megabytes available without drama), you never actually own this memory.

A 32-bit process that spins thousands of threads at once could conceivably run out of address space without actually committing any of that memory to the process, but on 64-bit systems, this (should) never be a problem.

I do think being able to specify stack size is a good idea, but I also don't think this change is urgent, if the primary concern is what glibc does.

--ryan.

I think the concern is about what musl libc does and how to specify an appropriate stack size in that instance.

I don't actually know, does musl libc have the same behavior as glibc (e.g. not committing the pages until they're needed?)

(In reply to Sam Lantinga from comment #9)
> I think the concern is about what musl libc does and how to specify an
> appropriate stack size in that instance.

Whoops, I should have read more carefully.

> I don't actually know, does musl libc have the same behavior as glibc (e.g.
> not committing the pages until they're needed?)

It uses mmap() to allocate the new thread's stack...

http://git.musl-libc.org/cgit/musl/tree/src/thread/pthread_create.c?id=fdf5f1b13123883ac1d5e298e5f32c7ed43578ce#n151

...so it would work like glibc: reserve the memory in the process's address space, but not actually commit the pages until you touch them.

--ryan.

Created attachment 1557 [details]
Implements SDL_HINT_THREAD_STACK_SIZE for pthread

Sam or Ryan, when you get a chance please review this patch. Thanks!

The "default" value should be 0, not -1. Can you take out the environment override stuff for now? I want to make a more general solution for hints that should only be programmatically set.

(In reply to Sam Lantinga from comment #12)
> The "default" value should be 0, not -1. Can you take out the environment
> override stuff for now? I want to make a more general solution for hints
> that should only be programmatically set.

A better question is should this be a hint at all, or should we just add SDL_SetThreadStackSize() or something for 2.0.2?

(I don't really want to add an SDL_CreateThreadEx(), but maybe we should do that anyhow.)

--ryan.

We could use the GL model and have a generic SDL_THREAD_SetConfig, that's easier to expand later on

I'm fine with adding an explicit API for 2.0.3. We should definitely not add a generic config API as this is the second thread creation parameter we've wanted in 10 years. :)

Ok, let's hold off until after 2.0.2 is released and use SDL_SetThreadStackSize

(In reply to Ryan C. Gordon from comment #8)
> It's worth noting that glibc doesn't actually devour 8 megabytes for each
> thread you spin; the stack memory is committed page by page as you touch it.
> So unless you have a deep recursion or a massive amount of stack data (both
> of which benefit from having 8 megabytes available without drama), you never
> actually own this memory.
> 
> A 32-bit process that spins thousands of threads at once could conceivably
> run out of address space without actually committing any of that memory to
> the process, but on 64-bit systems, this (should) never be a problem.
> 
> I do think being able to specify stack size is a good idea, but I also don't
> think this change is urgent, if the primary concern is what glibc does.
> 
> --ryan.

what you said is only true when overcommit is enabled - which is not necessarily a good idea - see http://ewontfix.com/3/ .
additionally those 8MB will always reserve virtual address space, so you can use at most ~400 threads on a 32bit glibc system.

Indeed, Ryan's use of the word "commit" is a bit unusual, but the behavior he describes is roughly correct when overcommit is enabled. With overcommit (the Linux default), the 8MB glibc allocates for a thread stack is "committed"; it just doesn't get accounted against a commit limit that would prevent the kernel from committing memory to other processes. Whether or not overcommit is enabled, the pages will not be present in physical memory prior to usage (they'll be COW references to the zero page) and thus will not preclude use of the memory for cache, etc. In any case, they consume (and possibly contribute to the fragmentation of) virtual address space, a scarce resource for 32-bit processes.

For musl we consider both no-overcommit systems and systems with resource limit policies (setrlimit or cgroup/container based) as major supported targets; using a large default thread stack size would undermine resource accounting by requiring vastly inflated resource limits that would be undesirably high if the application actually used all the memory it was granted.

I was just going to make an identical suggestion (i.e. using a HINT) since that would be less intrusive than an API change.

Meanwhile, is there a reason why the hint or the addition of SDL_SetThreadStackSize hasn't been adopted?

Thanks?

- Mark

(In reply to Mark Pizzolato from comment #19)
> Meanwhile, is there a reason why the hint or the addition of
> SDL_SetThreadStackSize hasn't been adopted?

We hadn't gotten around to it yet; I'm looking at this for 2.0.4, but it might slip to 2.0.5.

(and in the distant future, when 2.1.0 breaks ABI again, we'll just make this part of SDL_CreateThread())

--ryan.

Marking a large number of bugs with the "triage-2.0.4" keyword at once. Sorry if you got a lot of email from this. This is to help me sort through some bugs in regards to a 2.0.4 release. We may or may not fix this bug for 2.0.4, though!

(sorry if you get a lot of copies of this email, I'm marking several bugs at once)

Marking bugs for the (mostly) final 2.0.4 TODO list. This means we're hoping to resolve this bug before 2.0.4 ships if possible. In a perfect world, the open bug count with the target-2.0.4 keyword is zero when we ship.

(Note that closing a bug report as WONTFIX, INVALID or WORKSFORME might still happen.)

--ryan.

Gabriel's patch is now https://hg.libsdl.org/SDL/rev/e6bf740c92f8, and the changes Sam requested are now https://hg.libsdl.org/SDL/rev/d7762e30ba24.

Resolving bug. We'll explore this again with a proper API in SDL 2.1.

--ryan.