perf: try eliding bounds checks when indexing (#51)

When indices are returned by the `Core::idx_gen` function, we can assume
that they are in bounds, since any bits that would make the index out of
bounds for the core's array size are masked out. Therefore, we can
safely use `slice::get_unchecked` to elide bounds checks.

In debug mode, we assert that the index is in bounds, just in case
there's a bug somewhere.

In conjunction with #50, this means MPSCs should never panic in release
mode.

Signed-off-by: Eliza Weisman <[email protected]>

src/lib.rs

@@ -174,14 +174,13 @@ impl Core {
     }
 
     #[inline(always)]
-    fn push_ref<'slots, T, S, R>(
+    fn push_ref<'slots, T, R>(
         &self,
-        slots: &'slots S,
+        slots: &'slots [Slot<T>],
         recycle: &R,
     ) -> Result<Ref<'slots, T>, TrySendError<()>>
     where
         R: Recycle<T>,
-        S: ops::Index<usize, Output = Slot<T>> + ?Sized,
     {
         test_println!("push_ref");
         let mut backoff = Backoff::new();
@@ -194,7 +193,23 @@ impl Core {
             let (idx, gen) = self.idx_gen(tail);
             test_dbg!(idx);
             test_dbg!(gen);
-            let slot = &slots[idx];
+            let slot = unsafe {
+                // Safety: `get_unchecked` does not check that the accessed
+                // index was within bounds. However, `idx` is produced by
+                // masking the current tail index to extract only the part
+                // that's within the array's bounds.
+                debug_assert!(
+                    idx < slots.len(),
+                    "index out of bounds (index was {} but the length was {})\n\n\
+                    /!\\ EXTREMELY SERIOUS WARNING /!\\: in release mode, this \
+                    access would not have been bounds checked, resulting in \
+                    undefined behavior!\nthis is a bug in `thingbuf`! please \
+                    report an issue immediately!",
+                    idx,
+                    slots.len()
+                );
+                slots.get_unchecked(idx)
+            };
             let state = test_dbg!(slot.state.load(Acquire));
 
             if test_dbg!(state == tail) {
@@ -266,10 +281,7 @@ impl Core {
     }
 
     #[inline(always)]
-    fn pop_ref<'slots, T, S>(&self, slots: &'slots S) -> Result<Ref<'slots, T>, TrySendError>
-    where
-        S: ops::Index<usize, Output = Slot<T>> + ?Sized,
-    {
+    fn pop_ref<'slots, T>(&self, slots: &'slots [Slot<T>]) -> Result<Ref<'slots, T>, TrySendError> {
         test_println!("pop_ref");
         let mut backoff = Backoff::new();
         let mut head = self.head.load(Relaxed);
@@ -279,7 +291,23 @@ impl Core {
             let (idx, gen) = self.idx_gen(head);
             test_dbg!(idx);
             test_dbg!(gen);
-            let slot = &slots[idx];
+            let slot = unsafe {
+                // Safety: `get_unchecked` does not check that the accessed
+                // index was within bounds. However, `idx` is produced by
+                // masking the current head index to extract only the part
+                // that's within the array's bounds.
+                debug_assert!(
+                    idx < slots.len(),
+                    "index out of bounds (index was {} but the length was {})\n\n\
+                    /!\\ EXTREMELY SERIOUS WARNING /!\\: in release mode, this \
+                    access would not have been bounds checked, resulting in \
+                    undefined behavior!\nthis is a bug in `thingbuf`! please \
+                    report an issue immediately!",
+                    idx,
+                    slots.len()
+                );
+                slots.get_unchecked(idx)
+            };
             let state = test_dbg!(slot.state.load(Acquire));
 
             // If the slot's state is ahead of the head index by one, we can pop

src/mpsc.rs

@@ -252,7 +252,7 @@ use crate::{
     wait::{Notify, WaitCell, WaitQueue, WaitResult},
     Core, Ref, Slot,
 };
-use core::{fmt, ops::Index, task::Poll};
+use core::{fmt, task::Poll};
 
 pub mod errors;
 use self::errors::TrySendError;
@@ -361,14 +361,11 @@ where
     /// The loop itself has to be written in the actual `send` method's
     /// implementation, rather than on `inner`, because it might be async and
     /// may yield, or might park the thread.
-    fn poll_recv_ref<'a, T, S>(
+    fn poll_recv_ref<'a, T>(
         &'a self,
-        slots: &'a S,
+        slots: &'a [Slot<T>],
         mk_waiter: impl Fn() -> N,
-    ) -> Poll<Option<Ref<'a, T>>>
-    where
-        S: Index<usize, Output = Slot<T>> + ?Sized,
-    {
+    ) -> Poll<Option<Ref<'a, T>>> {
         macro_rules! try_poll_recv {
             () => {
                 // If we got a value, return it!