Use more explecic cfg guard for domain timing

Author: TroyNeubauer

benches/folly.rs

@@ -48,7 +48,7 @@ folly_bench!(concurrent_new_holder, {
 });
 folly_bench!(concurrent_retire, {
     let foo = Box::into_raw(Box::new(HazPtrObjectWrapper::with_global_domain(0)));
-    black_box(unsafe { foo.retire(&deleters::drop_box) });
+    black_box(unsafe { HazPtrObjectWrapper::retire(foo, &deleters::drop_box) });
 });
 
 criterion_group!(benches, concurrent_new_holder, concurrent_retire);

src/domain.rs

@@ -5,9 +5,16 @@ use alloc::collections::BTreeSet;
 use core::marker::PhantomData;
 use core::sync::atomic::Ordering;
 
-#[cfg(all(feature = "std", not(miri)))]
+// Like folly's implementation, we use a time a based check to run reclamation about every
+// `SYNC_TIME_PERIOD` nanoseconds. The next time we should run reclamation is stored in
+// `due_time` inside `Domain`. On `no_std` we don't (yet) have access to time so this feature is
+// disabled. Also on platforms with < 64 bits, we can only store 2^32 nanoseconds -> ~4 seconds or
+// less, so this feature is also disabled. Additionally, miri can't support time because of
+// system calls (perhaps this can be mocked), and loom can't support time for reasons of determinism.
+
+#[cfg(all(feature = "std", target_pointer_width = "64", not(miri), not(loom)))]
 const SYNC_TIME_PERIOD: u64 = std::time::Duration::from_nanos(2000000000).as_nanos() as u64;
-#[cfg(all(feature = "std", not(miri)))]
+#[cfg(all(feature = "std", target_pointer_width = "64", not(miri), not(loom)))]
 use crate::sync::atomic::AtomicU64;
 
 #[cfg(loom)]
