-
Notifications
You must be signed in to change notification settings - Fork 54
Avoid boxing large futures #88
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
Conversation
taiki-e
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
@taiki-e any thoughts on the CI failures? I'm not sure where the leaks are coming from. |
|
All errors come from doctest, so I think it's likely because doctest-xcompile has stabilized in 1.89 and doctests are now tested even when runner is set. (So it's not due to this PR.) All "definitely lost" indicate a leak of allocation from flume::unbounded that is captured in the closure, so this is probably due to the task being scheduled but not polled. |
Fixes #83. Fixes #66. Partially fixes #78.
This PR gets rid of the extra boxing in
spawn_uncheckedthat results in extra monomorphization and overhead of allocating and moving into aPin<Box<Fut>>. The boxing causes an extra copy onto the stack before moving onto the heap. This negates only one of the two stack copies made on lower optimization builds, and requires an extra indirection on every access, so we get rid of it.Instead, this PR turns
spawn_uncheckedinto a macro to avoid creating extra stack frames, effectively forcibly inlining the code regardless of what optimization level. This PR also opts to replace theabort_on_panicimplementation with a manual one to avoid the extra stack copy there.