From 0d84c87b0abb522dcc7ec85bb72424d2cc78387b Mon Sep 17 00:00:00 2001 From: luca3s <41015240+luca3s@users.noreply.github.com> Date: Mon, 18 Nov 2024 23:24:59 +0100 Subject: [PATCH] removed spin_loop from left-right --- Cargo.lock | 5 -- simple-left-right/Cargo.toml | 14 ---- simple-left-right/src/lib.rs | 83 ++----------------- simple-left-right/src/{inner.rs => shared.rs} | 0 simple-left-right/tests/tests.rs | 26 +++--- tracker-engine/Cargo.toml | 4 +- tracker-engine/src/manager.rs | 65 +++++++++------ 7 files changed, 62 insertions(+), 135 deletions(-) rename simple-left-right/src/{inner.rs => shared.rs} (100%) diff --git a/Cargo.lock b/Cargo.lock index 1c44ee1..3f6a7d7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -715,11 +715,6 @@ checksum = "0fda2ff0d084019ba4d7c6f371c95d8fd75ce3524c3cb8fb653a3023f6323e64" [[package]] name = "simple-left-right" version = "0.2.0" -dependencies = [ - "async-io", - "crossbeam-utils", - "futures-lite", -] [[package]] name = "slab" diff --git a/simple-left-right/Cargo.toml b/simple-left-right/Cargo.toml index d4a95fa..e799974 100644 --- a/simple-left-right/Cargo.toml +++ b/simple-left-right/Cargo.toml @@ -16,17 +16,3 @@ description = "Lockfree, realtime safe and copy-free Synchronisation" [features] default = [] - -# enable thread sleeping in spin-locks -std = ["crossbeam-utils/std"] -# enable async sleeping and yielding in spin-locks -# uses the smol ecosystem. Should be compatible with every runtime -async = ["std", "dep:async-io", "dep:futures-lite"] - -[dependencies] -# only for backoff spinlocks. -crossbeam-utils = { version = "0.8.20", default-features = false } -# async sleeping -async-io = { version = "2.3.4", optional = true } -# async yielding -futures-lite = {version = "2.3.0", optional = true, default-features = false } diff --git a/simple-left-right/src/lib.rs b/simple-left-right/src/lib.rs index 168ba6d..280f325 100644 --- a/simple-left-right/src/lib.rs +++ b/simple-left-right/src/lib.rs @@ -21,14 +21,10 @@ clippy::missing_safety_doc, clippy::undocumented_unsafe_blocks )] -#![cfg_attr(not(feature = "std"), no_std)] -extern crate alloc; +#![no_std] -#[cfg(feature = "std")] -use core::time::Duration; -#[cfg(feature = "std")] -use std::thread; +extern crate alloc; use core::{ cell::UnsafeCell, @@ -40,9 +36,9 @@ use core::{ use alloc::{boxed::Box, collections::vec_deque::VecDeque}; -mod inner; +mod shared; -use inner::{Ptr, Shared}; +use shared::{Ptr, Shared}; /// Should be implemented on structs that want to be shared with this library pub trait Absorb { @@ -201,75 +197,8 @@ impl Writer { } impl, O> Writer { - /// Blocks if the Reader has a `ReadGuard` pointing to the old value. - /// - /// Uses a Spinlock because for anything else the OS needs to be involved and `Reader` can't talk to the OS. - pub fn lock(&mut self) -> WriteGuard<'_, T, O> { - let backoff = crossbeam_utils::Backoff::new(); - - loop { - if self.shared_ref().lock_write(self.write_ptr).is_ok() { - break; - } - - backoff.snooze(); - } - - // SAFETY: The spinloop before is only exited once the ReadState allows writing to the current - // write_ptr value. - unsafe { WriteGuard::new(self) } - } - - /// Blocks if the Reader has a `ReadGuard` pointing to the old value. - /// - /// Uses a spin-lock, because the `Reader` can't talk to the OS. Sleeping and Yielding is done to avoid wasting cycles. - /// Equivalent to ´lock´, except that it starts sleeping the given duration after a certaint point until the lock could be aquired. - #[cfg(feature = "std")] - pub fn sleep_lock(&mut self, sleep: Duration) -> WriteGuard<'_, T, O> { - let backoff = crossbeam_utils::Backoff::new(); - - loop { - if self.shared_ref().lock_write(self.write_ptr).is_ok() { - break; - } - - if backoff.is_completed() { - thread::sleep(sleep); - } else { - backoff.snooze(); - } - } - - // SAFETY: The spinloop before is only exited once the ReadState allows writing to the current - // write_ptr value. - unsafe { WriteGuard::new(self) } - } - - /// Equivalent to `lock` but the sleeping is done asyncly to not block the runtime. - /// It is still a spinlock, it just give control to the runtime when locking is slow, before trying again. - #[cfg(feature = "async")] - pub async fn async_lock(&mut self, sleep: Duration) -> WriteGuard<'_, T, O> { - let backoff = crossbeam_utils::Backoff::new(); - - loop { - if self.shared_ref().lock_write(self.write_ptr).is_ok() { - break; - } - - if backoff.is_completed() { - async_io::Timer::after(sleep).await; - } else { - backoff.spin(); - futures_lite::future::yield_now().await; - } - } - - // SAFETY: The spinloop before is only exited once the ReadState allows writing to the current - // write_ptr value. - unsafe { WriteGuard::new(self) } - } - - /// doesn't block. Returns None if the Reader has a `ReadGuard` pointing to the old value + /// doesn't block. Returns None if the Reader has a `ReadGuard` pointing to the old value. + #[must_use] pub fn try_lock(&mut self) -> Option> { self.shared_ref() .lock_write(self.write_ptr) diff --git a/simple-left-right/src/inner.rs b/simple-left-right/src/shared.rs similarity index 100% rename from simple-left-right/src/inner.rs rename to simple-left-right/src/shared.rs diff --git a/simple-left-right/tests/tests.rs b/simple-left-right/tests/tests.rs index 6df8140..9cf2be8 100644 --- a/simple-left-right/tests/tests.rs +++ b/simple-left-right/tests/tests.rs @@ -1,7 +1,15 @@ #[cfg(test)] mod tests { - use simple_left_right::{Absorb, Writer}; - use std::{cell::Cell, time::Duration}; + use simple_left_right::{Absorb, WriteGuard, Writer}; + use std::{cell::Cell, hint, time::Duration}; + + fn spin_lock, O>(writer: &mut Writer) -> WriteGuard<'_, T, O> { + // please polonius save me from this hell + while writer.try_lock().is_none() { + hint::spin_loop(); + } + writer.try_lock().unwrap() + } #[derive(Clone)] pub struct CounterAddOp(i32); @@ -34,18 +42,17 @@ mod tests { fn write_guard_drop() { let mut writer = Writer::new(0); let mut reader = writer.build_reader().unwrap(); - let sleep = Duration::from_millis(5); assert_eq!(*reader.lock(), 0); - let mut write_lock = writer.sleep_lock(sleep); + let mut write_lock = spin_lock(&mut writer); assert_eq!(*write_lock.read(), 0); write_lock.apply_op(CounterAddOp(2)); assert_eq!(*write_lock.read(), 2); drop(write_lock); - let write_lock = writer.sleep_lock(sleep); + let write_lock = spin_lock(&mut writer); assert_eq!(*write_lock.read(), 2); write_lock.swap(); assert_eq!(*reader.lock(), 2); - assert_eq!(*writer.sleep_lock(sleep).read(), 2); + assert_eq!(*spin_lock(&mut writer).read(), 2); } #[test] @@ -106,9 +113,8 @@ mod tests { fn block() { let mut writer = Writer::new(0); let mut reader = writer.build_reader().unwrap(); - let sleep = Duration::from_millis(5); - writer.sleep_lock(sleep).apply_op(CounterAddOp(2)); + spin_lock(&mut writer).apply_op(CounterAddOp(2)); std::thread::spawn(move || { let lock = reader.lock(); assert_eq!(*lock, 0); @@ -117,9 +123,9 @@ mod tests { assert_eq!(*reader.lock(), 2); }); std::thread::sleep(Duration::from_secs(1)); - writer.sleep_lock(sleep).swap(); + spin_lock(&mut writer).swap(); // blocks until the spawned thread drops the read_lock - let write_lock = writer.sleep_lock(sleep); + let write_lock = spin_lock(&mut writer); assert_eq!(*write_lock.read(), 2); } diff --git a/tracker-engine/Cargo.toml b/tracker-engine/Cargo.toml index 2c5ee44..bd3274a 100644 --- a/tracker-engine/Cargo.toml +++ b/tracker-engine/Cargo.toml @@ -12,7 +12,7 @@ keywords = ["audio"] categories = ["audio"] [features] -async = ["dep:async-io", "dep:futures-lite", "dep:async-channel", "dep:futures-io", "simple-left-right/async"] +async = ["dep:async-io", "dep:futures-lite", "dep:async-channel", "dep:futures-io"] [dependencies] async-io = { version = "2.3.4", optional = true } @@ -24,7 +24,7 @@ basedrop = "0.1.2" # miri reports Race Condition. Wait for fix or pull inside cpal = "0.15.3" crossbeam-utils = "0.8.20" rtrb = "0.3.1" -simple-left-right = { path = "../simple-left-right", features = ["std"] } +simple-left-right = { path = "../simple-left-right" } # assert_no_alloc [dev-dependencies] diff --git a/tracker-engine/src/manager.rs b/tracker-engine/src/manager.rs index ba05398..e5b708f 100644 --- a/tracker-engine/src/manager.rs +++ b/tracker-engine/src/manager.rs @@ -23,6 +23,37 @@ enum ManageCollector { External(async_channel::Sender, Handle), } +fn spin(mut f: impl FnMut() -> bool, time: Duration) { + let backoff = crossbeam_utils::Backoff::new(); + loop { + if f() { + return; + } + + if backoff.is_completed() { + std::thread::sleep(time); + } else { + backoff.snooze(); + } + } +} + +#[cfg(feature = "async")] +async fn async_spin(mut f: impl FnMut() -> bool, time: Duration) { + let backoff = crossbeam_utils::Backoff::new(); + loop { + if f() { + return; + } + + if backoff.is_completed() { + async_io::Timer::after(time).await; + } else { + backoff.snooze(); + } + } +} + impl ManageCollector { fn handle(&self) -> Handle { match self { @@ -166,8 +197,9 @@ impl AudioManager { /// /// Spinloops until no more ReadGuard to the old value exists pub fn edit_song(&mut self) -> SongEdit<'_> { + spin(|| self.song.try_lock().is_some(), Self::SPIN_SLEEP); SongEdit { - song: self.song.sleep_lock(Self::SPIN_SLEEP), + song: self.song.try_lock().unwrap(), gc: &mut self.gc } } @@ -233,18 +265,7 @@ impl AudioManager { pub fn send_worker_msg(&mut self, msg: ToWorkerMsg) { if let Some((_, channel)) = &mut self.stream { - let backoff = crossbeam_utils::Backoff::new(); - loop { - if channel.push(msg).is_ok() { - return; - } - - if backoff.is_completed() { - std::thread::sleep(Self::SPIN_SLEEP); - } else { - backoff.snooze(); - } - } + spin(|| channel.push(msg).is_ok(), Self::SPIN_SLEEP); } } @@ -253,7 +274,7 @@ impl AudioManager { self.send_worker_msg(ToWorkerMsg::StopPlayback); self.send_worker_msg(ToWorkerMsg::StopLiveNote); drop(stream); - #[cfg(not(feature = "async"))] + self.gc.collect(); } } @@ -264,18 +285,7 @@ impl AudioManager { impl AudioManager { pub async fn async_send_worker_msg(&mut self, msg: ToWorkerMsg) { if let Some((_, channel)) = &mut self.stream { - let backoff = crossbeam_utils::Backoff::new(); - loop { - if channel.push(msg).is_ok() { - return; - } - - if backoff.is_completed() { - async_io::Timer::after(Self::SPIN_SLEEP).await; - } else { - backoff.snooze(); - } - } + async_spin(|| channel.push(msg).is_ok(), Self::SPIN_SLEEP).await; } } @@ -296,8 +306,9 @@ impl AudioManager { ManageCollector::External(_, handle) => handle.clone(), }; + async_spin(|| self.song.try_lock().is_some(), Self::SPIN_SLEEP).await; SongEdit { - song: self.song.async_lock(Self::SPIN_SLEEP).await, + song: self.song.try_lock().unwrap(), gc: &mut self.gc } } -- 2.51.2