Skip to content

Commit cfea4f2

Browse files
committed
got rid of the role field in the guard, made acquire_shard_guard a helper method instead of an accessible method and introduce other ways of obtaining a guard for enqueuing
1 parent 0003feb commit cfea4f2

4 files changed

Lines changed: 74 additions & 42 deletions

File tree

CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
# Change for shardedringbuf:
22

3+
## In v0.5.0:
4+
* Removed access to `acquire_shard_guard()`. Instead, users should use `enqueue_guard_in_shard()` for obtaining
5+
a guard for enqueuing. Dequeue does not face this consequence because it's attempt on obtaining a guard internally
6+
does not cause a cancel safety issue on data being lost; as a result, users can just directly obtain an item through
7+
`dequeue_in_shard()` or `dequeue()`
8+
* Reduced the `ShardLockGuard<'a, T>` size from 32 bytes to 24 bytes (got rid of an unnecessary enum relating to first
9+
bullet point).
10+
311
## In v0.4.0:
412
* Added `CSShardedRingBuf<T>`, which is a 100% cancel safe version of `ShardedRingBuf<T>`. Users need
513
to acquire a guard on the shard first in an async manner before passing ownership of their item to the

benches/cs_srb.rs

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
use std::time::{Duration, Instant};
55

66
use criterion::{BenchmarkId, Criterion, criterion_group, criterion_main};
7-
use sharded_ringbuf::cs_srb::{self, CSShardedRingBuf};
7+
use sharded_ringbuf::cs_srb::CSShardedRingBuf;
88
use tokio::spawn;
99
use tokio::task::yield_now;
1010

