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

Remove the lifetime constraint from the scope OP #785

Merged
merged 1 commit into from Aug 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions rayon-core/src/scope/mod.rs
Expand Up @@ -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, _| {
Expand Down Expand Up @@ -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, _| {
Expand Down
82 changes: 81 additions & 1 deletion 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;
Expand Down Expand Up @@ -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);
}
4 changes: 2 additions & 2 deletions rayon-core/src/thread_pool/mod.rs
Expand Up @@ -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))
Expand All @@ -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))
Expand Down
77 changes: 73 additions & 4 deletions rayon-core/src/thread_pool/test.rs
Expand Up @@ -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!")]
Expand Down Expand Up @@ -267,3 +264,75 @@ fn spawn_fifo_order() {
let expected: Vec<i32> = (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());
}