Skip to content

Commit 829591c

Browse files
userFRMclaude
andcommitted
refactor!: SubscriberGroup wraps Subscriber, drop redundant Publisher::sequence
SubscriberGroup carried a byte-for-byte copy of Subscriber's fields and of try_recv, recv_with, recv_batch, pending, the counter getters, update_tracker, and Drop. All N logical subscribers already share one cursor, so the type is behaviourally a Subscriber plus a compile-time count. It becomes a newtype over Subscriber that delegates, keeping aligned_count() as the only group-specific method. Two incidental improvements fall out: the group now uses Subscriber::recv's two-phase spin (64 bare iterations before the power-efficient wait) instead of its own single-phase loop, and Send is auto-derived rather than an unsafe impl. Publisher::sequence returned self.seq exactly as published() does. Removed, with the lag-computation note folded into published(); its test duplicated published_count and is removed with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent beb3f14 commit 829591c

4 files changed

Lines changed: 20 additions & 215 deletions

File tree

src/channel/group.rs

Lines changed: 16 additions & 181 deletions
Original file line numberDiff line numberDiff line change
@@ -2,44 +2,26 @@
22
// SPDX-License-Identifier: MIT OR Apache-2.0
33

44
use super::errors::TryRecvError;
5+
use super::subscriber::Subscriber;
56
use crate::pod::Pod;
6-
use crate::ring::{Padded, RingIndex, SharedRing};
7-
use crate::slot::Slot;
87
use crate::wait::WaitStrategy;
9-
use alloc::sync::Arc;
10-
use core::sync::atomic::{AtomicU64, Ordering};
118

129
/// A group of `N` logical subscribers backed by a single ring read.
1310
///
1411
/// All `N` logical subscribers share one cursor —
1512
/// [`try_recv`](SubscriberGroup::try_recv) performs **one** seqlock read
1613
/// and a single cursor increment, eliminating the N-element sweep loop.
14+
/// That makes the group behaviourally a single [`Subscriber`] carrying a
15+
/// compile-time count, which is exactly how it is implemented; `N` is
16+
/// reported by [`aligned_count`](SubscriberGroup::aligned_count).
1717
///
1818
/// ```
1919
/// let (mut p, subs) = photon_ring::channel::<u64>(64);
2020
/// let mut group = subs.subscribe_group::<4>();
2121
/// p.publish(42);
2222
/// assert_eq!(group.try_recv(), Ok(42));
2323
/// ```
24-
pub struct SubscriberGroup<T: Pod, const N: usize> {
25-
pub(super) ring: Arc<SharedRing<T>>,
26-
/// Cached raw pointer to the slot array. Avoids Arc + Box deref on the
27-
/// hot path. Valid for the lifetime of `ring` (the Arc keeps it alive).
28-
pub(super) slots_ptr: *const Slot<T>,
29-
/// Precomputed slot indexing (capacity, mask, reciprocal, pow2 flag).
30-
pub(super) index: RingIndex,
31-
/// Single cursor shared by all `N` logical subscribers.
32-
pub(super) cursor: u64,
33-
/// Cumulative messages skipped due to lag.
34-
pub(super) total_lagged: u64,
35-
/// Cumulative messages successfully received.
36-
pub(super) total_received: u64,
37-
/// Per-group cursor tracker for backpressure. `None` on regular
38-
/// (lossy) channels — zero overhead.
39-
pub(super) tracker: Option<Arc<Padded<AtomicU64>>>,
40-
}
41-
42-
unsafe impl<T: Pod, const N: usize> Send for SubscriberGroup<T, N> {}
24+
pub struct SubscriberGroup<T: Pod, const N: usize>(pub(super) Subscriber<T>);
4325

