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

Provide map() and map_mut() for par_iter_mut() #10215

Open
mscofield0 opened this issue Oct 21, 2023 · 3 comments
Open

Provide map() and map_mut() for par_iter_mut() #10215

mscofield0 opened this issue Oct 21, 2023 · 3 comments
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Trivial Nice and easy! A great choice to get started with Bevy

Comments

@mscofield0
Copy link

mscofield0 commented Oct 21, 2023

What problem does this solve or what need does it fill?

For when you need to iterate through a query and generate some sort of result from each of the parallel iterators. e.g. reading websockets of sessions and then returning the locally collected requests, flattening it and collecting them all into a single requests list for further processing.

What solution would you like?

I would like map() and map_mut() (because websocket readers require mutable access). They should be used like so:

fn read_ws(mut query: Query<(&mut WsReader)>) {
  let reqs = query
    .par_iter_mut()
    .map_mut(|(mut ws_reader)| {
      let mut reqs = Vec::new();
      // ... reading from the ws reader
      reqs
    })
    .flatten()
    .collect::<Vec<_>>();
}

What alternative(s) have you considered?

The alternative is cumbersome, but doable:

let len = query.iter().len();
let mut i = AtomicUsize::new(0);
let mut reqs = Arc::<Vec<MaybeUninit<Vec<wire::Req>>>>::default();
reqs.resize_with(len, || MaybeUninit::uninit());
query.par_iter_mut().for_each_mut(|(mut ws_reader)| {
  let local_i = i.fetch_add(1, std::sync::atomic::Ordering::AcqRel);
  let mut local_reqs = Vec::new();
  // ... read from ws
  reqs[local_i] = MaybeUninit::new(local_reqs);
});

let reqs = unsafe { std::mem::transmute::<_, Vec<Vec<Req>>>(results) };
// do something with reqs

As you can see, not very pretty.

@mscofield0 mscofield0 added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Oct 21, 2023
@alice-i-cecile alice-i-cecile added D-Trivial Nice and easy! A great choice to get started with Bevy A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use and removed C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Oct 21, 2023
@alice-i-cecile
Copy link
Member

alice-i-cecile commented Oct 21, 2023

Fully on board with this. Do we want filter and reduce as well?

@mscofield0
Copy link
Author

Might as well!

@hymm
Copy link
Contributor

hymm commented Oct 21, 2023

note that for this specific use case it's probably better to do what bevy is doing internally and use the thread_local crate and store the thread local inside of a Local<T>, so that you can reuse the allocations.

pub fn check_visibility(
mut thread_queues: Local<ThreadLocal<Cell<Vec<Entity>>>>,

We have this PR to make that pattern more ergonomic #7348.

For more parallel iterator methods, we have a bunch of traits in bevy_tasks that used to be implemented on queries, but were not ported over with the ecs v2 rewrite. I'm not sure if we should use those or just throw them away and write something else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Trivial Nice and easy! A great choice to get started with Bevy
Projects
None yet
Development

No branches or pull requests

3 participants