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

async block occupies twice the size of its captures (causing explosion in size). #108906

Open
nolanderc opened this issue Mar 8, 2023 · 6 comments
Labels
C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-async Working group: Async & await

Comments

@nolanderc
Copy link
Contributor

I tried this code (playground link) which wraps a future in a trivial async block:

let foo = async move { ... };
dbg!(std::mem::size_of_val(&foo));

let bar = async move { foo.await };
dbg!(std::mem::size_of_val(&bar));

I expected to see that size_of(bar) == size_of(foo) (plus some bytes for a tag). Instead, I found that size_of(bar) > 2 * size_of(foo).

I discovered this while nesting a couple of futures which largely had the following form:

let future = function_returning_future(...);
async move { 
    let result = future.await;
    let output = /* ...do something with result... */;
    output
}

The final future occupied a few kilobytes. I was able work around this by using a map combinator, but it is not ideal.

This could possibly be related to #62958, but I'm not familiar enough with the compiler internals to say with certainty if async-block captures are implemented the same way as async fn arguments.

Meta

This happens in stable 1.67.1, beta 1.70.0, and the latest nightly.

@nolanderc nolanderc added the C-bug Category: This is a bug. label Mar 8, 2023
@csmoe
Copy link
Member

csmoe commented Mar 8, 2023

Duplicate of #59087

@pnkfelix
Copy link
Member

pnkfelix commented Mar 8, 2023

Both #59087 and #62958 are relevant here. Issue #62958 talks about async fn, and part of the problems it outlines are indeed due to how async fn is desugared, but the exponential blowup you describe here is also inherent to how even async blocks and .await are desugared.

@nolanderc , do you have the capability to try out locally built branches? If so, could you give PR #108590 a whirl and see if that resolves the problematic behavior you are observing?

(We still need to resolve whether the strategy employed by #108590 is sound, given the misgivings posted on #59087 and elsewhere. But it seems to me that we will have a very fundamental problem if we cannot perform the optimization that is implemented in PR #108590.)

@pnkfelix pnkfelix added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-async Working group: Async & await labels Mar 8, 2023
@nolanderc
Copy link
Contributor Author

@pnkfelix I tried using PR #108590 by following the rustc build instructions but still see the same behaviour: the size of the futures still double for each level of nesting. Although, this is my first time building rustc from source, so I might have messed something up. Particularly, I was confused by the difference between stage1 and stage2, and which I was supposed to use, but I tried both and they gave the same results.

@pnkfelix
Copy link
Member

@nolanderc there are still scenarios where the exponential blowup is expected. PR #108590 is only a band-aid and doesn't cover all the known cases where it can occur. (The code snippets that you posted seemed like ones that should be covered by PR #108590, but its possible I'm misunderstanding something about them.)

If you could post a concrete self-contained example, in a form similar to that shown in #62958, then that will help me figure out 1. whether PR #108590 should be able to deal with your situation, and 2. if it cannot, then what extensions of PR #108590 would be necessary to deal with your situation.

@nolanderc
Copy link
Contributor Author

nolanderc commented Mar 14, 2023

@pnkfelix There already is a self-contained example (see the playground link in the first post). I have reduced it further here:

fn main() {
    let foo = async { 123 };
    let foo_size = std::mem::size_of_val(&foo);

    let bar0 = async move { foo.await };
    let bar0_size = std::mem::size_of_val(&bar0);

    let bar1 = async move { bar0.await };
    let bar1_size = std::mem::size_of_val(&bar1);

    let bar2 = async move { bar1.await };
    let bar2_size = std::mem::size_of_val(&bar2);

    let bar3 = async move { bar2.await };
    let bar3_size = std::mem::size_of_val(&bar3);

    dbg!(foo_size, bar0_size, bar1_size, bar2_size, bar3_size);

    // These are the sizes I would expect assuming 1-byte state tag:
    assert_eq!(foo_size,  1); // ok
    assert_eq!(bar0_size, 2); // fails with `3 != 2`
    assert_eq!(bar1_size, 3); // fails with `7 != 3`
    assert_eq!(bar2_size, 4); // fails with `15 != 4`
    assert_eq!(bar3_size, 5); // fails with `31 != 5`
}

However, if async-blocks were implemented using something like enums with niche optimization, I would rather expect all cases to occupy a single byte.

@pnkfelix
Copy link
Member

Ah, my apologies for overlooking the playground link you had provided.

bherrera pushed a commit to misttech/integration that referenced this issue Feb 11, 2025
The size of the wrapping future created in ExecutionScope::spawn was
twice the size of the task passed into it. This was because the wrapping
future was capturing the task in an upvar and then moving it to a local
when polling it requiring space for 2 copies of the task.
rust-lang/rust#108906

Writing our own future prevents duplicating the space requirements. The
task spawned in VolumesDirectory::add_directory_entry is now 15312
bytes, down from 30640 bytes.

Original-Bug: b/393365596
Original-Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1198686
Original-Revision: fe8f3638c56955f5fbbc4f03428e7caf869e8cbb
GitOrigin-RevId: c89c6fd011718cde4f0f61941f314b03ead66fe4
Change-Id: I17dc4f4a3435d6abaa9c124f6a5f4e88c2d37a97
bherrera pushed a commit to misttech/integration that referenced this issue Feb 11, 2025
Rust futures generated from async move {} blocks are at least double the
size of all captured variables:
rust-lang/rust#108906

There are 3 locations in fxfs where a future and a guard object are
captured inside of another future with the purpose of running the
original future while the guard is held. Rolling our own future for
this specific case halves the size of the outer futures.

This reduces the size of the future spawned in
FxFile::create_connection_async for prefetch_keys from 10032 bytes to
5024 bytes.

Original-Bug: b/393365596
Original-Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1198687
Original-Revision: ec292e24290afb1d0aa96659f85075b6b23bbfbc
GitOrigin-RevId: f4b6749bb4fce66ec43aca06cf370f8cf6a3a9e3
Change-Id: I95c5034e44e2401253829efec2fa6ddadcdd37ba
bherrera pushed a commit to misttech/mist-os that referenced this issue Feb 11, 2025
The size of the wrapping future created in ExecutionScope::spawn was
twice the size of the task passed into it. This was because the wrapping
future was capturing the task in an upvar and then moving it to a local
when polling it requiring space for 2 copies of the task.
rust-lang/rust#108906

Writing our own future prevents duplicating the space requirements. The
task spawned in VolumesDirectory::add_directory_entry is now 15312
bytes, down from 30640 bytes.

Bug: b/393365596
Change-Id: Ib90a3ad99d42bd937effcac6bfa39d780ef188f4
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1198686
Commit-Queue: Chris Drouillard <cdrllrd@google.com>
Reviewed-by: Chris Suter <csuter@google.com>
bherrera pushed a commit to misttech/mist-os that referenced this issue Feb 11, 2025
Rust futures generated from async move {} blocks are at least double the
size of all captured variables:
rust-lang/rust#108906

There are 3 locations in fxfs where a future and a guard object are
captured inside of another future with the purpose of running the
original future while the guard is held. Rolling our own future for
this specific case halves the size of the outer futures.

This reduces the size of the future spawned in
FxFile::create_connection_async for prefetch_keys from 10032 bytes to
5024 bytes.

Bug: b/393365596
Change-Id: Id46f9939191a5ec63076a4ad35d220a00831152f
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1198687
Commit-Queue: Chris Drouillard <cdrllrd@google.com>
Reviewed-by: James Sullivan <jfsulliv@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-async Working group: Async & await
Projects
None yet
Development

No branches or pull requests

3 participants