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

Replace custom spinlock with spin #41

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ __test = []
crossbeam-utils = { version = "0.8.12", default-features = false }
parking = { version = "2.0.0", optional = true }
slab = { version = "0.4.7", default-features = false }
spin = { version = "0.9.4", default-features = false, features = ["spin_mutex"] }
Copy link
Member

Choose a reason for hiding this comment

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

imo it would be more readable to replace this with

[dependencies.spin]
version = "0.9.4"
default-features = false
features = ["spin_mutex"]


[dev-dependencies]
waker-fn = "1"
Expand Down
48 changes: 12 additions & 36 deletions src/inner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@
use crate::list::List;
use crate::node::Node;
use crate::queue::Queue;
use crate::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
use crate::sync::cell::UnsafeCell;
use crate::sync::atomic::{AtomicUsize, Ordering};
use crate::Task;

use alloc::vec;
Expand Down Expand Up @@ -129,32 +128,22 @@ impl Drop for ListGuard<'_> {

/// A simple mutex type that optimistically assumes that the lock is uncontended.
struct Mutex<T> {
/// The inner value.
value: UnsafeCell<T>,

/// Whether the mutex is locked.
locked: AtomicBool,
inner: spin::Mutex<T>,
}

impl<T> Mutex<T> {
/// Create a new mutex.
pub(crate) fn new(value: T) -> Self {
Self {
value: UnsafeCell::new(value),
locked: AtomicBool::new(false),
inner: spin::Mutex::new(value),
}
}

/// Lock the mutex.
pub(crate) fn try_lock(&self) -> Option<MutexGuard<'_, T>> {
// Try to lock the mutex.
if self
.locked
.compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed)
.is_ok()
{
// We have successfully locked the mutex.
Some(MutexGuard { mutex: self })
if let Some(guard) = self.inner.try_lock() {
Some(MutexGuard { guard })
} else {
self.try_lock_slow()
}
Expand All @@ -167,17 +156,13 @@ impl<T> Mutex<T> {
let mut spins = 100u32;

loop {
if self
.locked
.compare_exchange_weak(false, true, Ordering::Acquire, Ordering::Relaxed)
.is_ok()
{
if let Some(guard) = self.inner.try_lock() {
// We have successfully locked the mutex.
return Some(MutexGuard { mutex: self });
return Some(MutexGuard { guard });
}

// Use atomic loads instead of compare-exchange.
while self.locked.load(Ordering::Relaxed) {
// Use is_locked instead of try_lock, as it is only a load as opposed to a swap.
while self.inner.is_locked() {
// Return None once we've exhausted the number of spins.
spins = spins.checked_sub(1)?;
}
Expand All @@ -186,28 +171,19 @@ impl<T> Mutex<T> {
}

struct MutexGuard<'a, T> {
mutex: &'a Mutex<T>,
}

impl<'a, T> Drop for MutexGuard<'a, T> {
fn drop(&mut self) {
self.mutex.locked.store(false, Ordering::Release);
}
guard: spin::MutexGuard<'a, T>,
Copy link
Member

Choose a reason for hiding this comment

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

wouldn't it make more sense to just do use spin::MutexGuard; here`

}

impl<'a, T> ops::Deref for MutexGuard<'a, T> {
type Target = T;

fn deref(&self) -> &T {
unsafe { &*self.mutex.value.get() }
&self.guard
}
}

impl<'a, T> ops::DerefMut for MutexGuard<'a, T> {
fn deref_mut(&mut self) -> &mut T {
unsafe { &mut *self.mutex.value.get() }
&mut self.guard
}
}

unsafe impl<T: Send> Send for Mutex<T> {}
unsafe impl<T: Send> Sync for Mutex<T> {}