Merge branch 'main' into chore-rename-feature-fuzzing

Signed-off-by: Lars Eggert <[email protected]>

.github/workflows/bench.yml

@@ -2,6 +2,9 @@ name: Bench
 on:
   workflow_call:
   workflow_dispatch:
+  schedule:
+    # Run at 1 AM each day, so there is a `main`-branch baseline in the cache.
+    - cron: '0 1 * * *'
 env:
   CARGO_PROFILE_BENCH_BUILD_OVERRIDE_DEBUG: true
   CARGO_PROFILE_RELEASE_DEBUG: true
@@ -115,7 +118,7 @@ jobs:
             echo "### Benchmark results"
             echo
           } > results.md
-          SHA=$(cat target/criterion/baseline-sha.txt)
+          SHA=$(cat target/criterion/baseline-sha.txt || true)
           if [ -n "$SHA" ]; then
             {
               echo "Performance differences relative to $SHA."

.github/workflows/check.yml

@@ -150,7 +150,7 @@ jobs:
           # respective default features only. Can reveal warnings otherwise
           # hidden given that a plain cargo clippy combines all features of the
           # workspace. See e.g. https://github.com/mozilla/neqo/pull/1695.
-          cargo +${{ matrix.rust-toolchain }} hack clippy --all-targets -- -D warnings || ${{ matrix.rust-toolchain == 'nightly' }}
+          cargo +${{ matrix.rust-toolchain }} hack clippy --all-targets --feature-powerset --exclude-features gecko -- -D warnings || ${{ matrix.rust-toolchain == 'nightly' }}
         if: success() || failure()
 
       - name: Check rustdoc links

neqo-common/Cargo.toml

@@ -21,6 +21,7 @@ qlog = { version = "0.12", default-features = false }
 time = { version = "0.3", default-features = false, features = ["formatting"] }
 
 [dev-dependencies]
+criterion = { version = "0.5", default-features = false, features = ["html_reports"] }
 test-fixture = { path = "../test-fixture" }
 
 [features]
@@ -33,3 +34,7 @@ features = ["timeapi"]
 [lib]
 # See https://github.com/bheisler/criterion.rs/blob/master/book/src/faq.md#cargo-bench-gives-unrecognized-option-errors-for-valid-command-line-options
 bench = false
+
+[[bench]]
+name = "timer"
+harness = false

neqo-common/benches/timer.rs

@@ -0,0 +1,39 @@
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+use std::time::{Duration, Instant};
+
+use criterion::{criterion_group, criterion_main, Criterion};
+use neqo_common::timer::Timer;
+use test_fixture::now;
+
+fn benchmark_timer(c: &mut Criterion) {
+    c.bench_function("drain a timer quickly", |b| {
+        b.iter_batched_ref(
+            make_timer,
+            |(_now, timer)| {
+                while let Some(t) = timer.next_time() {
+                    assert!(timer.take_next(t).is_some());
+                }
+            },
+            criterion::BatchSize::SmallInput,
+        );
+    });
+}
+
+fn make_timer() -> (Instant, Timer<()>) {
+    const TIMES: &[u64] = &[1, 2, 3, 5, 8, 13, 21, 34];
+
+    let now = now();
+    let mut timer = Timer::new(now, Duration::from_millis(777), 100);
+    for &t in TIMES {
+        timer.add(now + Duration::from_secs(t), ());
+    }
+    (now, timer)
+}
+
+criterion_group!(benches, benchmark_timer);
+criterion_main!(benches);

neqo-common/src/timer.rs

@@ -5,6 +5,7 @@
 // except according to those terms.
 
 use std::{
+    collections::VecDeque,
     mem,
     time::{Duration, Instant},
 };
@@ -27,7 +28,7 @@ impl<T> TimerItem<T> {
 /// points). Time is relative, the wheel has an origin time and it is unable to represent times that
 /// are more than `granularity * capacity` past that time.
 pub struct Timer<T> {
-    items: Vec<Vec<TimerItem<T>>>,
+    items: Vec<VecDeque<TimerItem<T>>>,
     now: Instant,
     granularity: Duration,
     cursor: usize,
@@ -55,9 +56,14 @@ impl<T> Timer<T> {
     /// Return a reference to the time of the next entry.
     #[must_use]
     pub fn next_time(&self) -> Option<Instant> {
-        for i in 0..self.items.len() {
-            let idx = self.bucket(i);
-            if let Some(t) = self.items[idx].first() {
+        let idx = self.bucket(0);
+        for i in idx..self.items.len() {
+            if let Some(t) = self.items[i].front() {
+                return Some(t.time);
+            }
+        }
+        for i in 0..idx {
+            if let Some(t) = self.items[i].front() {
                 return Some(t.time);
             }
         }
@@ -145,6 +151,9 @@ impl<T> Timer<T> {
 
     /// Given knowledge of the time an item was added, remove it.
     /// This requires use of a predicate that identifies matching items.
+    ///
+    /// # Panics
+    /// Impossible, I think.
     pub fn remove<F>(&mut self, time: Instant, mut selector: F) -> Option<T>
     where
         F: FnMut(&T) -> bool,
@@ -167,7 +176,7 @@ impl<T> Timer<T> {
                 break;
             }
             if selector(&self.items[bucket][i].item) {
-                return Some(self.items[bucket].remove(i).item);
+                return Some(self.items[bucket].remove(i).unwrap().item);
             }
         }
         // ... then forwards.
@@ -176,7 +185,7 @@ impl<T> Timer<T> {
                 break;
             }
             if selector(&self.items[bucket][i].item) {
-                return Some(self.items[bucket].remove(i).item);
+                return Some(self.items[bucket].remove(i).unwrap().item);
             }
         }
         None
@@ -185,10 +194,25 @@ impl<T> Timer<T> {
     /// Take the next item, unless there are no items with
     /// a timeout in the past relative to `until`.
     pub fn take_next(&mut self, until: Instant) -> Option<T> {
-        for i in 0..self.items.len() {
-            let idx = self.bucket(i);
-            if !self.items[idx].is_empty() && self.items[idx][0].time <= until {
-                return Some(self.items[idx].remove(0).item);
+        fn maybe_take<T>(v: &mut VecDeque<TimerItem<T>>, until: Instant) -> Option<T> {
+            if !v.is_empty() && v[0].time <= until {
+                Some(v.pop_front().unwrap().item)
+            } else {
+                None
+            }
+        }
+
+        let idx = self.bucket(0);
+        for i in idx..self.items.len() {
+            let res = maybe_take(&mut self.items[i], until);
+            if res.is_some() {
+                return res;
+            }
+        }
+        for i in 0..idx {
+            let res = maybe_take(&mut self.items[i], until);
+            if res.is_some() {
+                return res;
             }
         }
         None
@@ -201,7 +225,7 @@ impl<T> Timer<T> {
         if until >= self.now + self.span() {
             // Drain everything, so a clean sweep.
             let mut empty_items = Vec::with_capacity(self.items.len());
-            empty_items.resize_with(self.items.len(), Vec::default);
+            empty_items.resize_with(self.items.len(), VecDeque::default);
             let mut items = mem::replace(&mut self.items, empty_items);
             self.now = until;
             self.cursor = 0;

neqo-crypto/src/aead_fuzzing.rs

@@ -41,6 +41,7 @@ impl FuzzingAead {
         }
     }
 
+    #[allow(clippy::missing_errors_doc)]
     pub fn encrypt<'a>(
         &self,
         count: u64,
@@ -58,6 +59,7 @@ impl FuzzingAead {
         Ok(&output[..l + 16])
     }
 
+    #[allow(clippy::missing_errors_doc)]
     pub fn decrypt<'a>(
         &self,
         count: u64,

neqo-transport/benches/range_tracker.rs

@@ -11,30 +11,32 @@ const CHUNK: u64 = 1000;
 const END: u64 = 100_000;
 fn build_coalesce(len: u64) -> RangeTracker {
     let mut used = RangeTracker::default();
-    used.mark_acked(0, CHUNK as usize);
-    used.mark_sent(CHUNK, END as usize);
+    let chunk = usize::try_from(CHUNK).expect("should fit");
+    used.mark_acked(0, chunk);
+    used.mark_sent(CHUNK, usize::try_from(END).expect("should fit"));
     // leave a gap or it will coalesce here
     for i in 2..=len {
         // These do not get immediately coalesced when marking since they're not at the end or start
-        used.mark_acked(i * CHUNK, CHUNK as usize);
+        used.mark_acked(i * CHUNK, chunk);
     }
     used
 }
 
 fn coalesce(c: &mut Criterion, count: u64) {
+    let chunk = usize::try_from(CHUNK).expect("should fit");
     c.bench_function(
         &format!("coalesce_acked_from_zero {count}+1 entries"),
         |b| {
             b.iter_batched_ref(
                 || build_coalesce(count),
                 |used| {
-                    used.mark_acked(CHUNK, CHUNK as usize);
+                    used.mark_acked(CHUNK, chunk);
                     let tail = (count + 1) * CHUNK;
-                    used.mark_sent(tail, CHUNK as usize);
-                    used.mark_acked(tail, CHUNK as usize);
+                    used.mark_sent(tail, chunk);
+                    used.mark_acked(tail, chunk);
                 },
                 criterion::BatchSize::SmallInput,
-            )
+            );
         },
     );
 }

neqo-transport/benches/rx_stream_orderer.rs

@@ -11,14 +11,14 @@ fn rx_stream_orderer() {
     let mut rx = RxStreamOrderer::new();
     let data: &[u8] = &[0; 1337];
 
-    for i in 0..100000 {
+    for i in 0..100_000 {
         rx.inbound_frame(i * 1337, data);
     }
 }
 
 fn criterion_benchmark(c: &mut Criterion) {
     c.bench_function("RxStreamOrderer::inbound_frame()", |b| {
-        b.iter(rx_stream_orderer)
+        b.iter(rx_stream_orderer);
     });
 }
 

neqo-transport/benches/transfer.rs

@@ -20,9 +20,10 @@ const ZERO: Duration = Duration::from_millis(0);
 const JITTER: Duration = Duration::from_millis(10);
 const TRANSFER_AMOUNT: usize = 1 << 22; // 4Mbyte
 
-fn benchmark_transfer(c: &mut Criterion, label: &str, seed: Option<impl AsRef<str>>) {
+fn benchmark_transfer(c: &mut Criterion, label: &str, seed: &Option<impl AsRef<str>>) {
     let mut group = c.benchmark_group("transfer");
     group.throughput(Throughput::Bytes(u64::try_from(TRANSFER_AMOUNT).unwrap()));
+    group.noise_threshold(0.03);
     group.bench_function(label, |b| {
         b.iter_batched(
             || {
@@ -44,7 +45,7 @@ fn benchmark_transfer(c: &mut Criterion, label: &str, seed: Option<impl AsRef<st
                 sim.run();
             },
             SmallInput,
-        )
+        );
     });
     group.finish();
 }
@@ -53,15 +54,15 @@ fn benchmark_transfer_variable(c: &mut Criterion) {
     benchmark_transfer(
         c,
         "Run multiple transfers with varying seeds",
-        std::env::var("SIMULATION_SEED").ok(),
+        &std::env::var("SIMULATION_SEED").ok(),
     );
 }
 
 fn benchmark_transfer_fixed(c: &mut Criterion) {
     benchmark_transfer(
         c,
         "Run multiple transfers with the same seed",
-        Some("62df6933ba1f543cece01db8f27fb2025529b27f93df39e19f006e1db3b8c843"),
+        &Some("62df6933ba1f543cece01db8f27fb2025529b27f93df39e19f006e1db3b8c843"),
     );
 }