Skip to content

Commit 24e86eb

Browse files
committed
Remove the modulo operations in spsc
These modulo operations used to be well optimized when N was a power of 2 However, the consumer and producer now use view-types that make N runtime dependant, preventing the compiler from optimizing these modulo operations even when N is always a power of 2. This patch leverages the fact that `head` and `tail` are always kept lower than N to replace the modulo operations with a simple if, which gets optimized pretty well by the compiler and no branch is left. Closes #650
1 parent be8628a commit 24e86eb

File tree

2 files changed

+88
-15
lines changed

2 files changed

+88
-15
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ of panicking drop implementations.
1212
- Added `from_bytes_truncating_at_nul` to `CString`
1313
- Added `CString::{into_bytes, into_bytes_with_nul, into_string}`
1414
- Added `pop_front_if` and `pop_back_if` to `Deque`
15+
- Fixed long division being instroduced by the const-erasure in spsc
1516

1617
## [v0.9.2] 2025-11-12
1718

src/spsc.rs

Lines changed: 87 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -129,12 +129,6 @@ pub struct QueueInner<T, S: Storage> {
129129
/// A statically allocated single-producer, single-consumer queue with a capacity of `N - 1`
130130
/// elements.
131131
///
132-
/// <div class="warning">
133-
///
134-
/// To get better performance, use a value for `N` that is a power of 2.
135-
///
136-
/// </div>
137-
///
138132
/// You will likely want to use [`split`](QueueInner::split) to create a producer-consumer pair.
139133
pub type Queue<T, const N: usize> = QueueInner<T, OwnedStorage<N>>;
140134

@@ -182,7 +176,12 @@ impl<T, S: Storage> QueueInner<T, S> {
182176

183177
#[inline]
184178
fn increment(&self, val: usize) -> usize {
185-
(val + 1) % self.n()
179+
let val = val + 1;
180+
if val >= self.n() {
181+
val - self.n()
182+
} else {
183+
val
184+
}
186185
}
187186

188187
#[inline]
@@ -202,10 +201,13 @@ impl<T, S: Storage> QueueInner<T, S> {
202201
let current_head = self.head.load(Ordering::Relaxed);
203202
let current_tail = self.tail.load(Ordering::Relaxed);
204203

205-
current_tail
206-
.wrapping_sub(current_head)
207-
.wrapping_add(self.n())
208-
% self.n()
204+
if current_tail >= current_head {
205+
current_tail - current_head
206+
} else {
207+
current_tail
208+
.wrapping_sub(current_head)
209+
.wrapping_add(self.n())
210+
}
209211
}
210212

211213
/// Returns whether the queue is empty.
@@ -626,7 +628,8 @@ impl<'a, T> Iterator for Iter<'a, T> {
626628
if self.index < self.len {
627629
let head = self.rb.head.load(Ordering::Relaxed);
628630

629-
let i = (head + self.index) % self.rb.n();
631+
let i = head + self.index;
632+
let i = if i >= self.rb.n() { i - self.rb.n() } else { i };
630633
self.index += 1;
631634

632635
Some(unsafe { &*(self.rb.buffer.borrow().get_unchecked(i).get() as *const T) })
@@ -643,7 +646,8 @@ impl<'a, T> Iterator for IterMut<'a, T> {
643646
if self.index < self.len {
644647
let head = self.rb.head.load(Ordering::Relaxed);
645648

646-
let i = (head + self.index) % self.rb.n();
649+
let i = head + self.index;
650+
let i = if i >= self.rb.n() { i - self.rb.n() } else { i };
647651
self.index += 1;
648652

649653
Some(unsafe { &mut *self.rb.buffer.borrow().get_unchecked(i).get().cast::<T>() })
@@ -659,7 +663,8 @@ impl<T> DoubleEndedIterator for Iter<'_, T> {
659663
let head = self.rb.head.load(Ordering::Relaxed);
660664

661665
// self.len > 0, since it's larger than self.index > 0
662-
let i = (head + self.len - 1) % self.rb.n();
666+
let i = head + self.len - 1;
667+
let i = if i >= self.rb.n() { i - self.rb.n() } else { i };
663668
self.len -= 1;
664669
Some(unsafe { &*(self.rb.buffer.borrow().get_unchecked(i).get() as *const T) })
665670
} else {
@@ -674,7 +679,8 @@ impl<T> DoubleEndedIterator for IterMut<'_, T> {
674679
let head = self.rb.head.load(Ordering::Relaxed);
675680

676681
// self.len > 0, since it's larger than self.index > 0
677-
let i = (head + self.len - 1) % self.rb.n();
682+
let i = head + self.len - 1;
683+
let i = if i >= self.rb.n() { i - self.rb.n() } else { i };
678684
self.len -= 1;
679685
Some(unsafe { &mut *self.rb.buffer.borrow().get_unchecked(i).get().cast::<T>() })
680686
} else {
@@ -1076,6 +1082,28 @@ mod tests {
10761082
assert_eq!(items.next(), None);
10771083
}
10781084

1085+
/// Exercise the modulo self.n() operation in next()
1086+
#[test]
1087+
fn iter_modulo() {
1088+
let mut rb: Queue<i32, 4> = Queue::new();
1089+
1090+
for _ in 0..2 {
1091+
rb.enqueue(0).unwrap();
1092+
rb.dequeue().unwrap();
1093+
}
1094+
rb.enqueue(1).unwrap();
1095+
rb.enqueue(2).unwrap();
1096+
rb.enqueue(3).unwrap();
1097+
1098+
let mut items = rb.iter();
1099+
1100+
// assert_eq!(items.next(), Some(&0));
1101+
assert_eq!(items.next(), Some(&1));
1102+
assert_eq!(items.next(), Some(&2));
1103+
assert_eq!(items.next(), Some(&3));
1104+
assert_eq!(items.next(), None);
1105+
}
1106+
10791107
#[test]
10801108
fn iter_double_ended() {
10811109
let mut rb: Queue<i32, 4> = Queue::new();
@@ -1093,6 +1121,28 @@ mod tests {
10931121
assert_eq!(items.next_back(), None);
10941122
}
10951123

1124+
/// Test that the modulo in next_back works as expected
1125+
#[test]
1126+
fn iter_double_ended_modulo() {
1127+
let mut rb: Queue<i32, 4> = Queue::new();
1128+
1129+
for _ in 0..2 {
1130+
rb.enqueue(0).unwrap();
1131+
rb.dequeue().unwrap();
1132+
}
1133+
rb.enqueue(0).unwrap();
1134+
rb.enqueue(1).unwrap();
1135+
rb.enqueue(2).unwrap();
1136+
1137+
let mut items = rb.iter();
1138+
1139+
assert_eq!(items.next(), Some(&0));
1140+
assert_eq!(items.next_back(), Some(&2));
1141+
assert_eq!(items.next(), Some(&1));
1142+
assert_eq!(items.next(), None);
1143+
assert_eq!(items.next_back(), None);
1144+
}
1145+
10961146
#[test]
10971147
fn iter_mut() {
10981148
let mut rb: Queue<i32, 4> = Queue::new();
@@ -1126,6 +1176,28 @@ mod tests {
11261176
assert_eq!(items.next_back(), None);
11271177
}
11281178

1179+
/// Test that the modulo in next_back works as expected
1180+
#[test]
1181+
fn iter_mut_double_ended_modulo() {
1182+
let mut rb: Queue<i32, 4> = Queue::new();
1183+
1184+
for _ in 0..2 {
1185+
rb.enqueue(0).unwrap();
1186+
rb.dequeue().unwrap();
1187+
}
1188+
rb.enqueue(0).unwrap();
1189+
rb.enqueue(1).unwrap();
1190+
rb.enqueue(2).unwrap();
1191+
1192+
let mut items = rb.iter_mut();
1193+
1194+
assert_eq!(items.next(), Some(&mut 0));
1195+
assert_eq!(items.next_back(), Some(&mut 2));
1196+
assert_eq!(items.next(), Some(&mut 1));
1197+
assert_eq!(items.next(), None);
1198+
assert_eq!(items.next_back(), None);
1199+
}
1200+
11291201
#[test]
11301202
fn wrap_around() {
11311203
let mut rb: Queue<i32, 4> = Queue::new();

0 commit comments

Comments
 (0)