From cc093d0cf47a81ec4b11a717904d12d5e9e9524e Mon Sep 17 00:00:00 2001 From: Andreas Fuchs Date: Mon, 21 Oct 2024 12:29:41 -0400 Subject: [PATCH 1/2] Revert "remove `Clock: Clone` bound" This reverts commit b11f0583c90ba8862b45cd3153fbb9a7e080afa1, as it's not semver-compatible, and I need to cut a new release in the 0.6 series. Will re-apply for 0.7.0. --- governor/benches/multi_threaded.rs | 4 ++-- governor/benches/realtime_clock.rs | 4 ++-- governor/benches/single_threaded.rs | 4 ++-- governor/src/_guide.rs | 4 ++-- governor/src/clock.rs | 3 ++- governor/src/gcra.rs | 2 +- governor/src/state.rs | 3 ++- governor/src/state/direct.rs | 4 ++-- governor/src/state/keyed.rs | 10 +++++----- governor/src/state/keyed/dashmap.rs | 2 +- governor/src/state/keyed/hashmap.rs | 2 +- governor/tests/direct.rs | 18 +++++++++--------- governor/tests/keyed_dashmap.rs | 10 +++++----- governor/tests/keyed_hashmap.rs | 10 +++++----- governor/tests/middleware.rs | 6 +++--- governor/tests/proptests.rs | 2 +- 16 files changed, 45 insertions(+), 43 deletions(-) diff --git a/governor/benches/multi_threaded.rs b/governor/benches/multi_threaded.rs index 64c5a086..2a864ca9 100644 --- a/governor/benches/multi_threaded.rs +++ b/governor/benches/multi_threaded.rs @@ -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(); @@ -68,7 +68,7 @@ fn bench_keyed + Default + Send + Sync + 'static>(c: &mu let lim: Arc> = Arc::new(RateLimiter::new( Quota::per_second(nonzero!(50u32)), state, - clock.clone(), + &clock, )); let mut children = vec![]; diff --git a/governor/benches/realtime_clock.rs b/governor/benches/realtime_clock.rs index 0c59c85c..864d2ad3 100644 --- a/governor/benches/realtime_clock.rs +++ b/governor/benches/realtime_clock.rs @@ -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()); @@ -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()); }); diff --git a/governor/benches/single_threaded.rs b/governor/benches/single_threaded.rs index b9caa5f1..bf496881 100644 --- a/governor/benches/single_threaded.rs +++ b/governor/benches/single_threaded.rs @@ -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); @@ -48,7 +48,7 @@ fn bench_keyed + Default + Send + Sync + 'static>(c: &mu _, _, NoOpMiddleware<::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); diff --git a/governor/src/_guide.rs b/governor/src/_guide.rs index a6ff3fcb..9a65b2b3 100644 --- a/governor/src/_guide.rs +++ b/governor/src/_guide.rs @@ -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 @@ -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| { diff --git a/governor/src/clock.rs b/governor/src/clock.rs index 7895bcdd..7dfd86df 100644 --- a/governor/src/clock.rs +++ b/governor/src/clock.rs @@ -33,6 +33,7 @@ //! } //! } //! +//! #[derive(Clone)] //! struct MyCounter(u64); //! //! impl Clock for MyCounter { @@ -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; diff --git a/governor/src/gcra.rs b/governor/src/gcra.rs index bb458872..cf2a0b0b 100644 --- a/governor/src/gcra.rs +++ b/governor/src/gcra.rs @@ -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() diff --git a/governor/src/state.rs b/governor/src/state.rs index 3bca1e35..42ee2031 100644 --- a/governor/src/state.rs +++ b/governor/src/state.rs @@ -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, diff --git a/governor/src/state/direct.rs b/governor/src/state/direct.rs index c35aa336..5e94d6f4 100644 --- a/governor/src/state/direct.rs +++ b/governor/src/state/direct.rs @@ -46,7 +46,7 @@ impl RateLimiter { quota: Quota, ) -> RateLimiter { let clock = clock::DefaultClock::default(); - Self::direct_with_clock(quota, clock) + Self::direct_with_clock(quota, &clock) } } @@ -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) } diff --git a/governor/src/state/keyed.rs b/governor/src/state/keyed.rs index 3203bcb4..1ff92e77 100644 --- a/governor/src/state/keyed.rs +++ b/governor/src/state/keyed.rs @@ -43,7 +43,7 @@ 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"))] @@ -51,7 +51,7 @@ where 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")))] @@ -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) } } @@ -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) } } @@ -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()); diff --git a/governor/src/state/keyed/dashmap.rs b/governor/src/state/keyed/dashmap.rs index 5f67ccd1..fe3e1c6d 100755 --- a/governor/src/state/keyed/dashmap.rs +++ b/governor/src/state/keyed/dashmap.rs @@ -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 = DashMap::default(); RateLimiter::new(quota, state, clock) } diff --git a/governor/src/state/keyed/hashmap.rs b/governor/src/state/keyed/hashmap.rs index 891bc481..8a5df6f2 100644 --- a/governor/src/state/keyed/hashmap.rs +++ b/governor/src/state/keyed/hashmap.rs @@ -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 = HashMapStateStore::new(HashMap::new()); RateLimiter::new(quota, state, clock) } diff --git a/governor/tests/direct.rs b/governor/tests/direct.rs index 62562923..ca21f13d 100644 --- a/governor/tests/direct.rs +++ b/governor/tests/direct.rs @@ -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): @@ -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); @@ -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: @@ -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: @@ -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))); @@ -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 { @@ -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| { @@ -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()); } diff --git a/governor/tests/keyed_dashmap.rs b/governor/tests/keyed_dashmap.rs index cd1bb599..2c427f31 100755 --- a/governor/tests/keyed_dashmap.rs +++ b/governor/tests/keyed_dashmap.rs @@ -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); } @@ -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 { @@ -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(); @@ -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 { @@ -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!( diff --git a/governor/tests/keyed_hashmap.rs b/governor/tests/keyed_hashmap.rs index 8663ac89..f7eafd02 100644 --- a/governor/tests/keyed_hashmap.rs +++ b/governor/tests/keyed_hashmap.rs @@ -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); } @@ -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 { @@ -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(); @@ -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 { @@ -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!( diff --git a/governor/tests/middleware.rs b/governor/tests/middleware.rs index ed7ce24c..eeabf2ae 100644 --- a/governor/tests/middleware.rs +++ b/governor/tests/middleware.rs @@ -28,7 +28,7 @@ impl RateLimitingMiddleware<::Instant> for My #[test] fn changes_allowed_type() { let clock = FakeRelativeClock::default(); - let lim = RateLimiter::direct_with_clock(Quota::per_hour(nonzero!(1_u32)), clock.clone()) + let lim = RateLimiter::direct_with_clock(Quota::per_hour(nonzero!(1_u32)), &clock) .with_middleware::(); assert_eq!(Ok(666), lim.check()); assert_eq!(Err(()), lim.check()); @@ -37,7 +37,7 @@ fn changes_allowed_type() { #[test] fn state_information() { let clock = FakeRelativeClock::default(); - let lim = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(4u32)), clock.clone()) + let lim = RateLimiter::direct_with_clock(Quota::per_second(nonzero!(4u32)), &clock) .with_middleware::(); assert_eq!( Ok(3), @@ -82,7 +82,7 @@ fn state_snapshot_tracks_quota_accurately() { let clock = FakeRelativeClock::default(); // First test - let lim = RateLimiter::direct_with_clock(quota, clock.clone()) + let lim = RateLimiter::direct_with_clock(quota, &clock) .with_middleware::(); assert_eq!(lim.check().unwrap().remaining_burst_capacity(), 1); diff --git a/governor/tests/proptests.rs b/governor/tests/proptests.rs index 3beafaeb..4926e114 100644 --- a/governor/tests/proptests.rs +++ b/governor/tests/proptests.rs @@ -42,7 +42,7 @@ fn cover_count_derives() { fn accurate_not_until() { proptest!(test_config(), |(capacity: Count, additional: Count, wait_time_parts: Count)| { let clock = FakeRelativeClock::default(); - let lb = RateLimiter::direct_with_clock(Quota::per_second(capacity.0), clock.clone()); + let lb = RateLimiter::direct_with_clock(Quota::per_second(capacity.0), &clock); let step = Duration::from_secs(1) / capacity.0.get(); // use up the burst capacity: From 25c97713cab7f3d0f6d11879497534da00c778f6 Mon Sep 17 00:00:00 2001 From: Andreas Fuchs Date: Mon, 21 Oct 2024 12:33:34 -0400 Subject: [PATCH 2/2] Add a changelog entry --- governor/CHANGELOG.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/governor/CHANGELOG.md b/governor/CHANGELOG.md index 16e85b12..58c66f7f 100644 --- a/governor/CHANGELOG.md +++ b/governor/CHANGELOG.md @@ -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