Skip to content

perf(uring): reuse the worker turn timestamp - #3136

Open
kixelated wants to merge 1 commit into
devfrom
codex/3122-uring-turn-clock
Open

perf(uring): reuse the worker turn timestamp#3136
kixelated wants to merge 1 commit into
devfrom
codex/3122-uring-turn-clock

Conversation

@kixelated

@kixelated kixelated commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

Sample the monotonic clock once at the start of each io_uring worker turn and share that instant with the timer sweep, Timer::poll, and keep-alive re-arming.

Root cause: block_on read Instant::now() for its sweep, and then every deadline polled in that same turn read it again independently. Timer::poll is the volume: the connection driver polls its quiche timeout and its keep-alive on every pass, so a busy turn paid a vDSO clock_gettime per connection per poll to learn a time the loop had just measured. #3122 measured [vdso] at 2.95% of relay CPU on the io_uring path against 0.72% on tokio.

The snapshot lives behind an RAII guard. Clock::turn(now) freezes the clock and dropping the guard, including on unwind, thaws it, so reads before the first turn or while the worker is parked still hit the real clock and a stale instant is never served past the turn that took it. Handle publishes it as moq_net::Timers::now, which is the clock the trait doc already tells callers arming a relative deadline (now + interval) to read, so the keep-alive re-arm goes through the trait rather than a bespoke accessor.

One deliberate behavior change: a timer that comes due during a turn's polling now fires on the next turn rather than in that turn's sweep. That is what keeps the sweep and the eager Timer::poll path agreeing on the time instead of one calling a deadline due while the other calls it pending. It costs at most one extra loop iteration, since maybe_park hands an already-past deadline to io_uring_enter, which returns immediately rather than blocking.

What this does not cover

quiche reads the clock itself, once inside recv and once inside send (quiche/src/lib.rs:2963 and :3968), and again in on_timeout. Those are per packet batch and are not reachable from here, so this recovers the timer and keep-alive share of that 2.95%, not all of it. Filed as a follow-up rather than claimed here.

Public API changes

  • None. Timer gains a private field; Timers::now is a defaulted trait method now overridden for Handle.

Wire behavior changes

  • None.

Test plan

moq-uring is cfg(linux)-gated, so it was verified in a container on real io_uring (kernel 6.19), not just cross-compiled:

  • cargo clippy -p moq-uring --all-targets -- -D warnings: clean.
  • cargo fmt -p moq-uring -- --check: clean. macOS cargo fmt skips this crate entirely, so it has to run on Linux.
  • cargo test -p moq-uring: 47 tests pass (21 lib + echo/session/endpoint/workers/web), no skipping io_uring lines, so these ran against the real ring.
  • cargo clippy -p moq-uring --target x86_64-unknown-linux-gnu --all-targets -- -D warnings from macOS: clean.

Both new tests were mutation-verified in isolation:

  • worker::the_clock_is_frozen_for_a_turn asserts Handle::now() is stable across a 5ms sleep inside block_on and moves outside it. Removing the Timers::now override fails it.
  • shared::the_turn_freezes_and_the_guard_thaws asserts the guard restores the real clock. Making Turn::drop a no-op fails it.
  • timer::poll_uses_the_worker_turn_clock arms a deadline a second out in real time, so its Ready can only come from the turn's instant.

Fixes #3122

(Written by Claude Opus 5, taking over Codex's original patch)

@kixelated
kixelated force-pushed the codex/3122-uring-turn-clock branch from ad6d23f to 29b0204 Compare August 28, 2026 16:50

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ad6d23f4eb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread rs/moq-uring/src/worker.rs Outdated
let waker = self.shared.unpark.waker();
loop {
let now = Instant::now();
self.shared.clock.begin(now);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reset the turn clock during unwinding

When the future passed to block_on panics and the caller catches the unwind, neither normal clock.end() call executes, so Clock::turn remains true indefinitely. The retained Handle then returns the stale turn snapshot even outside the worker, and a relative deadline created before the next block_on can be shortened or fire immediately. Use an unwind-safe scope guard to clear the active snapshot whenever the polling turn exits.

Useful? React with 👍 / 👎.

Sample the monotonic clock once at the start of each io_uring worker turn
and share that instant with the timer sweep, `Timer::poll`, and keep-alive
re-arming, so a turn that polls a deadline per connection pays for one vDSO
`clock_gettime` instead of one per poll.

The snapshot lives behind an RAII guard: `Clock::turn(now)` freezes the
clock and dropping the guard, including on unwind, thaws it, so reads
before the first turn or while the worker is parked still hit the real
clock and a stale instant is never served past the turn that took it.
`Handle` publishes it as `moq_net::Timers::now`, which is the clock the
trait already tells callers arming a relative deadline to read.

Fixes #3122

Co-Authored-By: Codex <codex@openai.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant