refactor: finish the ez waiter conversion, and build the poll bridge on kio::Park - #358
refactor: finish the ez waiter conversion, and build the poll bridge on kio::Park#358kixelated wants to merge 2 commits into
Conversation
The last poll method in `ez` still taking a `Context` was the keep-alive ticker's, so the driver built one from its waiter to call it. Take the waiter instead and build the `Context` at the one place that genuinely needs it, where tokio's `Interval::poll_tick` demands it. No behaviour change: the `Context` is made from the same waker, one level down. Everything else in `ez` already takes `&kio::Waiter`; the only `Context`s left are the `AsyncRead`/`AsyncWrite` impls, whose signatures are tokio's. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
kio 0.5.3 released `Park` (the `WaiterCell` of moq-dev/moq#2560, renamed), so the hand-rolled retention in `Parked` goes away: it now wraps `Park` and inherits the reuse `Park` does — a steady-state cell allocates nothing, where this one built a fresh `Waiter` and `Arc` on every poll. What stays is the release-on-`Ready`. `Park` holds its waiter until the next poll or until it drops, which is right for a pending operation but not a finished one: the stream is done with that caller, and holding its waker pins the polling task's allocation until something polls the cell again. Neutering `settle` — that is, using bare `Park` — fails `a_finished_read_releases_the_poller`, so the wrapper earns its keep. 0.5.3 also made `Waiter` `Clone`, which retires the two-step dance in the `AsyncRead`/`AsyncWrite` impls. They took that shape only because the cell's borrow could not be ended before the `&mut self` poll; a cloned waiter shares the parked one's identity, so `hold` hands one out and the borrow ends there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 32 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0ad2305ee
ℹ️ 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".
| #[derive(Clone, Default)] | ||
| pub(crate) struct Parked(Park); |
There was a problem hiding this comment.
When a Connection is cloned after a poll_* operation has returned Pending, deriving Clone here clones Park and therefore the live waiter identity. If the original handle is then dropped or completes, an otherwise unused clone continues owning its registration, retaining the cancelled task's waker/allocation and causing stale wakeups for potentially the rest of the connection. The previous custom Clone deliberately returned default() because these registrations are per-handle; preserve that behavior here and in the identical Quinn copy at rs/web-transport-quinn/src/waiters.rs, with a regression test for cloning a pending handle.
AGENTS.md reference: AGENTS.md:L46-L47
Useful? React with 👍 / 👎.
1 participant
Follow-up to #353, now that kio 0.5.3 has shipped. Two independent commits.
eztakes a waiter everywhere#353 converted
ez's poll surface from&Wakerto&kio::Waiter, but one method kept aContext: the keep-alive ticker's, which the driver called by building aContextfrom its waiter. It takes the waiter now, and theContextis built one level down at the only place that genuinely needs one — tokio'sInterval::poll_tick.No behaviour change; the
Contextis made from the same waker. After this the onlyContexts inezare theAsyncRead/AsyncWriteimpls, whose signatures are tokio's.The poll bridge moves onto
kio::Parkkio 0.5.3 released
Park— theWaiterCellof moq-dev/moq#2560, renamed — so the hand-rolled retention inParkedgoes away. It wrapsParkand inherits the reuseParkdoes: a steady-state cell allocates nothing, where this one built a freshWaiterandArcon every poll.What stays is the release-on-
Ready, and deliberately so.Parkholds its waiter until the next poll or until it drops — right for a pending operation, wrong for a finished one, where the stream is done with that caller and holding its waker pins the polling task's allocation until something polls the cell again. That was a review finding on #353, so I checked rather than assumed: neuteringsettle(i.e. using barePark) failsa_finished_read_releases_the_poller. The twowaiters.rscopies still lose ~40% of their lines.0.5.3 also made
WaiterClone, which retires the two-step dance in theAsyncRead/AsyncWriteimpls. They took that shape only because the cell's borrow could not be ended before the&mut selfpoll; a cloned waiter shares the parked one's identity, soholdhands one out and the borrow ends there.Still outstanding
AcceptWaitersremains duplicated across the four backends. moq-dev/moq#2604 adds it upstream askio::Fan; when that releases, those copies collapse to an import. APark::release()upstream would remove the rest ofParkedtoo — right now reassigning the cell is the only way to drop its waiter.Verified
Full suites for quinn, quiche, noq and iroh, clippy
--all-targets --all-features -D warnings, andcargo fmt --check.🤖 Generated with Claude Code
(written by Opus 5)