[#139] Improve bitset code

Author: elfenpiff

iceoryx2-bb/lock-free/src/mpmc/bit_set.rs

@@ -66,6 +66,20 @@ pub mod details {
     pub type BitsetElement = AtomicU8;
     const BITSET_ELEMENT_BITSIZE: usize = core::mem::size_of::<BitsetElement>() * 8;
 
+    struct Id {
+        index: usize,
+        bit: u8,
+    }
+
+    impl Id {
+        fn new(value: usize) -> Id {
+            Self {
+                index: value / BITSET_ELEMENT_BITSIZE,
+                bit: (value % BITSET_ELEMENT_BITSIZE) as u8,
+            }
+        }
+    }
+
     #[derive(Debug)]
     #[repr(C)]
     pub struct BitSet<PointerType: PointerTrait<BitsetElement>> {
@@ -191,20 +205,17 @@ pub mod details {
             );
         }
 
-        fn set_bit(&self, id: usize) -> bool {
-            let index = id / BITSET_ELEMENT_BITSIZE;
-            let bit = id % BITSET_ELEMENT_BITSIZE;
-
-            let data_ref = unsafe { &(*self.data_ptr.as_ptr().add(index)) };
+        fn set_bit(&self, id: Id) -> bool {
+            let data_ref = unsafe { &(*self.data_ptr.as_ptr().add(id.index)) };
             let mut current = data_ref.load(Ordering::Relaxed);
-            let bit = 1 << bit;
+            let mask = 1 << id.bit;
 
             loop {
-                if current & bit != 0 {
+                if current & mask != 0 {
                     return false;
                 }
 
-                let current_with_bit = current | bit;
+                let current_with_bit = current | mask;
 
                 match data_ref.compare_exchange(
                     current,
@@ -221,24 +232,21 @@ pub mod details {
             }
         }
 
-        fn clear_bit(&self, id: usize) -> bool {
-            let index = id / BITSET_ELEMENT_BITSIZE;
-            let bit = id % BITSET_ELEMENT_BITSIZE;
-
-            let data_ref = unsafe { &(*self.data_ptr.as_ptr().add(index)) };
+        fn clear_bit(&self, id: Id) -> bool {
+            let data_ref = unsafe { &(*self.data_ptr.as_ptr().add(id.index)) };
             let mut current = data_ref.load(Ordering::Relaxed);
-            let bit = 1 << bit;
+            let mask = 1 << id.bit;
 
             loop {
-                if current & bit == 0 {
+                if current & mask == 0 {
                     return false;
                 }
 
-                let current_with_bit = current & !bit;
+                let current_with_cleared_bit = current & !mask;
 
                 match data_ref.compare_exchange(
                     current,
-                    current_with_bit,
+                    current_with_cleared_bit,
                     Ordering::Relaxed,
                     Ordering::Relaxed,
                 ) {
@@ -262,7 +270,7 @@ pub mod details {
                 id
             );
 
-            self.set_bit(id)
+            self.set_bit(Id::new(id))
         }
 
         /// Resets the next set bit and returns the bit index. If no bit was set it returns
@@ -273,14 +281,11 @@ pub mod details {
                 return None;
             }
 
-            let mut current_position = self.reset_position.load(Ordering::Relaxed);
-            for _ in 0..self.capacity {
-                current_position = (current_position + 1) % self.capacity;
-
-                if self.clear_bit(current_position) {
-                    self.reset_position
-                        .store(current_position, Ordering::Relaxed);
-                    return Some(current_position);
+            let current_position = self.reset_position.load(Ordering::Relaxed);
+            for pos in (current_position..self.capacity).chain(0..current_position) {
+                if self.clear_bit(Id::new(pos)) {
+                    self.reset_position.store(pos + 1, Ordering::Relaxed);
+                    return Some(pos);
                 }
             }
 
@@ -295,15 +300,15 @@ pub mod details {
             for i in 0..self.array_capacity {
                 let value = unsafe { (*self.data_ptr.as_ptr().add(i)).swap(0, Ordering::Relaxed) };
                 let mut counter = 0;
+                let main_index = i * BITSET_ELEMENT_BITSIZE;
                 for b in 0..BITSET_ELEMENT_BITSIZE {
-                    let main_index = i * BITSET_ELEMENT_BITSIZE;
                     if value & (1 << b) != 0 {
                         callback(main_index + b);
                         counter += 1;
                     }
                 }
 
-                if self.active_bits.fetch_sub(counter, Ordering::Relaxed) == 1 {
+                if self.active_bits.fetch_sub(counter, Ordering::Relaxed) == counter {
                     return;
                 }
             }

iceoryx2-cal/src/event/common.rs

@@ -15,6 +15,7 @@ pub mod details {
     use std::{fmt::Debug, marker::PhantomData, time::Duration};
 
     use iceoryx2_bb_log::fail;
+    use iceoryx2_bb_memory::bump_allocator::BumpAllocator;
     use iceoryx2_bb_system_types::{file_name::FileName, path::Path};
 
     use crate::{
@@ -260,7 +261,8 @@ pub mod details {
             }
 
             unsafe { self.storage.get().id_tracker.add(id)? };
-            Ok(unsafe { self.storage.get().signal_mechanism.notify()? })
+            unsafe { self.storage.get().signal_mechanism.notify()? };
+            Ok(())
         }
     }
 
@@ -447,6 +449,27 @@ pub mod details {
         }
     }
 
+    impl<
+            Tracker: IdTracker,
+            WaitMechanism: SignalMechanism,
+            Storage: DynamicStorage<Management<Tracker, WaitMechanism>>,
+        > ListenerBuilder<Tracker, WaitMechanism, Storage>
+    {
+        fn init(
+            mgmt: &mut Management<Tracker, WaitMechanism>,
+            allocator: &mut BumpAllocator,
+        ) -> bool {
+            if unsafe { mgmt.id_tracker.init(allocator).is_err() } {
+                return false;
+            }
+            if unsafe { mgmt.signal_mechanism.init().is_err() } {
+                return false;
+            }
+
+            true
+        }
+    }
+
     impl<
             Tracker: IdTracker,
             WaitMechanism: SignalMechanism,
@@ -471,16 +494,7 @@ pub mod details {
             match Storage::Builder::new(&self.name)
                 .config(&self.config.convert())
                 .supplementary_size(Tracker::memory_size(id_tracker_capacity))
-                .initializer(|mgmt, allocator| {
-                    if unsafe { mgmt.id_tracker.init(allocator).is_err() } {
-                        return false;
-                    }
-                    if unsafe { mgmt.signal_mechanism.init().is_err() } {
-                        return false;
-                    }
-
-                    true
-                })
+                .initializer(Self::init)
                 .has_ownership(true)
                 .create(Management {
                     id_tracker: unsafe { Tracker::new_uninit(id_tracker_capacity) },

iceoryx2-cal/src/event/id_tracker/bit_set.rs

@@ -37,9 +37,6 @@ impl IdTracker for RelocatableBitSet {
     }
 
     unsafe fn acquire(&self) -> Option<TriggerId> {
-        match self.reset_next() {
-            Some(id) => Some(TriggerId::new(id as u64)),
-            None => None,
-        }
+        self.reset_next().map(|id| TriggerId::new(id as u64))
     }
 }