Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SDL_Thread lacks control over stack size #1007

Closed
SDLBugzilla opened this issue Feb 10, 2021 · 0 comments
Closed

SDL_Thread lacks control over stack size #1007

SDLBugzilla opened this issue Feb 10, 2021 · 0 comments

Comments

@SDLBugzilla
Copy link
Collaborator

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: don't know
Reported for operating system, platform: Linux, x86_64

Comments on the original bug report:

On 2013-08-07 04:07:28 +0000, wrote:

Created attachment 1274
sdl default thread stacksize 128KB patch

on linux and probably all other unices, SDL_Thread is implemented via usage of pthreads.

POSIX leaves it entirely up to the implementation to decide about the stack size to use for the new thread, unless the caller sets it explicitly by using pthread_attr_setstacksize().

it's an unfortunate coincidence that glibc's default stack size is insanely huge value of about 8 MB, which is sufficient for almost anything, but very resource wasting by default.

other implementations, for example musl libc, use a by far smaller default stack size. in musl, this default is 80KB.

this means that some games that use SDL_Thread crash on musl because they need a bigger stack. one example is Dwarf Fortress, which works with a setting of 96KB.

there are 2 possible fixes:

  1. change the SDL API in a way to offer the SDL_Thread user the possibility to set the required stacksize himself

  2. explicitly set the required stack size when calling SDL_CreateThread().

of course 1) gives much more flexibility. however it would need some API changes since currently a SDL_Thread is only returned after the creation of the thread, so you everything's already done and you cannot change the thread anymore.

as for 2) the hard part is to find a size that suits everybody. historically, there has been an alternative implementation for linux using clone (
http://svn.libsdl.org/tags/SDL/release-1.2.4/src/thread/linux/SDL_systhread.c ) which used a default stacksize of 64KB via a buffer obtained by malloc() and directly used with the clone() syscall.
64 KB is too little nowadays; but so far i've been able to use a couple of games with the default 80KB, and only Dwarf Fortess 34.11 required more than that.
so my preference would be to limit the thread stack to 128K.

attached is an example patch for 2).

On 2013-08-07 04:21:10 +0000, wrote:

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.

On 2013-08-27 05:56:40 +0000, Nathaniel J Fries wrote:

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)

On 2014-01-21 14:47:05 +0000, Gabriel Jacobo wrote:

Would a hint work? SDL_THREAD_STACK_SIZE or whatever.

On 2014-02-02 09:00:47 +0000, Sam Lantinga wrote:

A hint is probably the right approach here.

On 2014-02-02 09:01:51 +0000, Sam Lantinga wrote:

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

On 2014-02-04 03:37:48 +0000, Gabriel Jacobo wrote:

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?

On 2014-02-09 10:47:58 +0000, Sam Lantinga wrote:

That sounds great. :)

On 2014-02-09 19:17:47 +0000, Ryan C. Gordon wrote:

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.

On 2014-02-09 20:37:20 +0000, Sam Lantinga wrote:

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

On 2014-02-10 19:25:34 +0000, Ryan C. Gordon wrote:

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

On 2014-02-12 22:01:46 +0000, Gabriel Jacobo wrote:

Created attachment 1557
Implements SDL_HINT_THREAD_STACK_SIZE for pthread

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

On 2014-02-23 01:44:29 +0000, Sam Lantinga wrote:

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.

On 2014-02-23 02:32:49 +0000, Ryan C. Gordon wrote:

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

On 2014-02-23 02:40:57 +0000, Gabriel Jacobo wrote:

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

On 2014-02-23 02:42:30 +0000, Sam Lantinga wrote:

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. :)

On 2014-02-23 13:49:24 +0000, Gabriel Jacobo wrote:

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

On 2014-02-26 17:35:35 +0000, wrote:

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

On 2014-02-26 18:33:58 +0000, Rich Felker wrote:

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.

On 2015-02-04 17:55:21 +0000, Mark Pizzolato wrote:

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

On 2015-02-18 20:12:23 +0000, Ryan C. Gordon wrote:

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

On 2015-02-19 05:22:17 +0000, Ryan C. Gordon wrote:

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!

On 2015-04-07 04:57:54 +0000, Ryan C. Gordon wrote:

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

On 2015-05-27 01:20:31 +0000, Ryan C. Gordon wrote:

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant