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

Replace MaybeDone/Fuse with separate input/output/state fields #22

Open
9 of 13 tasks
yoshuawuyts opened this issue Nov 4, 2022 · 4 comments
Open
9 of 13 tasks
Labels
performance computer goes brrrrr

Comments

@yoshuawuyts
Copy link
Owner

yoshuawuyts commented Nov 4, 2022

Right now we're using MaybeDone internally to track the state of each future. It can either be: "pending", "containing data", or "done". This is nice from an implementation perspective, but bad for performance. Namely:

  1. It creates unnecessary copies, and sometimes also allocations. Once to store the futures in the MaybeDone structure. And once to copy the data to the output structure.
  2. It creates unnecessarily large memory needs. At any point we may be using up to 2x the amount of memory needed, since we're doing redundant copies.

The solution is to move the container of futures in-line. Create the container of outputs containing MaybeUninit. And finally create a separate container to track the state. Once all futures have completed, we can mark the output container as "initialized" and immediately yield that.

Tasks

Priority

Secondary

Eventually

  • TryJoin for Vec
  • TryJoin for Array
  • RaceOk for Vec
  • RaceOk for Array
This was referenced Nov 4, 2022
@yoshuawuyts yoshuawuyts added enhancement New feature or request performance computer goes brrrrr and removed enhancement New feature or request labels Nov 9, 2022
@yoshuawuyts yoshuawuyts changed the title Move away from MaybeDone to sets of outputs Move away from MaybeDone/Fuse to sets of outputs Nov 14, 2022
@yoshuawuyts yoshuawuyts changed the title Move away from MaybeDone/Fuse to sets of outputs Replace MaybeDone/Fuse with separate input/output/state fields Nov 14, 2022
@poliorcetics
Copy link
Contributor

So @yoshuawuyts talked about this on mastodon, I proposed in response to associate index and relevant generic in the macros like so:

macro_rules! associated {
    (/* other elements */; $( ($Fi: literal, $Ft: ident) ),+; /* other elements */) => {
        #[test]
        fn test_indexes_are_sorted() {
            let indexes = [$($Fi),+];
            assert!(indexes.is_sorted(), "Indexes are not sorted in call to macro `associated!()`");
        }
        // With the test running in CI you can use the following to get the max index easily:
        const LEN: usize = {
            let indexes = [$($Fi),+];
            let min = match indexes {
                 [min, ..] => min,
            };
            let max = match indexes {
                 [.., max] => max,
            };
            max - min + 1
        } ;
    };
}

@poliorcetics
Copy link
Contributor

If you are having names conflicts for testing or such, what you can do is add a $test_mod_name: ident member to the macro and put them in

#[cfg(test)
mod $test_mod_name {
    use super::*;
    /* put tests here */
}

@yoshuawuyts
Copy link
Owner Author

#74 is probably the furthest along in setting up the machinery required for the tuples. It just needs one last change to ensure that tuple outputs are written in-place, rather than copied.

@poliorcetics
Copy link
Contributor

It can probably be done without using "paste" but I'm being picky 😄 if I find the time to make a PR before the other is merged I'll make a comparison

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance computer goes brrrrr
Projects
None yet
Development

No branches or pull requests

2 participants