@@ -28,12 +28,10 @@ async fn cssrb_bench(capacity: usize, shards: usize, task_count: usize) {
2828
for item in items {
2929
if counter != 0 && counter % full_enq == 0 {
3030
// let guard = rb_clone.enqueue_shard_guard().await;
31-
let guard = rb_clone
32-
.acquire_shard_guard(cs_srb::Acquire::Enqueue, _i)
33-
.await;
31+
let guard = rb_clone.enqueue_guard_in_shard(_i).await;
3432
match guard {
3533
None => break,
36-
Some(guard) => rb_clone.enqueue_item(enq_vec, guard),
34+
Some(guard) => rb_clone.enqueue(enq_vec, guard),
3735
}
3836
enq_vec = Vec::with_capacity(full_enq);
3937
enq_vec.push(item);
@@ -45,12 +43,10 @@ async fn cssrb_bench(capacity: usize, shards: usize, task_count: usize) {
4543
}
4644
if !enq_vec.is_empty() {
4745
// let guard = rb_clone.enqueue_shard_guard().await;
48-
let guard = rb_clone
49-
.acquire_shard_guard(cs_srb::Acquire::Enqueue, _i)
50-
.await;
46+
let guard = rb_clone.enqueue_guard_in_shard(_i).await;
5147
match guard {
5248
None => {}
53-
Some(guard) => rb_clone.enqueue_item(enq_vec, guard),
49+
Some(guard) => rb_clone.enqueue(enq_vec, guard),
5450
}
5551
}
5652
}
@@ -64,13 +60,11 @@ async fn cssrb_bench(capacity: usize, shards: usize, task_count: usize) {
6460
// let counter_clone = counter.clone();
6561
async move {
6662
loop {
67-
let guard = rb_clone
68-
.acquire_shard_guard(cs_srb::Acquire::Dequeue, i)
69-
.await;
63+
let items = rb_clone.dequeue_in_shard(i).await;
7064
// match rb_clone.dequeue_full_in_shard(i).await {
71-
match guard {
72-
Some(guard) => {
73-
for _j in rb_clone.dequeue_item(guard) {
65+
match items {
66+
Some(items) => {
67+
for _j in items {
7468
// println!("{j}");
7569
// counter_clone.fetch_add(1, Ordering::Relaxed);
7670
}

src/cs_srb/guards.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
1-
use crate::cs_srb::{Acquire, InnerRingBuffer};
1+
use crate::cs_srb::InnerRingBuffer;
22
use std::sync::atomic::{AtomicBool, Ordering};
33
use tokio::sync::Notify;
44

55
/// This is a guard for all the shards in CSShardedRingBuf struct
66
/// to make it cancel safe.
77
pub struct ShardLockGuard<'a, T> {
8-
pub(crate) role: Acquire,
98
lock: &'a AtomicBool,
109
notify: &'a Notify,
1110
pub(crate) shard: &'a InnerRingBuffer<T>,
@@ -14,13 +13,11 @@ pub struct ShardLockGuard<'a, T> {
1413
impl<'a, T> ShardLockGuard<'a, T> {
1514
#[inline(always)]
1615
pub(crate) fn acquire_shard_guard(
17-
role: Acquire,
1816
lock: &'a AtomicBool,
1917
notify: &'a Notify,
2018
shard: &'a InnerRingBuffer<T>,
2119
) -> Self {
2220
ShardLockGuard {
23-
role,
2421
lock,
2522
notify,
2623
shard,

src/cs_srb/mod.rs

Lines changed: 56 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use tokio::sync::Notify;
1515
/// This enum is used to declare whether you are enqueuing
1616
/// or dequeuing an item for `acquire_shard_guard()`
1717
#[derive(Debug, PartialEq, Eq)]
18-
pub enum Acquire {
18+
enum Acquire {
1919
Enqueue,
2020
Dequeue,
2121
}
@@ -296,7 +296,8 @@ impl<T> CSShardedRingBuf<T> {
296296
/// - Dequeue: only returns None if the shard is *empty*
297297
///
298298
/// Time Complexity: O(s_t) where s_t is how long it takes to acquire the shard
299-
pub async fn acquire_shard_guard(
299+
#[inline]
300+
async fn acquire_shard_guard(
300301
&self,
301302
acquire: Acquire,
302303
shard_ind: usize,
@@ -346,14 +347,12 @@ impl<T> CSShardedRingBuf<T> {
346347
}
347348
if matches!(acquire, Acquire::Dequeue) {
348349
Some(ShardLockGuard::acquire_shard_guard(
349-
acquire,
350350
&self.shard_locks[shard_ind],
351351
&self.job_space_shard_notifs[enq_shard_ind],
352352
&self.inner_rb[shard_ind],
353353
))
354354
} else {
355355
Some(ShardLockGuard::acquire_shard_guard(
356-
acquire,
357356
&self.shard_locks[shard_ind],
358357
&self.job_post_shard_notifs[shard_ind],
359358
&self.inner_rb[shard_ind],
@@ -363,17 +362,14 @@ impl<T> CSShardedRingBuf<T> {
363362

364363
/// Given a `ShardLockGuard<'_, T>`, it enqueues an item inside the shard.
365364
///
366-
/// If a guard was obtained with the role of dequeuing, this function will panic.
365+
/// You must obtain a ShardLockGuard<'_, T> from either `enqueue_guard()`
366+
/// or `enqueue_guard_in_shard()`
367367
///
368368
/// Time Complexity: O(1)
369369
///
370370
/// Space Complexity: O(1)
371371
#[inline(always)]
372-
pub fn enqueue_item(&self, item: T, shard_guard: ShardLockGuard<'_, T>) {
373-
assert!(
374-
matches!(shard_guard.role, Acquire::Enqueue),
375-
"You must acquire an Enqueue guard through 'enqueue_shard_guard()' or 'acquire_shard_guard' with Acquire::Enqueue."
376-
);
372+
pub fn enqueue(&self, item: T, shard_guard: ShardLockGuard<'_, T>) {
377373
let inner = shard_guard.shard;
378374
// we use fetch add here because we want to obtain the previous value
379375
// to dequeue while also incrementing this counter (separate load and store
@@ -387,32 +383,43 @@ impl<T> CSShardedRingBuf<T> {
387383
}
388384
}
389385

390-
/// Obtain a guard on a shard in the ring buffer for the purpose of enqueuing.
386+
/// Obtain a guard on a specific shard the ring buffer for the purpose of
387+
/// enqueuing.
388+
///
389+
/// Returns None if the buffer was poisoned.
390+
///
391+
/// Time Complexity: O(s_t) where s_t is the time it takes to acquire a shard
392+
///
393+
/// Space complexity: O(1)
394+
#[inline(always)]
395+
pub async fn enqueue_guard_in_shard(&self, shard_ind: usize) -> Option<ShardLockGuard<'_, T>> {
396+
self.acquire_shard_guard(Acquire::Enqueue, shard_ind).await
397+
}
398+
399+
/// Obtain a guard on a shard the ring buffer for the purpose of enqueuing.
400+
///
391401
/// Returns None if the buffer was poisoned.
392402
///
403+
/// Note: It obtains the shard in a circular manner, so for example, at the beginning,
404+
/// it obtains shard 0, then shard 1, the shard 2, ... (all modded with the number
405+
/// of shards that exists in this buffer)
406+
///
393407
/// Time Complexity: O(s_t) where s_t is the time it takes to acquire a shard
394408
///
395409
/// Space complexity: O(1)
396410
#[inline(always)]
397-
pub async fn enqueue_shard_guard(&self) -> Option<ShardLockGuard<'_, T>> {
411+
pub async fn enqueue_guard(&self) -> Option<ShardLockGuard<'_, T>> {
398412
let shard_ind = self.shard_enq.fetch_add(1, Ordering::Relaxed) % self.get_num_of_shards();
399413
self.acquire_shard_guard(Acquire::Enqueue, shard_ind).await
400414
}
401415

402416
/// Given a `ShardLockGuard<'_, T>`, it dequeues an item inside the shard.
403417
///
404-
/// If a guard was obtained with the role of enqueuing, this function will panic.
405-
///
406418
/// Time Complexity: O(1)
407419
///
408420
/// Space Complexity: O(1)
409421
#[inline(always)]
410-
pub fn dequeue_item(&self, shard_guard: ShardLockGuard<'_, T>) -> T {
411-
assert!(
412-
matches!(shard_guard.role, Acquire::Dequeue),
413-
"You must acquire a Dequeue guard through 'dequeue_shard_guard()' or 'acquire_shard_guard' with Acquire::Dequeue."
414-
);
415-
422+
fn dequeue_item(&self, shard_guard: ShardLockGuard<'_, T>) -> T {
416423
let inner = shard_guard.shard;
417424
// we use fetch add here because we want to obtain the previous value
418425
// to dequeue while also incrementing this counter (separate load and store
@@ -422,16 +429,42 @@ impl<T> CSShardedRingBuf<T> {
422429
unsafe { (*item_cell).assume_init_read() }
423430
}
424431

425-
/// Obtain a guard on a shard in the ring buffer for the purpose of dequeuing.
432+
/// Performs both the operation of obtaining a guard on a shard in the ring buffer
433+
/// and dequeuing an item off from the buffer. This can be done like together because
434+
/// due to inherent cancellation safety (data either stays in buffer on cancellation
435+
/// or it gets taken out without touching an await point).
436+
///
426437
/// Returns None if the buffer was poisoned and the shard contains no more items.
427438
///
428439
/// Time Complexity: O(s_t) where s_t is the time it takes to acquire a shard
429440
///
430441
/// Space Complexity: O(1)
431442
#[inline(always)]
432-
pub async fn dequeue_shard_guard(&self) -> Option<ShardLockGuard<'_, T>> {
443+
pub async fn dequeue_in_shard(&self, shard_ind: usize) -> Option<T> {
444+
let shard_guard = self.acquire_shard_guard(Acquire::Dequeue, shard_ind).await;
445+
shard_guard.map(|guard| self.dequeue_item(guard))
446+
}
447+
448+
/// Performs both the operation of obtaining a guard on a shard in the ring buffer
449+
/// and dequeuing an item off from the buffer. This can be done like together because
450+
/// due to inherent cancellation safety (data either stays in buffer on cancellation
451+
/// or it gets taken out without touching an await point).
452+
///
453+
/// Returns None if the buffer was poisoned and the shard contains no more items.
454+
///
455+
/// Note: It obtains the shard in a circular manner, so for example, at the beginning,
456+
/// it obtains shard 0, then shard 1, the shard 2, ... (all modded with the number
457+
/// of shards that exists in this buffer)
458+
///
459+
/// Time Complexity: O(s_t) where s_t is the time it takes to acquire a shard
460+
///
461+
/// Space Complexity: O(1)
462+
#[inline(always)]
463+
pub async fn dequeue(&self) -> Option<T> {
433464
let shard_ind = self.shard_deq.fetch_add(1, Ordering::Relaxed) % self.get_num_of_shards();
434-
self.acquire_shard_guard(Acquire::Dequeue, shard_ind).await
465+
let shard_guard = self.acquire_shard_guard(Acquire::Dequeue, shard_ind).await;
466+
467+
shard_guard.map(|guard| self.dequeue_item(guard))
435468
}
436469

437470
/// Sets the poison flag of the ring buffer to true. This will prevent enqueuers

0 commit comments

Comments
 (0)