Skip to content

Commit

Permalink
fix: address more comments
Browse files Browse the repository at this point in the history
  • Loading branch information
rymnc committed Jan 11, 2025
1 parent 6f30b26 commit 51d079c
Showing 1 changed file with 18 additions and 4 deletions.
22 changes: 18 additions & 4 deletions crates/services/src/seqlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::{
cell::UnsafeCell,
panic::UnwindSafe,
sync::atomic::{
fence,
AtomicU64,
Ordering,
},
Expand Down Expand Up @@ -37,15 +38,20 @@ impl<T: Copy> SeqLockWriter<T> {
let lock = &self.lock;

// Indicate that a write operation is starting.
lock.sequence.fetch_add(1, Ordering::Release);
lock.sequence.fetch_add(1, Ordering::AcqRel);
// reordering safety
fence(Ordering::Acquire);

// attempt to perform the write, and catch any panics
// we won't have partial write problems since data <= 64 bytes
// safety: panics are caught and resumed
let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| unsafe {
let data = &mut *lock.data.get();
f(data);
}));

// reordering safety
fence(Ordering::Release);
// Indicate that the write operation has finished.
lock.sequence.fetch_add(1, Ordering::Release);

Expand Down Expand Up @@ -74,11 +80,20 @@ impl<T: Copy> SeqLockReader<T> {

// if odd, write in progress
if start % 2 != 0 {
std::thread::yield_now();
continue;
}

// reordering safety
fence(Ordering::Acquire);

// safety: when the data <=64 bytes, it fits in a single cache line
// and cannot be subject to torn reads
let data = unsafe { *lock.data.get() };

// reordering safety
fence(Ordering::Acquire);

// check starting/ending guard
let end = lock.sequence.load(Ordering::Acquire);

Expand All @@ -95,9 +110,8 @@ impl<T: Copy> SeqLock<T> {
/// Optimized for occasional writes and frequent reads
/// !!WARNING!!
/// ONLY USE IF ALL THE BELOW CRITERIA ARE MET
/// 1. Internal data < 64 bytes
/// 2. ONLY 1 writer
/// 3. VERY frequent reads
/// 1. Internal data <= 64 bytes
/// 2. VERY frequent reads
/// # Safety
/// The data must be `Copy`
#[allow(clippy::new_ret_no_self)]
Expand Down

0 comments on commit 51d079c

Please sign in to comment.