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

join! doubles the size of the joined futures #1473

Closed
Matthias247 opened this issue Mar 11, 2019 · 6 comments
Closed

join! doubles the size of the joined futures #1473

Matthias247 opened this issue Mar 11, 2019 · 6 comments

Comments

@Matthias247
Copy link
Contributor

I spent some time in investigating sizes of async functions, and discovered that the usage of join! lead to a doubling of the required size for a future.

After looking into the associated code, I think the move/copy of the joined futures is the issue:

$(
    // Move future into a local so that it is pinned in one place and
    // is no longer accessible by the end user.
    let mut $fut = $crate::future::maybe_done($fut);
)*

While this seems harmless, these operations add up, and every composition layer can double the future size. And moving futures in the 10kB+ range is costly and wasting memory.

Maybe there are other ways to do this too. E.g. requiring already pinned futures and polling these in-place? With an extra-layer that stack-pins futures that are not yet pinned?

There are probably also a few other combinators which inhibit the same behavior.

@Matthias247
Copy link
Contributor Author

@cramertj, @tmandry: Do you think this will get fixed with the generator optimizations in rust-lang/rust#52924? Or are other optimizations needed for this?

Futures uses the "move future into other place" to make it inaccessible or just in order to compose things quite often, and in the current state the sizes suffer quite a bit from that.

I just replaced a join!(a, b, c) with await!(a); await!(b); await!(c); and saved 30kB. Since that piece of code is intended for embedded software that's quite huge.

@tmandry
Copy link
Member

tmandry commented May 5, 2019

@Matthias247 await! does the same moving you're talking about:

https://github.com/rust-lang/rust/blob/d628c2e642c6f8f85f24dd5d7f49de89b95bf682/src/libstd/macros.rs#L365-L368

awaiting three futures in order is always going to be smaller, because you only have to save the state of one future at a time.

The optimization in rust-lang/rust#60187 is designed to shrink async fns with multiple await!s, so that will hopefully shrink the underlying futures.

@seanmonstar
Copy link
Contributor

The join macro also puts each future in a MaybeDone enum, which may be affecting its size growth.

@cramertj
Copy link
Member

cramertj commented May 6, 2019

An unconditional move out of the original non-Copy future should be able to be trivially optimized away by NRVO that works any good at all. However, we don't have NRVO that works any good at all today, so I'd be happy to accept any hack you can come up with that prevents these extra locals from being created.

@Matthias247
Copy link
Contributor Author

I experimented a bit around this with a custom join implementation, where I could debug the size of the returned Future. And it seems like join! might actually not be the problem here, but still the generator compilation inside the compiler.

Which somehow makes sense. Even if we move the Futures around inside join, that move and the creation of the compound Future only requires "normal" stack, not persisted stack on the generator.

@cramertj
Copy link
Member

I'm going to close this as I don't think there's anything more we can do in the futures-rs implementation, and size issues are already being tracked upstream.

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

4 participants