Skip to content

Sync ReadLimiter #201

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Dec 19, 2020
Merged

Sync ReadLimiter #201

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
55729c9
Atomic read limiter
appaquet Sep 11, 2020
04b2110
Use AtomicUsize for read limiter + feature gate it
appaquet Sep 22, 2020
f6c86c7
Atomic read limiter: better logic
appaquet Nov 27, 2020
25f9948
Atomic read limiter: better logic
appaquet Nov 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions capnp/src/private/arena.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
// THE SOFTWARE.

use alloc::vec::Vec;
use core::cell::{Cell, RefCell};
use core::cell::RefCell;
use core::sync::atomic::{AtomicU64, Ordering};
use core::slice;
use core::u64;

Expand All @@ -31,21 +32,25 @@ use crate::{Error, OutputSegments, Result};
pub type SegmentId = u32;

pub struct ReadLimiter {
pub limit: Cell<u64>,
pub limit: u64,
pub read: AtomicU64,
}

impl ReadLimiter {
pub fn new(limit: u64) -> ReadLimiter {
ReadLimiter { limit: Cell::new(limit) }
ReadLimiter {
limit,
read: AtomicU64::new(0),
}
}

#[inline]
pub fn can_read(&self, amount: u64) -> Result<()> {
let current = self.limit.get();
if amount > current {
let read = self.read.fetch_add(amount, Ordering::Relaxed) + amount;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This potentially allows self.read to overflow and wrap around.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about it, but it also return an error at first if it reaches the limit, so I would expect this to be an undefined behaviour to call it afterward.

My first implementation was a load + cas, but that would be definitely less performant.

We could also have a second AtomicBool that indicates that we reached the limit and return right away. That could potentially be over the limit, but decreases odds of overflow.


if read > self.limit {
Err(Error::failed(format!("read limit exceeded")))
} else {
self.limit.set(current - amount);
Ok(())
}
}
Expand Down