From e48bf5e99671a696c2acf503b941685f81c3dccc Mon Sep 17 00:00:00 2001 From: Max Inden Date: Thu, 28 Mar 2024 18:09:03 +0100 Subject: [PATCH] perf(common/timer): check subset of self.items `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 Timer { /// 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 { ``` 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? --- neqo-common/src/timer.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/neqo-common/src/timer.rs b/neqo-common/src/timer.rs index 3feddb2226..9c9d0f639d 100644 --- a/neqo-common/src/timer.rs +++ b/neqo-common/src/timer.rs @@ -202,15 +202,9 @@ impl Timer { } } - 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; }