4426
impl<T: Pod, const N: usize> SubscriberGroup<T, N> {
4527
/// Try to receive the next message for the group.
@@ -48,74 +30,21 @@ impl<T: Pod, const N: usize> SubscriberGroup<T, N> {
4830
/// N-element sweep needed since all logical subscribers share one cursor.
4931
#[inline]
5032
pub fn try_recv(&mut self) -> Result<T, TryRecvError> {
51-
let cur = self.cursor;
52-
// SAFETY: slots_ptr is valid for the lifetime of self.ring (Arc-owned).
53-
let slot = unsafe { &*self.slots_ptr.add(self.index.slot(cur)) };
54-
let expected = cur * 2 + 2;
55-
56-
match slot.try_read(cur) {
57-
Ok(Some(value)) => {
58-
self.cursor = cur + 1;
59-
self.total_received += 1;
60-
self.update_tracker();
61-
Ok(value)
62-
}
63-
Ok(None) => Err(TryRecvError::Empty),
64-
Err(actual_stamp) => {
65-
if actual_stamp & 1 != 0 || actual_stamp < expected {
66-
return Err(TryRecvError::Empty);
67-
}
68-
// Lagged — recompute from head cursor
69-
let head = self.ring.cursor.0.load(Ordering::Acquire);
70-
let cap = self.ring.capacity();
71-
if head == u64::MAX || cur > head {
72-
return Err(TryRecvError::Empty);
73-
}
74-
if head >= cap {
75-
let oldest = head - cap + 1;
76-
if cur < oldest {
77-
let skipped = oldest - cur;
78-
self.cursor = oldest;
79-
self.total_lagged += skipped;
80-
self.update_tracker();
81-
return Err(TryRecvError::Lagged { skipped });
82-
}
83-
}
84-
Err(TryRecvError::Empty)
85-
}
86-
}
33+
self.0.try_recv()
8734
}
8835

8936
/// Spin until the next message is available.
9037
///
91-
/// On aarch64: uses SEVL + WFE for near-zero-power cache-line-event
92-
/// wakeup. On x86: uses PAUSE (spin_loop hint).
38+
/// Uses the same two-phase spin as [`Subscriber::recv`]: bare spin for
39+
/// the first 64 iterations, then a power-efficient wait (`PAUSE` on x86,
40+
/// `SEVL`/`WFE` on aarch64).
9341
#[inline]
9442
pub fn recv(&mut self) -> T {
95-
#[cfg(target_arch = "aarch64")]
96-
unsafe {
97-
core::arch::asm!("sevl", options(nomem, nostack));
98-
}
99-
loop {
100-
match self.try_recv() {
101-
Ok(val) => return val,
102-
Err(TryRecvError::Empty) => {
103-
#[cfg(target_arch = "aarch64")]
104-
unsafe {
105-
core::arch::asm!("wfe", options(nomem, nostack));
106-
}
107-
#[cfg(not(target_arch = "aarch64"))]
108-
core::hint::spin_loop();
109-
}
110-
Err(TryRecvError::Lagged { .. }) => {}
111-
}
112-
}
43+
self.0.recv()
11344
}
11445

11546
/// Block until the next message using the given [`WaitStrategy`].
11647
///
117-
/// Like [`Subscriber::recv_with`], but for the grouped fast path.
118-
///
11948
/// # Example
12049
/// ```
12150
/// use photon_ring::{channel, WaitStrategy};
@@ -127,49 +56,7 @@ impl<T: Pod, const N: usize> SubscriberGroup<T, N> {
12756
/// ```
12857
#[inline]
12958
pub fn recv_with(&mut self, strategy: WaitStrategy) -> T {
130-
let cur = self.cursor;
131-
let slot = unsafe { &*self.slots_ptr.add(self.index.slot(cur)) };
132-
let expected = cur * 2 + 2;
133-
let mut iter: u32 = 0;
134-
loop {
135-
match slot.try_read(cur) {
136-
Ok(Some(value)) => {
137-
self.cursor = cur + 1;
138-
self.total_received += 1;
139-
self.update_tracker();
140-
return value;
141-
}
142-
Ok(None) => {
143-
strategy.wait(iter);
144-
iter = iter.saturating_add(1);
145-
}
146-
Err(stamp) => {
147-
if stamp >= expected {
148-
return self.recv_with_slow(strategy);
149-
}
150-
strategy.wait(iter);
151-
iter = iter.saturating_add(1);
152-
}
153-
}
154-
}
155-
}
156-
157-
#[cold]
158-
#[inline(never)]
159-
fn recv_with_slow(&mut self, strategy: WaitStrategy) -> T {
160-
let mut iter: u32 = 0;
161-
loop {
162-
match self.try_recv() {
163-
Ok(val) => return val,
164-
Err(TryRecvError::Empty) => {
165-
strategy.wait(iter);
166-
iter = iter.saturating_add(1);
167-
}
168-
Err(TryRecvError::Lagged { .. }) => {
169-
iter = 0;
170-
}
171-
}
172-
}
59+
self.0.recv_with(strategy)
17360
}
17461

17562
/// How many of the `N` logical subscribers are aligned.
@@ -184,37 +71,26 @@ impl<T: Pod, const N: usize> SubscriberGroup<T, N> {
18471
/// Number of messages available to read (capped at ring capacity).
18572
#[inline]
18673
pub fn pending(&self) -> u64 {
187-
let head = self.ring.cursor.0.load(Ordering::Acquire);
188-
if head == u64::MAX || self.cursor > head {
189-
0
190-
} else {
191-
let raw = head - self.cursor + 1;
192-
raw.min(self.ring.capacity())
193-
}
74+
self.0.pending()
19475
}
19576

19677
/// Total messages successfully received by this group.
19778
#[inline]
19879
pub fn total_received(&self) -> u64 {
199-
self.total_received
80+
self.0.total_received()
20081
}
20182

20283
/// Total messages lost due to lag (group fell behind the ring).
20384
#[inline]
20485
pub fn total_lagged(&self) -> u64 {
205-
self.total_lagged
86+
self.0.total_lagged()
20687
}
20788

20889
/// Ratio of received to total (received + lagged). Returns 0.0 if no
20990
/// messages have been processed.
21091
#[inline]
21192
pub fn receive_ratio(&self) -> f64 {
212-
let total = self.total_received + self.total_lagged;
213-
if total == 0 {
214-
0.0
215-
} else {
216-
self.total_received as f64 / total as f64
217-
}
93+
self.0.receive_ratio()
21894
}
21995

22096
/// Receive up to `buf.len()` messages in a single call.
@@ -224,47 +100,6 @@ impl<T: Pod, const N: usize> SubscriberGroup<T, N> {
224100
/// advanced and filling continues from the oldest available message.
225101
#[inline]
226102
pub fn recv_batch(&mut self, buf: &mut [T]) -> usize {
227-
let mut count = 0;
228-
for slot in buf.iter_mut() {
229-
match self.try_recv() {
230-
Ok(value) => {
231-
*slot = value;
232-
count += 1;
233-
}
234-
Err(TryRecvError::Empty) => break,
235-
Err(TryRecvError::Lagged { .. }) => {
236-
// Cursor was advanced — retry from oldest available.
237-
match self.try_recv() {
238-
Ok(value) => {
239-
*slot = value;
240-
count += 1;
241-
}
242-
Err(_) => break,
243-
}
244-
}
245-
}
246-
}
247-
count
248-
}
249-
250-
/// Update the backpressure tracker to reflect the current cursor position.
251-
/// No-op on regular (lossy) channels.
252-
#[inline]
253-
fn update_tracker(&self) {
254-
if let Some(ref tracker) = self.tracker {
255-
tracker.0.store(self.cursor, Ordering::Relaxed);
256-
}
257-
}
258-
}
259-
260-
impl<T: Pod, const N: usize> Drop for SubscriberGroup<T, N> {
261-
fn drop(&mut self) {
262-
if let Some(ref tracker) = self.tracker {
263-
if let Some(ref bp) = self.ring.backpressure {
264-
let weak = Arc::downgrade(tracker);
265-
let mut trackers = bp.trackers.lock();
266-
trackers.retain(|t| !t.ptr_eq(&weak));
267-
}
268-
}
103+
self.0.recv_batch(buf)
269104
}
270105
}

src/channel/publisher.rs

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -196,19 +196,14 @@ impl<T: Pod> Publisher<T> {
196196
}
197197
}
198198

199-
/// Number of messages published so far.
199+
/// Number of messages published so far, which is also the next sequence
200+
/// number to be written. Useful for computing lag:
201+
/// `publisher.published() - subscriber.cursor`.
200202
#[inline]
201203
pub fn published(&self) -> u64 {
202204
self.seq
203205
}
204206

205-
/// Current sequence number (same as `published()`).
206-
/// Useful for computing lag: `publisher.sequence() - subscriber.cursor`.
207-
#[inline]
208-
pub fn sequence(&self) -> u64 {
209-
self.seq
210-
}
211-
212207
/// Ring capacity.
213208
#[inline]
214209
pub fn capacity(&self) -> u64 {

src/channel/subscribable.rs

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -58,20 +58,7 @@ impl<T: Pod> Subscribable<T> {
5858
/// Panics if `N` is 0.
5959
pub fn subscribe_group<const N: usize>(&self) -> SubscriberGroup<T, N> {
6060
assert!(N > 0, "SubscriberGroup requires at least 1 subscriber");
61-
let head = self.ring.cursor.0.load(Ordering::Acquire);
62-
let start = if head == u64::MAX { 0 } else { head + 1 };
63-
let tracker = self.ring.register_tracker(start);
64-
let slots_ptr = self.ring.slots_ptr();
65-
let idx = self.ring.index;
66-
SubscriberGroup {
67-
ring: self.ring.clone(),
68-
slots_ptr,
69-
index: idx,
70-
cursor: start,
71-
total_lagged: 0,
72-
total_received: 0,
73-
tracker,
74-
}
61+
SubscriberGroup(self.subscribe())
7562
}
7663

7764
/// Create a subscriber starting from the **oldest available** message

tests/correctness.rs

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -813,18 +813,6 @@ fn group_counters() {
813813
assert!(group.total_lagged() > 0);
814814
}
815815

816-
#[test]
817-
fn publisher_sequence() {
818-
let (mut p, _s) = channel::<u64>(8);
819-
assert_eq!(p.sequence(), 0);
820-
p.publish(1);
821-
assert_eq!(p.sequence(), 1);
822-
p.publish_batch(&[2, 3, 4]);
823-
assert_eq!(p.sequence(), 4);
824-
// sequence() == published()
825-
assert_eq!(p.sequence(), p.published());
826-
}
827-
828816
// -------------------------------------------------------------------------
829817
// Bug fix: publish() respects backpressure on bounded channels
830818
// -------------------------------------------------------------------------

0 commit comments

Comments
 (0)