@@ -55,7 +62,7 @@ pub struct Domain<F> {
     hazptrs: HazPtrRecords,
     untagged: [RetiredList; NUM_SHARDS],
     family: PhantomData<F>,
-    #[cfg(all(feature = "std", not(miri)))]
+    #[cfg(all(feature = "std", target_pointer_width = "64", not(miri), not(loom)))]
     due_time: AtomicU64,
     nbulk_reclaims: AtomicUsize,
     count: AtomicIsize,
@@ -104,7 +111,7 @@ macro_rules! new {
                 },
                 untagged,
                 count: AtomicIsize::new(0),
-                #[cfg(all(feature = "std", not(miri)))]
+                #[cfg(all(feature = "std", target_pointer_width = "64", not(miri), not(loom)))]
                 due_time: AtomicU64::new(0),
                 nbulk_reclaims: AtomicUsize::new(0),
                 family: PhantomData,
@@ -361,7 +368,7 @@ impl<F> Domain<F> {
                 .compare_exchange_weak(rcount, 0, Ordering::AcqRel, Ordering::Relaxed)
             {
                 Ok(_) => {
-                    #[cfg(all(feature = "std", not(miri)))]
+                    #[cfg(all(feature = "std", target_pointer_width = "64", not(miri), not(loom)))]
                     {
                         self.due_time
                             .store(Self::now() + SYNC_TIME_PERIOD, Ordering::Release);
@@ -374,7 +381,7 @@ impl<F> Domain<F> {
         0
     }
 
-    #[cfg(all(feature = "std", not(miri)))]
+    #[cfg(all(feature = "std", target_pointer_width = "64", not(miri), not(loom)))]
     fn check_due_time(&self) -> isize {
         let time = Self::now();
         let due = self.due_time.load(Ordering::Acquire);
@@ -402,7 +409,7 @@ impl<F> Domain<F> {
             // TODO: Implement some kind of mock time for no_std.
             // Currently we reclaim only based on rcount on no_std
             // (also the reason for allow unused_mut)
-            #[cfg(all(feature = "std", not(miri)))]
+            #[cfg(all(feature = "std", target_pointer_width = "64", not(miri), not(loom)))]
             {
                 rcount = self.check_due_time();
             }
@@ -546,7 +553,7 @@ impl<F> Domain<F> {
         0
     }
 
-    #[cfg(all(not(loom), feature = "std"))]
+    #[cfg(all(feature = "std", target_pointer_width = "64", not(miri), not(loom)))]
     fn now() -> u64 {
         use std::convert::TryFrom;
         u64::try_from(

src/lib.rs

@@ -1,4 +1,3 @@
-//#![feature(arbitrary_self_types)]
 #![deny(unsafe_op_in_unsafe_fn)]
 #![allow(dead_code)]
 #![cfg_attr(not(feature = "std"), no_std)]

src/object.rs

@@ -20,6 +20,8 @@ where
     ///
     /// It is okay for existing readers to still refer to Self.
     ///   
+    //TODO: Once arbitrary_self_types is stabilized change `this` back to `self`.
+    //See: https://github.com/rust-lang/rust/issues/44874
     unsafe fn retire(this: *mut Self, deleter: &'static dyn Deleter) -> usize {
         let ptr = this as *mut (dyn Reclaim + 'domain);
         unsafe { (&*this).domain().retire(ptr, deleter) }

tests/folly.rs

@@ -65,7 +65,7 @@ fn basic_objects() {
         num += 1;
         let obj = Node::new(count, 0, std::ptr::null_mut());
         let x = Box::into_raw(Box::new(HazPtrObjectWrapper::with_domain(&domain, obj)));
-        unsafe { x.retire(&deleters::drop_box) };
+        unsafe { HazPtrObjectWrapper::retire(x, &deleters::drop_box) };
     }
     assert_eq!(num, count.ctors());
     domain.cleanup();
@@ -90,7 +90,7 @@ fn basic_protection() {
     let obj = Box::into_raw(Box::new(HazPtrObjectWrapper::with_domain(&domain, obj)));
     let mut h = HazardPointer::make_in_domain(&domain);
     unsafe { h.protect_raw(obj) };
-    unsafe { obj.retire(&deleters::drop_box) };
+    unsafe { HazPtrObjectWrapper::retire(obj, &deleters::drop_box) };
     assert_eq!(1, count.ctors());
     domain.cleanup();
     assert_eq!(0, count.dtors());
@@ -113,7 +113,7 @@ fn destruction() {
         fn drop(&mut self) {
             self.dtors.fetch_add(1, Ordering::AcqRel);
             if !self.next.is_null() {
-                unsafe { self.next.retire(&deleters::drop_box) };
+                unsafe { HazPtrObjectWrapper::retire(self.next, &deleters::drop_box) };
             }
         }
     }
@@ -125,7 +125,7 @@ fn destruction() {
             HeadRetireNext { next: last, dtors },
         )));
     }
-    unsafe { last.retire(&deleters::drop_box) };
+    unsafe { HazPtrObjectWrapper::retire(last, &deleters::drop_box) };
     domain.cleanup();
     assert_eq!(2000, dtors.load(Ordering::SeqCst));
 }
@@ -141,7 +141,7 @@ fn move_test() {
         let x = Box::into_raw(Box::new(HazPtrObjectWrapper::with_domain(&domain, obj)));
         let mut hptr0 = HazardPointer::make_in_domain(&domain);
         unsafe { hptr0.protect_raw(x) };
-        unsafe { x.retire(&deleters::drop_box) };
+        unsafe { HazPtrObjectWrapper::retire(x, &deleters::drop_box) };
         let hptr1 = hptr0;
         let hptr1 = hptr1;
         let mut hptr2;
@@ -163,7 +163,7 @@ fn array() {
         let mut hptr = HazardPointer::make_many_in_domain::<3>(&domain);
         let hptr = hptr.hazard_pointers();
         unsafe { hptr[2].protect_raw(x) };
-        unsafe { x.retire(&deleters::drop_box) };
+        unsafe { HazPtrObjectWrapper::retire(x, &deleters::drop_box) };
         // x is still protected from the call to protect_raw earlier.
         assert_eq!(unsafe { &*x }.val, i);
         hptr[2].reset_protection();
@@ -175,13 +175,13 @@ fn array() {
 fn free_function_retire() {
     let domain = unique_domain!();
     let foo = Box::into_raw(Box::new(HazPtrObjectWrapper::with_domain(&domain, 0)));
-    unsafe { foo.retire(&deleters::drop_box) };
+    unsafe { HazPtrObjectWrapper::retire(foo, &deleters::drop_box) };
     let foo2 = Box::into_raw(Box::new(HazPtrObjectWrapper::with_domain(&domain, 0)));
     unsafe fn _custom_drop(ptr: *mut (dyn Reclaim + 'static)) {
         let _ = Box::from_raw(ptr);
     }
     const CUSTOM_DROP: unsafe fn(*mut dyn Reclaim) = _custom_drop;
-    unsafe { foo2.retire(&CUSTOM_DROP) };
+    unsafe { HazPtrObjectWrapper::retire(foo2, &CUSTOM_DROP) };
 
     // Third test just checks that dtor is called, which is already covered by other tests.
     // It is _not_ using a custom deleter/retirer.
@@ -205,8 +205,8 @@ fn array_part_of_cleanup() {
         let h = h.hazard_pointers();
         unsafe { h[0].protect_raw(p0) };
         unsafe { h[1].protect_raw(p1) };
-        unsafe { p0.retire(&deleters::drop_box) };
-        unsafe { p1.retire(&deleters::drop_box) };
+        unsafe { HazPtrObjectWrapper::retire(p0, &deleters::drop_box) };
+        unsafe { HazPtrObjectWrapper::retire(p1, &deleters::drop_box) };
     }
     assert_eq!(2, count.ctors());
     domain.cleanup();

tests/loom.rs

@@ -288,7 +288,7 @@ fn folly_cleanup() {
                     for j in (tid..THREAD_OPS).step_by(NUM_THREADS) {
                         let obj = Node::new(count, j, std::ptr::null_mut());
                         let p = Box::into_raw(Box::new(HazPtrObjectWrapper::with_domain(&D, obj)));
-                        unsafe { p.retire(&deleters::drop_box) };
+                        unsafe { HazPtrObjectWrapper::retire(p, &deleters::drop_box) };
                     }
                     threads_done.fetch_add(1, Ordering::AcqRel);
                     let _ = main_done.lock();
@@ -299,7 +299,7 @@ fn folly_cleanup() {
             for i in 0..MAIN_OPS {
                 let obj = Node::new(count, i, std::ptr::null_mut());
                 let p = Box::into_raw(Box::new(HazPtrObjectWrapper::with_domain(&D, obj)));
-                unsafe { p.retire(&deleters::drop_box) };
+                unsafe { HazPtrObjectWrapper::retire(p, &deleters::drop_box) };
             }
         }
         while threads_done.load(Ordering::Acquire) < NUM_THREADS {