Skip to content

Conversation

@NthTensor
Copy link
Owner

This is a large-scale overhaul of the scope implementation.

  • Loosen an overly restrictive memory ordering for the scope counter.
  • Switch from rayon-style to bevy-style scopes (two lifetimes instead of one). This allows references to scopes to be passed around more freely, and will reduce migration pain down the road.
  • Make scopes properly panic safe by porting over the panic propagation from rayon.
  • Pass the worker to blocking tasks spawned on scopes, just like we do for static tasks.
  • Clean up documentation and generally spruce things up a bit.
  • Remove pointless use of pin in favor of an unsafe assertion that scopes don't move. Pin turned out to be far more haste than it was worth.
  • Remove the ability to spawn async functions on scopes. I'll be removing this from thread pool soon as well. Futures are simpler to work with, and this created some nontrivial duplication for little benefit.

@NthTensor NthTensor mentioned this pull request Aug 20, 2025
@NthTensor NthTensor force-pushed the scope-cleanup branch 2 times, most recently from 75e179b to ee9a95b Compare August 20, 2025 03:55
@NthTensor NthTensor merged commit 87f4caf into main Aug 21, 2025
4 checks passed
@NthTensor NthTensor deleted the scope-cleanup branch August 21, 2025 02:35
Copy link

@hymm hymm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just saw that you merged this. Code looks good, just had a couple non-blocking comments.

}

#[test]
fn scoped_concurrency() {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you might want to add a test for nested calls to scope. That was something that I saw deadlock in a couple of the other executors that I tested. In this case it'd be something like

        THREAD_POOL.with_worker(|worker| {
            scope(|scope| {
                scope.spawn_on(worker, |_| {
                    THREAD_POOL.with_worker(|worker| {
                         scope(|scope| {
                             scope.spawn_on(worker, |_| {
                                 string = "b";
                             })
                        });
                    });
                })
            });
        });

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

Successfully merging this pull request may close these issues.

5 participants