Use odd/even instead of high bit

It makes the code so much nicer.
A nod to #76 is also worthwhile here.

Author: jonhoo

left-right/src/lib.rs

@@ -172,7 +172,7 @@
 
 use std::sync::{atomic, Arc, Mutex};
 
-type Epochs = Arc<Mutex<slab::Slab<Arc<atomic::AtomicU64>>>>;
+type Epochs = Arc<Mutex<slab::Slab<Arc<atomic::AtomicUsize>>>>;
 
 mod write;
 pub use crate::write::WriteHandle;

left-right/src/read.rs

@@ -1,9 +1,9 @@
 use std::cell::Cell;
+use std::fmt;
 use std::marker::PhantomData;
 use std::sync::atomic;
 use std::sync::atomic::AtomicPtr;
 use std::sync::{self, Arc};
-use std::{fmt, mem};
 
 // To make [`WriteHandle`] and friends work.
 #[cfg(doc)]
@@ -40,9 +40,8 @@ pub use factory::ReadHandleFactory;
 pub struct ReadHandle<T> {
     pub(crate) inner: sync::Arc<AtomicPtr<T>>,
     pub(crate) epochs: crate::Epochs,
-    epoch: sync::Arc<sync::atomic::AtomicU64>,
+    epoch: sync::Arc<sync::atomic::AtomicUsize>,
     epoch_i: usize,
-    my_epoch: Cell<u64>,
     enters: Cell<usize>,
 
     // `ReadHandle` is _only_ Send if T is Sync. If T is !Sync, then it's not okay for us to expose
@@ -68,7 +67,6 @@ impl<T> fmt::Debug for ReadHandle<T> {
         f.debug_struct("ReadHandle")
             .field("epochs", &self.epochs)
             .field("epoch", &self.epoch)
-            .field("my_epoch", &self.my_epoch)
             .finish()
     }
 }
