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

Add story of Alan running in trouble with large stack allocations #38

Merged
merged 3 commits into from
Mar 16, 2021

Conversation

rylev
Copy link
Member

@rylev rylev commented Mar 16, 2021

@nikomatsakis This is the story I mentioned to you about large stack allocations caused by pinning large futures to the stack.

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

This is great @rylev!

@nikomatsakis nikomatsakis merged commit 809f8f0 into rust-lang:master Mar 16, 2021
@rylev rylev deleted the alan-stack-trouble branch March 17, 2021 09:11
* Barbara, as an expert in Rust, may have had the tools to understand that `pin_mut` is used for pinning to the stack while `Box::pin` is for pinning heap allocations.
* This problem is somewhat subtle, so someone like Niklaus would probably have had a much harder time figuring this out (or even getting the code to compile in the first place).
* **Could Alan have used another API to achieve the same objectives?**
* Perhaps! Tokio's `select!` macro doesn't require explicit pinning of the futures it's provided, but it's unclear to this author whether it would have been smart enough to avoid pinning large futures to the stack. However, pinning is a part of the way one uses futures in Rust, so it's possible that such an issue would have arisen elsewhere.

Choose a reason for hiding this comment

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

It just pins things on the "async stack" automatically for you. The futures-rs version however does that too after I implemented rust-lang/futures-rs#1873 . The tokio version mainly goes a little bit further and avoids the FusedFuture requirements based off the ideas in rust-lang/futures-rs#1989.

I don't any version of the select! macro would resolve the issue on its own - one will always need an explicit Box to decouple the Futures. Only something like FuturesUnordered or other sub-executors will do it automatically.

Still surprised that the whole thing took 4Gb and could be fixed by improving one select. While it's known that generators can grow exponentially in size it seems to take a while to get to 4Gb 😯

@nikomatsakis nikomatsakis added the status-quo-story-ideas "Status quo" user story ideas label Apr 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status-quo-story-ideas "Status quo" user story ideas
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants