Skip to content

Commit

Permalink
perf(common/timer): check subset of self.items
Browse files Browse the repository at this point in the history
`Timer::take_until` iterates each `VecDeque` in the `Vec` `self.items`. It then
checks the first item in those `VecDeque`s.

Under the assumption that all items in `self.items[t]` are smaller than all
items in `self.items[t+1]`, only a subset of `self.items` needs to be iterated.
Namely only from `self.cursor` to `self.delta(until)`.

This commit changes `take_until` to only check this subset.

---

Why is `Timer::take_until`'s performance relevant?

Whenever `Server::process_next_output` has no more other work to do, it checks
for expired timers.

https://github.com/mozilla/neqo/blob/3151adc53e71273eed1319114380119c70e169a2/neqo-transport/src/server.rs#L650

A `Server` has at most one item per connection in `Timer`. Thus, a `Server` with
a single connection has a single item in `Timer` total.

The `Timer` timer wheel has 16_384 slots.

https://github.com/mozilla/neqo/blob/3151adc53e71273eed1319114380119c70e169a2/neqo-transport/src/server.rs#L55

Thus whenever `Server::process_next_output` has no more other work to do, it
iterates a `Vec` of length `16_384`, only to find at most one timer, which might
or might not be expired.

This shows up in CPU profiles with up to 33%. See e.g. https://github.com/mozilla/neqo/actions/runs/8452074231/artifacts/1363138571.

Note that the profiles do not always show `take_until` as it is oftentimes
inlined. Add `#[inline(never)]` to make sure it isn't.

``` diff
modified   neqo-common/src/timer.rs
@@ -193,6 +193,7 @@ impl<T> Timer<T> {

     /// Take the next item, unless there are no items with
     /// a timeout in the past relative to `until`.
+    #[inline(never)]
     pub fn take_next(&mut self, until: Instant) -> Option<T> {
```

Arguably a 16_384 slot timer wheel is overkill for a single timer. Maybe, to
cover the use-case of a `Server` with a small amount of connections, a
hierarchical timer wheel is helpful?
  • Loading branch information
mxinden committed Mar 28, 2024
1 parent 3151adc commit e48bf5e
Showing 1 changed file with 3 additions and 9 deletions.
12 changes: 3 additions & 9 deletions neqo-common/src/timer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,15 +202,9 @@ impl<T> Timer<T> {
}
}

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);
for i in self.cursor..(self.cursor + self.delta(until)) {
let i = i % self.items.len();
let res = maybe_take(&mut self.items[i], until)?;
if res.is_some() {
return res;
}
Expand Down

0 comments on commit e48bf5e

Please sign in to comment.