From eb5493b4ec0fbb5458b9f84e3273900e1dd08cce Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 16 Apr 2025 14:32:48 +0200 Subject: [PATCH] Remove incorrect `parallel_scope` API Rayon does not allow joining on the spawns, meaning we might end up with results in the tests before reading out the panics --- src/lib.rs | 2 +- src/parallel.rs | 33 --------- tests/parallel/main.rs | 1 - tests/parallel/parallel_scope.rs | 114 ------------------------------- 4 files changed, 1 insertion(+), 149 deletions(-) delete mode 100644 tests/parallel/parallel_scope.rs diff --git a/src/lib.rs b/src/lib.rs index b0e3836cd..48676b83f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -34,7 +34,7 @@ mod zalsa; mod zalsa_local; #[cfg(feature = "rayon")] -pub use parallel::{join, par_map, scope, Scope}; +pub use parallel::{join, par_map}; #[cfg(feature = "macros")] pub use salsa_macros::{accumulator, db, input, interned, tracked, Supertype, Update}; diff --git a/src/parallel.rs b/src/parallel.rs index d3033af5b..1d2504b77 100644 --- a/src/parallel.rs +++ b/src/parallel.rs @@ -26,39 +26,6 @@ impl Clone for DbForkOnClone { } } -pub struct Scope<'scope, 'local, Db: Database + ?Sized> { - db: &'local Db, - base: &'local rayon::Scope<'scope>, -} - -impl<'scope, 'local, Db: Database + ?Sized> Scope<'scope, 'local, Db> { - pub fn spawn(&self, body: BODY) - where - BODY: for<'l> FnOnce(&'l Scope<'scope, 'l, Db>) + Send + 'scope, - { - let db = self.db.fork_db(); - self.base.spawn(move |scope| { - let scope = Scope { - db: db.as_view::(), - base: scope, - }; - body(&scope) - }) - } - - pub fn db(&self) -> &'local Db { - self.db - } -} - -pub fn scope<'scope, Db: Database + ?Sized, OP, R>(db: &Db, op: OP) -> R -where - OP: FnOnce(&Scope<'scope, '_, Db>) -> R + Send, - R: Send, -{ - rayon::in_place_scope(move |s| op(&Scope { db, base: s })) -} - pub fn join(db: &Db, a: A, b: B) -> (RA, RB) where A: FnOnce(&Db) -> RA + Send, diff --git a/tests/parallel/main.rs b/tests/parallel/main.rs index 4f4295463..613e43e1d 100644 --- a/tests/parallel/main.rs +++ b/tests/parallel/main.rs @@ -7,5 +7,4 @@ mod cycle_panic; mod parallel_cancellation; mod parallel_join; mod parallel_map; -mod parallel_scope; mod signal; diff --git a/tests/parallel/parallel_scope.rs b/tests/parallel/parallel_scope.rs deleted file mode 100644 index c4fc0103b..000000000 --- a/tests/parallel/parallel_scope.rs +++ /dev/null @@ -1,114 +0,0 @@ -#![cfg(feature = "rayon")] -// test for rayon-like scope interactions. - -use salsa::{Cancelled, Setter}; - -use crate::setup::{Knobs, KnobsDatabase}; - -#[salsa::input] -struct ParallelInput { - a: u32, - b: u32, -} - -#[salsa::tracked] -fn tracked_fn(db: &dyn salsa::Database, input: ParallelInput) -> (u32, u32) { - let mut a = None; - let mut b = None; - salsa::scope(db, |scope| { - scope.spawn(|scope| a = Some(input.a(scope.db()) + 1)); - scope.spawn(|scope| b = Some(input.b(scope.db()) + 1)); - }); - (a.unwrap(), b.unwrap()) -} - -#[salsa::tracked] -fn a1(db: &dyn KnobsDatabase, input: ParallelInput) -> (u32, u32) { - db.signal(1); - let mut a = None; - let mut b = None; - salsa::scope(db, |scope| { - scope.spawn(|scope| { - scope.db().wait_for(2); - a = Some(input.a(scope.db()) + 1) - }); - scope.spawn(|scope| { - scope.db().wait_for(2); - b = Some(input.b(scope.db()) + 1) - }); - }); - (a.unwrap(), b.unwrap()) -} - -#[salsa::tracked] -fn dummy(_db: &dyn KnobsDatabase) -> u32 { - panic!("should never get here!") -} - -#[test] -#[cfg_attr(miri, ignore)] -fn execute() { - let db = salsa::DatabaseImpl::new(); - - let input = ParallelInput::new(&db, 10, 20); - - tracked_fn(&db, input); -} - -// we expect this to panic, as `salsa::par_map` needs to be called from a query. -#[test] -#[cfg_attr(miri, ignore)] -#[should_panic] -fn direct_calls_panic() { - let db = salsa::DatabaseImpl::new(); - - let input = ParallelInput::new(&db, 10, 20); - salsa::scope(&db, |scope| { - scope.spawn(|scope| _ = input.a(scope.db()) + 1); - scope.spawn(|scope| _ = input.b(scope.db()) + 1); - }); -} - -// Cancellation signalling test -// -// The pattern is as follows. -// -// Thread A Thread B -// -------- -------- -// a1 -// | wait for stage 1 -// signal stage 1 set input, triggers cancellation -// wait for stage 2 (blocks) triggering cancellation sends stage 2 -// | -// (unblocked) -// dummy -// panics - -#[test] -#[cfg_attr(miri, ignore)] -fn execute_cancellation() { - let mut db = Knobs::default(); - - let input = ParallelInput::new(&db, 10, 20); - - let thread_a = std::thread::spawn({ - let db = db.clone(); - move || a1(&db, input) - }); - - db.signal_on_did_cancel(2); - input.set_a(&mut db).to(30); - - // Assert thread A was cancelled - let cancelled = thread_a - .join() - .unwrap_err() - .downcast::() - .unwrap(); - - // and inspect the output - expect_test::expect![[r#" - PendingWrite - "#]] - .assert_debug_eq(&cancelled); -}