Skip to content

feat: reduce stack-size of threads #774

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

thomaseizinger
Copy link
Contributor

Certain environments like iOS are very resource-constrained. A network extension on iOS is allowed to at most consume 50MB of RAM. By default, a thread in Rust allocates 2MB of RAM for the stack of the new thread. This can be tweaked if one knows that the particular thread doesn't need as much RAM.

@thomaseizinger
Copy link
Contributor Author

I am opening this as a starting point for a discussion. The implementation is purposely very minimal for now. I am happy to extend with comments, consts etc.

I wanted to discuss first if this is something that you'd be willing to merge.

@thomaseizinger
Copy link
Contributor Author

thomaseizinger commented Apr 22, 2025

I checked all of these threads on a high-level and none of them looked like they would consume a lot of stack-size. So I set it to 100KB for all of them which should be plenty for what we are doing in these threads.

Edit: Ha, it looks like sentry-transport needs more than that.

@thomaseizinger thomaseizinger force-pushed the feat/reduce-stack-size branch from 26587f5 to 28ee61c Compare April 22, 2025 09:32
Copy link
Member

@Swatinem Swatinem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is reasonable. As you commented, running a full tokio runtime for usage with reqwest is still quite heavy on stack usage though.

@thomaseizinger
Copy link
Contributor Author

I think this is reasonable. As you commented, running a full tokio runtime for usage with reqwest is still quite heavy on stack usage though.

500KB seems to allow CI to pass. I can experiment a bit more to see if we can get it any lower although it is already a great improvement :)

I am not too familiar with the test setup in this repo. Are they exhaustive enough that we hit all scenarios of stack usage and can therefore trust that green CI means we have enough stack for all situations?

@Swatinem
Copy link
Member

TBH, I wouldn’t put too much trust in them. In particular, I’m not even sure whether we are even testing the http interface at all, or just relying on a mock transport for tests mostly.

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

Successfully merging this pull request may close these issues.

2 participants