@@ -91,15 +89,14 @@ impl<T> ReadHandle<T> {
 
     fn new_with_arc(inner: Arc<AtomicPtr<T>>, epochs: crate::Epochs) -> Self {
         // tell writer about our epoch tracker
-        let epoch = sync::Arc::new(atomic::AtomicU64::new(0));
+        let epoch = sync::Arc::new(atomic::AtomicUsize::new(0));
         // okay to lock, since we're not holding up the epoch
         let epoch_i = epochs.lock().unwrap().insert(Arc::clone(&epoch));
 
         Self {
             epochs,
             epoch,
             epoch_i,
-            my_epoch: Cell::new(0),
             enters: Cell::new(0),
             inner,
             _unimpl_send: PhantomData,
@@ -128,7 +125,6 @@ impl<T> ReadHandle<T> {
                 self.enters.set(enters + 1);
                 Some(ReadGuard {
                     handle: guard::ReadHandleState::from(self),
-                    epoch: self.my_epoch.get(),
                     t: r_handle,
                 })
             } else {
@@ -164,9 +160,7 @@ impl<T> ReadHandle<T> {
         // in all cases, using a pointer we read *after* updating our epoch is safe.
 
         // so, update our epoch tracker.
-        let epoch = self.my_epoch.get() + 1;
-        self.my_epoch.set(epoch);
-        self.epoch.store(epoch, atomic::Ordering::Release);
+        self.epoch.fetch_add(1, atomic::Ordering::AcqRel);
 
         // ensure that the pointer read happens strictly after updating the epoch
         atomic::fence(atomic::Ordering::SeqCst);
@@ -183,16 +177,12 @@ impl<T> ReadHandle<T> {
             self.enters.set(enters);
             Some(ReadGuard {
                 handle: guard::ReadHandleState::from(self),
-                epoch,
                 t: r_handle,
             })
         } else {
             // the writehandle has been dropped, and so has both copies,
             // so restore parity and return None
-            self.epoch.store(
-                epoch | 1 << (mem::size_of_val(&self.my_epoch) * 8 - 1),
-                atomic::Ordering::Release,
-            );
+            self.epoch.fetch_add(1, atomic::Ordering::AcqRel);
             None
         }
     }

left-right/src/read/guard.rs

@@ -5,16 +5,14 @@ use std::sync::atomic;
 
 #[derive(Debug, Copy, Clone)]
 pub(super) struct ReadHandleState<'rh> {
-    pub(super) shared_epoch: &'rh sync::atomic::AtomicU64,
-    pub(super) own_epoch: &'rh Cell<u64>,
+    pub(super) epoch: &'rh sync::atomic::AtomicUsize,
     pub(super) enters: &'rh Cell<usize>,
 }
 
 impl<'rh, T> From<&'rh super::ReadHandle<T>> for ReadHandleState<'rh> {
     fn from(rh: &'rh super::ReadHandle<T>) -> Self {
         Self {
-            shared_epoch: &rh.epoch,
-            own_epoch: &rh.my_epoch,
+            epoch: &rh.epoch,
             enters: &rh.enters,
         }
     }
@@ -33,7 +31,6 @@ pub struct ReadGuard<'rh, T: ?Sized> {
     // NOTE: _technically_ this is more like &'self.
     // the reference is valid until the guard is dropped.
     pub(super) t: &'rh T,
-    pub(super) epoch: u64,
     pub(super) handle: ReadHandleState<'rh>,
 }
 
@@ -63,7 +60,6 @@ impl<'rh, T: ?Sized> ReadGuard<'rh, T> {
     {
         let rg = ReadGuard {
             t: f(orig.t),
-            epoch: orig.epoch,
             handle: orig.handle,
         };
         mem::forget(orig);
@@ -99,7 +95,6 @@ impl<'rh, T: ?Sized> ReadGuard<'rh, T> {
     {
         let rg = ReadGuard {
             t: f(orig.t)?,
-            epoch: orig.epoch,
             handle: orig.handle,
         };
         mem::forget(orig);
@@ -126,11 +121,7 @@ impl<'rh, T: ?Sized> Drop for ReadGuard<'rh, T> {
         self.handle.enters.set(enters);
         if enters == 0 {
             // We are the last guard to be dropped -- now release our epoch.
-            let epoch = self.handle.own_epoch.get();
-            self.handle.shared_epoch.store(
-                epoch | 1 << (mem::size_of_val(&epoch) * 8 - 1),
-                atomic::Ordering::Release,
-            );
+            self.handle.epoch.fetch_add(1, atomic::Ordering::AcqRel);
         }
     }
 }

left-right/src/write.rs

@@ -4,7 +4,7 @@ use crate::read::ReadHandle;
 use std::ptr::NonNull;
 use std::sync::atomic;
 use std::sync::{Arc, MutexGuard};
-use std::{fmt, mem, thread};
+use std::{fmt, thread};
 
 /// A writer handle to a left-right guarded data structure.
 ///
@@ -27,7 +27,7 @@ where
     oplog: Vec<O>,
     swap_index: usize,
     r_handle: ReadHandle<T>,
-    last_epochs: Vec<u64>,
+    last_epochs: Vec<usize>,
     #[cfg(test)]
     refreshes: usize,
 }
@@ -125,34 +125,36 @@ where
         }
     }
 
-    fn wait(&mut self, epochs: &mut MutexGuard<'_, slab::Slab<Arc<atomic::AtomicU64>>>) {
+    fn wait(&mut self, epochs: &mut MutexGuard<'_, slab::Slab<Arc<atomic::AtomicUsize>>>) {
         let mut iter = 0;
         let mut starti = 0;
 
-        // We want a 1 in the type of the epochs, without naming that type here.
-        #[allow(unused_assignments)]
-        let mut epoch_unit = self.last_epochs.get(0).copied().unwrap_or(0);
-        epoch_unit = 1;
-        let high_bit = epoch_unit << (mem::size_of_val(&epoch_unit) * 8 - 1);
-
         // we're over-estimating here, but slab doesn't expose its max index
         self.last_epochs.resize(epochs.capacity(), 0);
         'retry: loop {
             // read all and see if all have changed (which is likely)
             for (ii, (ri, epoch)) in epochs.iter().enumerate().skip(starti) {
-                // note that `ri` _may_ have been re-used since we last read into last_epochs.
+                // if the reader's epoch was even last we read it (which was _after_ the swap),
+                // then they either do not have the pointer, or must have read the pointer strictly
+                // after the swap. in either case, they cannot be using the old pointer value (what
+                // is now w_handle).
+                //
+                // note that this holds even with wrap-around since std::u{N}::MAX == 2 ^ N - 1,
+                // which is odd, and std::u{N}::MAX + 1 == 0 is even.
+                //
+                // note also that `ri` _may_ have been re-used since we last read into last_epochs.
                 // this is okay though, as a change still implies that the new reader must have
                 // arrived _after_ we did the atomic swap, and thus must also have seen the new
                 // pointer.
-                if self.last_epochs[ri] & high_bit != 0 {
-                    // reader was not active right after last swap
-                    // and therefore *must* only see new pointer
+                if self.last_epochs[ri] % 2 == 0 {
                     continue;
                 }
 
                 let now = epoch.load(atomic::Ordering::Acquire);
-                if (now != self.last_epochs[ri]) | (now & high_bit != 0) | (now == 0) {
-                    // reader must have seen last swap
+                if now != self.last_epochs[ri] {
+                    // reader must have seen the last swap, since they have done at least one
+                    // operation since we last looked at their epoch, which _must_ mean that they
+                    // are no longer using the old pointer value.
                 } else {
                     // reader may not have seen swap
                     // continue from this reader's epoch