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

rebinding values in an async function can double the memory usage #96084

Open
jkarneges opened this issue Apr 15, 2022 · 4 comments
Open

rebinding values in an async function can double the memory usage #96084

jkarneges opened this issue Apr 15, 2022 · 4 comments
Labels
A-async-await Area: Async & Await AsyncAwait-Polish Async-await issues that are part of the "polish" area AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting.

Comments

@jkarneges
Copy link
Contributor

Rebinding a value within an async function can cause the generated future to require twice the memory for the value. This seems to only happen if the value was borrowed earlier:

use std::mem;

async fn foo() {
    let x = [0u8; 100];
    async {}.await;
    println!("{}", x.len());
}

async fn a() {
    let fut = foo();
    let fut = fut;
    fut.await;
}

async fn b() {
    let fut = foo();
    println!("{}", mem::size_of_val(&fut));
    let fut = fut;
    fut.await;
}

fn main() {
    assert_eq!(mem::size_of_val(&foo()), 102);

    // 1 + sizeof(foo)
    assert_eq!(mem::size_of_val(&a()), 103);

    // 1 + (sizeof(foo) * 2)
    assert_eq!(mem::size_of_val(&b()), 205);
}

Playground link.

I don't know if this counts as a bug but the behavior is a bit surprising.

The effect is present in both debug and release modes with rustc 1.60.0.

@eholk
Copy link
Contributor

eholk commented Apr 15, 2022

@rustbot label A-async-await AsyncAwait-Polish

@rustbot rustbot added A-async-await Area: Async & Await AsyncAwait-Polish Async-await issues that are part of the "polish" area labels Apr 15, 2022
@tmandry
Copy link
Member

tmandry commented Apr 22, 2022

Hi! This looks related to #62321 and #61849 but not exactly the same as either of those. It might be easier to solve.

@eholk
Copy link
Contributor

eholk commented Apr 25, 2022

We discussed this in triage today. It seems like the issue is probably that MaybeBorrowedLocals is being too conservative. Once it finds a borrowed local, it stays live until it finds a StorageDead instruction, and these get inserted at the end of the scope. It seems like once we move out of a local, we should be able to mark that as StorageDead, instead of waiting until the end of the scope, which would probably fix this issue.

@rustbot label +AsyncAwait-Triaged

@rustbot rustbot added the AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. label Apr 25, 2022
@tmandry
Copy link
Member

tmandry commented Apr 25, 2022

In #60187 I think I attempted making a MIR building change that caused StorageDead to be emitted earlier. The performance hit was unacceptable for doing this for all functions (benchmark results), but there was discussion about doing this for only generators. I can't remember where exactly we landed on that, but I don't see a diff in the MIR building code of the final PR. So maybe that's the thread to pick up again from here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-async-await Area: Async & Await AsyncAwait-Polish Async-await issues that are part of the "polish" area AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting.
Projects
Status: On deck
4 participants