Skip to content

Commit

Permalink
Merge pull request #246 from boinkor-net/back-out-clock-clone-bound-c…
Browse files Browse the repository at this point in the history
…hange

Revert "remove `Clock: Clone` bound"
  • Loading branch information
antifuchs authored Oct 21, 2024
2 parents ae92838 + 25c9771 commit 5bc4234
Show file tree
Hide file tree
Showing 17 changed files with 56 additions and 43 deletions.
11 changes: 11 additions & 0 deletions governor/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,17 @@

## [Unreleased] - ReleaseDate

This is a quick bugfix release for the 0.6 version series: A
non-semver-compatible change snuck in, so 0.6.5 will back that change
out, and 0.7.0 will add it back.

### Changed

* The
[`Clock`](https://docs.rs/governor/0.6.5/governor/clock/trait.Clock.html)
trait needs to be Clone for the 0.6.5 release, as removing that
trait in 0.6.3 was not semver-compatible.

## [[0.6.4](https://docs.rs/governor/0.6.4/governor/)] - 2024-10-19

### Added
Expand Down
4 changes: 2 additions & 2 deletions governor/benches/multi_threaded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ fn bench_direct(c: &mut Criterion) {
b.iter_custom(|iters| {
let lim = Arc::new(RateLimiter::direct_with_clock(
Quota::per_second(nonzero!(50u32)),
clock.clone(),
&clock,
));
let mut children = vec![];
let start = Instant::now();
Expand Down Expand Up @@ -68,7 +68,7 @@ fn bench_keyed<M: KeyedStateStore<u32> + Default + Send + Sync + 'static>(c: &mu
let lim: Arc<RateLimiter<_, _, _, NoOpMiddleware>> = Arc::new(RateLimiter::new(
Quota::per_second(nonzero!(50u32)),
state,
clock.clone(),
&clock,
));

let mut children = vec![];
Expand Down
4 changes: 2 additions & 2 deletions governor/benches/realtime_clock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ fn bench_mostly_allow(c: &mut Criterion) {
with_realtime_clocks! {("mostly_allow", group) |b, clock| {
let rl = RateLimiter::direct_with_clock(
#[allow(deprecated)] Quota::new(nonzero!(u32::max_value()), Duration::from_nanos(1)).unwrap(),
clock.clone()
clock
);
b.iter(|| {
black_box(rl.check().is_ok());
Expand All @@ -51,7 +51,7 @@ fn bench_mostly_deny(c: &mut Criterion) {
let mut group = c.benchmark_group("realtime_clock");
group.throughput(Throughput::Elements(1));
with_realtime_clocks! {("mostly_deny", group) |b, clock| {
let rl = RateLimiter::direct_with_clock(Quota::per_hour(nonzero!(1u32)), clock.clone());
let rl = RateLimiter::direct_with_clock(Quota::per_hour(nonzero!(1u32)), clock);
b.iter(|| {
black_box(rl.check().is_ok());
});
Expand Down
4 changes: 2 additions & 2 deletions governor/benches/single_threaded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ fn bench_direct(c: &mut Criterion) {
group.bench_function("direct", |b| {
let clock = clock::FakeRelativeClock::default();
let step = Duration::from_millis(20);
let rl = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(50u32)), clock.clone());
let rl = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(50u32)), &clock);
b.iter_batched(
|| {
clock.advance(step);
Expand Down Expand Up @@ -48,7 +48,7 @@ fn bench_keyed<M: KeyedStateStore<u32> + Default + Send + Sync + 'static>(c: &mu
_,
_,
NoOpMiddleware<<clock::FakeRelativeClock as clock::Clock>::Instant>,
> = RateLimiter::new(Quota::per_second(nonzero!(50u32)), state, clock.clone());
> = RateLimiter::new(Quota::per_second(nonzero!(50u32)), state, &clock);
b.iter_batched(
|| {
clock.advance(step);
Expand Down
4 changes: 2 additions & 2 deletions governor/src/_guide.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
//! # use nonzero_ext::*;
//! # use governor::{clock::FakeRelativeClock, RateLimiter, Quota};
//! let clock = FakeRelativeClock::default();
//! RateLimiter::direct_with_clock(Quota::per_second(nonzero!(50u32)), clock);
//! RateLimiter::direct_with_clock(Quota::per_second(nonzero!(50u32)), &clock);
//! ```
//!
//! #### Constructing a keyed rate limiter
Expand Down Expand Up @@ -144,7 +144,7 @@
//! # use std::time::Duration;
//!
//! let mut clock = FakeRelativeClock::default();
//! let lim = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(20u32)), clock);
//! let lim = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(20u32)), &clock);
//! let ms = Duration::from_millis(1);
//!
//! crossbeam::scope(|scope| {
Expand Down
3 changes: 2 additions & 1 deletion governor/src/clock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
//! }
//! }
//!
//! #[derive(Clone)]
//! struct MyCounter(u64);
//!
//! impl Clock for MyCounter {
Expand Down Expand Up @@ -74,7 +75,7 @@ pub trait Reference:
}

/// A time source used by rate limiters.
pub trait Clock {
pub trait Clock: Clone {
/// A measurement of a monotonically increasing clock.
type Instant: Reference;

Expand Down
2 changes: 1 addition & 1 deletion governor/src/gcra.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ mod test {

let clock = FakeRelativeClock::default();
let quota = Quota::per_second(nonzero!(1u32));
let lb = RateLimiter::direct_with_clock(quota, clock);
let lb = RateLimiter::direct_with_clock(quota, &clock);
assert!(lb.check().is_ok());
assert!(lb
.check()
Expand Down
3 changes: 2 additions & 1 deletion governor/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,10 @@ where
///
/// This is the most generic way to construct a rate-limiter; most users should prefer
/// [`direct`] or other methods instead.
pub fn new(quota: Quota, state: S, clock: C) -> Self {
pub fn new(quota: Quota, state: S, clock: &C) -> Self {
let gcra = Gcra::new(quota);
let start = clock.now();
let clock = clock.clone();
RateLimiter {
state,
clock,
Expand Down
4 changes: 2 additions & 2 deletions governor/src/state/direct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ impl RateLimiter<NotKeyed, InMemoryState, clock::DefaultClock, NoOpMiddleware> {
quota: Quota,
) -> RateLimiter<NotKeyed, InMemoryState, clock::DefaultClock, NoOpMiddleware> {
let clock = clock::DefaultClock::default();
Self::direct_with_clock(quota, clock)
Self::direct_with_clock(quota, &clock)
}
}

Expand All @@ -55,7 +55,7 @@ where
C: clock::Clock,
{
/// Constructs a new direct rate limiter for a quota with a custom clock.
pub fn direct_with_clock(quota: Quota, clock: C) -> Self {
pub fn direct_with_clock(quota: Quota, clock: &C) -> Self {
let state: InMemoryState = Default::default();
RateLimiter::new(quota, state, clock)
}
Expand Down
10 changes: 5 additions & 5 deletions governor/src/state/keyed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,15 @@ where
pub fn keyed(quota: Quota) -> Self {
let state = DefaultKeyedStateStore::default();
let clock = clock::DefaultClock::default();
RateLimiter::new(quota, state, clock)
RateLimiter::new(quota, state, &clock)
}

#[cfg(all(feature = "std", feature = "dashmap"))]
/// Constructs a new keyed rate limiter explicitly backed by a [`DashMap`][::dashmap::DashMap].
pub fn dashmap(quota: Quota) -> Self {
let state = DashMapStateStore::default();
let clock = clock::DefaultClock::default();
RateLimiter::new(quota, state, clock)
RateLimiter::new(quota, state, &clock)
}

#[cfg(any(all(feature = "std", not(feature = "dashmap")), not(feature = "std")))]
Expand All @@ -60,7 +60,7 @@ where
pub fn hashmap(quota: Quota) -> Self {
let state = HashMapStateStore::default();
let clock = clock::DefaultClock::default();
RateLimiter::new(quota, state, clock)
RateLimiter::new(quota, state, &clock)
}
}

Expand All @@ -74,7 +74,7 @@ where
pub fn hashmap(quota: Quota) -> Self {
let state = HashMapStateStore::default();
let clock = clock::DefaultClock::default();
RateLimiter::new(quota, state, clock)
RateLimiter::new(quota, state, &clock)
}
}

Expand Down Expand Up @@ -281,7 +281,7 @@ mod test {
> = RateLimiter::new(
Quota::per_second(nonzero!(1_u32)),
NaiveKeyedStateStore::default(),
FakeRelativeClock::default(),
&FakeRelativeClock::default(),
);
assert_eq!(lim.check_key(&1u32), Ok(()));
assert!(lim.is_empty());
Expand Down
2 changes: 1 addition & 1 deletion governor/src/state/keyed/dashmap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ where
{
/// Constructs a new rate limiter with a custom clock, backed by a
/// [`DashMap`].
pub fn dashmap_with_clock(quota: Quota, clock: C) -> Self {
pub fn dashmap_with_clock(quota: Quota, clock: &C) -> Self {
let state: DashMapStateStore<K> = DashMap::default();
RateLimiter::new(quota, state, clock)
}
Expand Down
2 changes: 1 addition & 1 deletion governor/src/state/keyed/hashmap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ where
C: clock::Clock,
{
/// Constructs a new rate limiter with a custom clock, backed by a [`HashMap`].
pub fn hashmap_with_clock(quota: Quota, clock: C) -> Self {
pub fn hashmap_with_clock(quota: Quota, clock: &C) -> Self {
let state: HashMapStateStore<K> = HashMapStateStore::new(HashMap::new());
RateLimiter::new(quota, state, clock)
}
Expand Down
18 changes: 9 additions & 9 deletions governor/tests/direct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ use std::time::Duration;
#[test]
fn accepts_first_cell() {
let clock = FakeRelativeClock::default();
let lb = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(5u32)), clock);
let lb = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(5u32)), &clock);
assert_eq!(Ok(()), lb.check());
}

#[test]
fn rejects_too_many() {
let clock = FakeRelativeClock::default();
let lb = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(2u32)), clock.clone());
let lb = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(2u32)), &clock);
let ms = Duration::from_millis(1);

// use up our burst capacity (2 in the first second):
Expand All @@ -39,7 +39,7 @@ fn rejects_too_many() {
#[test]
fn all_1_identical_to_1() {
let clock = FakeRelativeClock::default();
let lb = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(2u32)), clock.clone());
let lb = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(2u32)), &clock);
let ms = Duration::from_millis(1);
let one = nonzero!(1u32);

Expand All @@ -64,7 +64,7 @@ fn all_1_identical_to_1() {
#[test]
fn never_allows_more_than_capacity_all() {
let clock = FakeRelativeClock::default();
let lb = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(4u32)), clock.clone());
let lb = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(4u32)), &clock);
let ms = Duration::from_millis(1);

// Use up the burst capacity:
Expand All @@ -87,7 +87,7 @@ fn never_allows_more_than_capacity_all() {
#[test]
fn rejects_too_many_all() {
let clock = FakeRelativeClock::default();
let lb = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(5u32)), clock.clone());
let lb = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(5u32)), &clock);
let ms = Duration::from_millis(1);

// Should not allow the first 15 cells on a capacity 5 bucket:
Expand All @@ -101,7 +101,7 @@ fn rejects_too_many_all() {
#[test]
fn all_capacity_check_rejects_excess() {
let clock = FakeRelativeClock::default();
let lb = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(5u32)), clock);
let lb = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(5u32)), &clock);

assert_eq!(Err(InsufficientCapacity(5)), lb.check_n(nonzero!(15u32)));
assert_eq!(Err(InsufficientCapacity(5)), lb.check_n(nonzero!(6u32)));
Expand All @@ -112,7 +112,7 @@ fn all_capacity_check_rejects_excess() {
fn correct_wait_time() {
let clock = FakeRelativeClock::default();
// Bucket adding a new element per 200ms:
let lb = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(5u32)), clock.clone());
let lb = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(5u32)), &clock);
let ms = Duration::from_millis(1);
let mut conforming = 0;
for _i in 0..20 {
Expand All @@ -137,7 +137,7 @@ fn actual_threadsafety() {
use crossbeam;

let clock = FakeRelativeClock::default();
let lim = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(20u32)), clock.clone());
let lim = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(20u32)), &clock);
let ms = Duration::from_millis(1);

crossbeam::scope(|scope| {
Expand All @@ -159,7 +159,7 @@ fn actual_threadsafety() {
fn default_direct() {
let clock = governor::clock::DefaultClock::default();
let limiter: DefaultDirectRateLimiter =
RateLimiter::direct_with_clock(Quota::per_second(nonzero!(20u32)), clock);
RateLimiter::direct_with_clock(Quota::per_second(nonzero!(20u32)), &clock);
assert_eq!(Ok(()), limiter.check());
}

Expand Down
10 changes: 5 additions & 5 deletions governor/tests/keyed_dashmap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const KEYS: &[u32] = &[1u32, 2u32];
#[test]
fn accepts_first_cell() {
let clock = FakeRelativeClock::default();
let lb = RateLimiter::dashmap_with_clock(Quota::per_second(nonzero!(5u32)), clock.clone());
let lb = RateLimiter::dashmap_with_clock(Quota::per_second(nonzero!(5u32)), &clock);
for key in KEYS {
assert_eq!(Ok(()), lb.check_key(&key), "key {:?}", key);
}
Expand All @@ -23,7 +23,7 @@ fn accepts_first_cell() {
#[test]
fn rejects_too_many() {
let clock = FakeRelativeClock::default();
let lb = RateLimiter::dashmap_with_clock(Quota::per_second(nonzero!(2u32)), clock.clone());
let lb = RateLimiter::dashmap_with_clock(Quota::per_second(nonzero!(2u32)), &clock);
let ms = Duration::from_millis(1);

for key in KEYS {
Expand Down Expand Up @@ -71,7 +71,7 @@ fn expiration() {
let ms = Duration::from_millis(1);

let make_bucket = || {
let lim = RateLimiter::dashmap_with_clock(Quota::per_second(nonzero!(1u32)), clock.clone());
let lim = RateLimiter::dashmap_with_clock(Quota::per_second(nonzero!(1u32)), &clock);
lim.check_key(&"foo").unwrap();
clock.advance(ms * 200);
lim.check_key(&"bar").unwrap();
Expand Down Expand Up @@ -107,7 +107,7 @@ fn actual_threadsafety() {
use crossbeam;

let clock = FakeRelativeClock::default();
let lim = RateLimiter::dashmap_with_clock(Quota::per_second(nonzero!(20u32)), clock.clone());
let lim = RateLimiter::dashmap_with_clock(Quota::per_second(nonzero!(20u32)), &clock);
let ms = Duration::from_millis(1);

for key in KEYS {
Expand Down Expand Up @@ -150,7 +150,7 @@ fn dashmap_length() {
fn dashmap_shrink_to_fit() {
let clock = FakeRelativeClock::default();
// a steady rate of 3ms between elements:
let lim = RateLimiter::dashmap_with_clock(Quota::per_second(nonzero!(20u32)), clock.clone());
let lim = RateLimiter::dashmap_with_clock(Quota::per_second(nonzero!(20u32)), &clock);
let ms = Duration::from_millis(1);

assert_eq!(
Expand Down
10 changes: 5 additions & 5 deletions governor/tests/keyed_hashmap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const KEYS: &[u32] = &[1u32, 2u32];
#[test]
fn accepts_first_cell() {
let clock = FakeRelativeClock::default();
let lb = RateLimiter::hashmap_with_clock(Quota::per_second(nonzero!(5u32)), clock.clone());
let lb = RateLimiter::hashmap_with_clock(Quota::per_second(nonzero!(5u32)), &clock);
for key in KEYS {
assert_eq!(Ok(()), lb.check_key(&key), "key {:?}", key);
}
Expand All @@ -21,7 +21,7 @@ fn accepts_first_cell() {
#[test]
fn rejects_too_many() {
let clock = FakeRelativeClock::default();
let lb = RateLimiter::hashmap_with_clock(Quota::per_second(nonzero!(2u32)), clock.clone());
let lb = RateLimiter::hashmap_with_clock(Quota::per_second(nonzero!(2u32)), &clock);
let ms = Duration::from_millis(1);

for key in KEYS {
Expand Down Expand Up @@ -65,7 +65,7 @@ fn expiration() {
let ms = Duration::from_millis(1);

let make_bucket = || {
let lim = RateLimiter::hashmap_with_clock(Quota::per_second(nonzero!(1u32)), clock.clone());
let lim = RateLimiter::hashmap_with_clock(Quota::per_second(nonzero!(1u32)), &clock);
lim.check_key(&"foo").unwrap();
clock.advance(ms * 200);
lim.check_key(&"bar").unwrap();
Expand Down Expand Up @@ -101,7 +101,7 @@ fn actual_threadsafety() {
use crossbeam;

let clock = FakeRelativeClock::default();
let lim = RateLimiter::hashmap_with_clock(Quota::per_second(nonzero!(20u32)), clock.clone());
let lim = RateLimiter::hashmap_with_clock(Quota::per_second(nonzero!(20u32)), &clock);
let ms = Duration::from_millis(1);

for key in KEYS {
Expand Down Expand Up @@ -144,7 +144,7 @@ fn hashmap_length() {
fn hashmap_shrink_to_fit() {
let clock = FakeRelativeClock::default();
// a steady rate of 3ms between elements:
let lim = RateLimiter::hashmap_with_clock(Quota::per_second(nonzero!(20u32)), clock.clone());
let lim = RateLimiter::hashmap_with_clock(Quota::per_second(nonzero!(20u32)), &clock);
let ms = Duration::from_millis(1);

assert_eq!(
Expand Down
Loading

0 comments on commit 5bc4234

Please sign in to comment.