From 66559fe9ce1e4f05ac011b65172c6a3ca9bf2ea9 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Fri, 14 Aug 2020 12:43:38 -0700 Subject: [PATCH] Remove the lifetime constraint from the scope OP We already know the `OP` lifetime must outlive the `scope` call itself, and we'll execute it synchronously, but we don't need to tie that to the `'scope` lifetime. By relaxing this, we can create scopes for different lifetimes than `OP` can satisfy, even fully `'static`. They will still await completion by the end of the `scope` call, but this can be useful for collecting the invariant scopes from different contexts, as the new nested tests demonstrate. --- rayon-core/src/scope/mod.rs | 4 +- rayon-core/src/scope/test.rs | 82 +++++++++++++++++++++++++++++- rayon-core/src/thread_pool/mod.rs | 4 +- rayon-core/src/thread_pool/test.rs | 77 ++++++++++++++++++++++++++-- 4 files changed, 158 insertions(+), 9 deletions(-) diff --git a/rayon-core/src/scope/mod.rs b/rayon-core/src/scope/mod.rs index 7d274920b..a41d408e1 100644 --- a/rayon-core/src/scope/mod.rs +++ b/rayon-core/src/scope/mod.rs @@ -285,7 +285,7 @@ struct ScopeBase<'scope> { /// propagated at that point. pub fn scope<'scope, OP, R>(op: OP) -> R where - OP: for<'s> FnOnce(&'s Scope<'scope>) -> R + 'scope + Send, + OP: FnOnce(&Scope<'scope>) -> R + Send, R: Send, { in_worker(|owner_thread, _| { @@ -376,7 +376,7 @@ where /// panics are propagated at that point. pub fn scope_fifo<'scope, OP, R>(op: OP) -> R where - OP: for<'s> FnOnce(&'s ScopeFifo<'scope>) -> R + 'scope + Send, + OP: FnOnce(&ScopeFifo<'scope>) -> R + Send, R: Send, { in_worker(|owner_thread, _| { diff --git a/rayon-core/src/scope/test.rs b/rayon-core/src/scope/test.rs index 3d855ecda..8cb82b615 100644 --- a/rayon-core/src/scope/test.rs +++ b/rayon-core/src/scope/test.rs @@ -1,6 +1,6 @@ use crate::unwind; use crate::ThreadPoolBuilder; -use crate::{scope, scope_fifo, Scope}; +use crate::{scope, scope_fifo, Scope, ScopeFifo}; use rand::{Rng, SeedableRng}; use rand_xorshift::XorShiftRng; use std::cmp; @@ -433,3 +433,83 @@ fn mixed_fifo_lifo_order() { let expected = vec![-3, 0, -2, 1, -1, 2, 3]; assert_eq!(vec, expected); } + +#[test] +fn static_scope() { + static COUNTER: AtomicUsize = AtomicUsize::new(0); + + let mut range = 0..100; + let sum = range.clone().sum(); + let iter = &mut range; + + COUNTER.store(0, Ordering::Relaxed); + scope(|s: &Scope<'static>| { + // While we're allowed the locally borrowed iterator, + // the spawns must be static. + for i in iter { + s.spawn(move |_| { + COUNTER.fetch_add(i, Ordering::Relaxed); + }); + } + }); + + assert_eq!(COUNTER.load(Ordering::Relaxed), sum); +} + +#[test] +fn static_scope_fifo() { + static COUNTER: AtomicUsize = AtomicUsize::new(0); + + let mut range = 0..100; + let sum = range.clone().sum(); + let iter = &mut range; + + COUNTER.store(0, Ordering::Relaxed); + scope_fifo(|s: &ScopeFifo<'static>| { + // While we're allowed the locally borrowed iterator, + // the spawns must be static. + for i in iter { + s.spawn_fifo(move |_| { + COUNTER.fetch_add(i, Ordering::Relaxed); + }); + } + }); + + assert_eq!(COUNTER.load(Ordering::Relaxed), sum); +} + +#[test] +fn mixed_lifetime_scope() { + fn increment<'slice, 'counter>(counters: &'slice [&'counter AtomicUsize]) { + scope(move |s: &Scope<'counter>| { + // We can borrow 'slice here, but the spawns can only borrow 'counter. + for &c in counters { + s.spawn(move |_| { + c.fetch_add(1, Ordering::Relaxed); + }); + } + }); + } + + let counter = AtomicUsize::new(0); + increment(&[&counter; 100]); + assert_eq!(counter.into_inner(), 100); +} + +#[test] +fn mixed_lifetime_scope_fifo() { + fn increment<'slice, 'counter>(counters: &'slice [&'counter AtomicUsize]) { + scope_fifo(move |s: &ScopeFifo<'counter>| { + // We can borrow 'slice here, but the spawns can only borrow 'counter. + for &c in counters { + s.spawn_fifo(move |_| { + c.fetch_add(1, Ordering::Relaxed); + }); + } + }); + } + + let counter = AtomicUsize::new(0); + increment(&[&counter; 100]); + assert_eq!(counter.into_inner(), 100); +} diff --git a/rayon-core/src/thread_pool/mod.rs b/rayon-core/src/thread_pool/mod.rs index 065d236e0..2209f6304 100644 --- a/rayon-core/src/thread_pool/mod.rs +++ b/rayon-core/src/thread_pool/mod.rs @@ -200,7 +200,7 @@ impl ThreadPool { /// [scope]: fn.scope.html pub fn scope<'scope, OP, R>(&self, op: OP) -> R where - OP: for<'s> FnOnce(&'s Scope<'scope>) -> R + 'scope + Send, + OP: FnOnce(&Scope<'scope>) -> R + Send, R: Send, { self.install(|| scope(op)) @@ -215,7 +215,7 @@ impl ThreadPool { /// [scope_fifo]: fn.scope_fifo.html pub fn scope_fifo<'scope, OP, R>(&self, op: OP) -> R where - OP: for<'s> FnOnce(&'s ScopeFifo<'scope>) -> R + 'scope + Send, + OP: FnOnce(&ScopeFifo<'scope>) -> R + Send, R: Send, { self.install(|| scope_fifo(op)) diff --git a/rayon-core/src/thread_pool/test.rs b/rayon-core/src/thread_pool/test.rs index 0d6815e12..8d1c90ca1 100644 --- a/rayon-core/src/thread_pool/test.rs +++ b/rayon-core/src/thread_pool/test.rs @@ -4,12 +4,9 @@ use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::mpsc::channel; use std::sync::{Arc, Mutex}; -use crate::join; -use crate::thread_pool::ThreadPool; - #[allow(deprecated)] use crate::Configuration; -use crate::ThreadPoolBuilder; +use crate::{join, Scope, ScopeFifo, ThreadPool, ThreadPoolBuilder}; #[test] #[should_panic(expected = "Hello, world!")] @@ -267,3 +264,75 @@ fn spawn_fifo_order() { let expected: Vec = (0..10).collect(); // FIFO -> natural order assert_eq!(vec, expected); } + +#[test] +fn nested_scopes() { + // Create matching scopes for every thread pool. + fn nest<'scope, OP>(pools: &[ThreadPool], scopes: Vec<&Scope<'scope>>, op: OP) + where + OP: FnOnce(&[&Scope<'scope>]) + Send, + { + if let Some((pool, tail)) = pools.split_first() { + pool.scope(move |s| { + // This move reduces the reference lifetimes by variance to match s, + // but the actual scopes are still tied to the invariant 'scope. + let mut scopes = scopes; + scopes.push(s); + nest(tail, scopes, op) + }) + } else { + (op)(&scopes) + } + } + + let pools: Vec<_> = (0..10) + .map(|_| ThreadPoolBuilder::new().num_threads(1).build().unwrap()) + .collect(); + + let counter = AtomicUsize::new(0); + nest(&pools, vec![], |scopes| { + for &s in scopes { + s.spawn(|_| { + // Our 'scope lets us borrow the counter in every pool. + counter.fetch_add(1, Ordering::Relaxed); + }); + } + }); + assert_eq!(counter.into_inner(), pools.len()); +} + +#[test] +fn nested_fifo_scopes() { + // Create matching fifo scopes for every thread pool. + fn nest<'scope, OP>(pools: &[ThreadPool], scopes: Vec<&ScopeFifo<'scope>>, op: OP) + where + OP: FnOnce(&[&ScopeFifo<'scope>]) + Send, + { + if let Some((pool, tail)) = pools.split_first() { + pool.scope_fifo(move |s| { + // This move reduces the reference lifetimes by variance to match s, + // but the actual scopes are still tied to the invariant 'scope. + let mut scopes = scopes; + scopes.push(s); + nest(tail, scopes, op) + }) + } else { + (op)(&scopes) + } + } + + let pools: Vec<_> = (0..10) + .map(|_| ThreadPoolBuilder::new().num_threads(1).build().unwrap()) + .collect(); + + let counter = AtomicUsize::new(0); + nest(&pools, vec![], |scopes| { + for &s in scopes { + s.spawn_fifo(|_| { + // Our 'scope lets us borrow the counter in every pool. + counter.fetch_add(1, Ordering::Relaxed); + }); + } + }); + assert_eq!(counter.into_inner(), pools.len()); +}