You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Sub-issue of #247. Raised during review of #275, with prior spike measurements attached so this does not get re-litigated from first principles.
Problem statement
CancelHandle::run_until_cancelled (mssql-tds/src/core.rs:43-57) keeps f live across two distinct suspend points:
match cancel_handle {Some(handle) => match handle.cancel_token.run_until_cancelled(f).await{Some(result) => result,None => Err(OperationCancelledError("Request was cancelled".to_string())),},None => f.await,}
Because f can be awaited in either arm, the generated state machine reserves room for both, costing roughly one extra copy of the inner future. Measured decomposition at the receive_row_into call site with W = dyn RowWriter + Send:
The outer wrapper adds 1,424 B on top of tokio-util's. This is on the per-row path, and #[async_trait] expands to Box::pin(async move { .. }) per call, so the size sets the per-row allocation size.
Proposed solution
Collapse to a single await point, for example by resolving to one CancellationToken up front and using a never-cancelled token when cancel_handle is None, at the cost of an Arc bump on the no-handle path. An alternative that avoids even that: pin f once before the match and pass f.as_mut() into CancellationToken::run_until_cancelled, keeping the direct None => f.await arm.
Affected crate
mssql-tds
Alternatives considered
Do it as a standalone performance change. Already prototyped and measured; currently a no-go on its own. The pin-once variant on post-#264main (7a9c0b93):
Size: 4,280 B -> 2,912 B (-1,368 B, -32.0%), landing essentially on the predicted ~2,904 B.
No handle: median +12.2 ns/row (0.82% slower), 5/7 processes slower.
Live uncancelled handle: median +6.8 ns/row (0.43% slower), 4/7 slower, per-process range -42.7 to +75.7 ns/row (sign flips across processes).
Old-vs-old A/A control: median -0.8 ns/row, so the harness itself is tight and the swing above is code-layout sensitivity, not harness noise.
Two other shapes were worse in smoke runs: a unified biased tokio::select! (+184.8 ns/row no-handle, +35.3 live) and a manual poll loop (+20.7 / +173.1).
Assume the allocation shrink pays for itself. It should not be assumed. Shrinking the future shrinks the Box::pin allocation but does not remove it; removing the per-row allocation is #265's territory, not this one.
Additional context
Why this is still worth recording despite the flat result:
The size reduction is real, reproducible, and matches prediction to within 8 bytes.
Any future attempt should re-measure with paired ABBA across multiple fresh processes and report the A/A control alongside, because single-process before/after runs on this path are dominated by layout effects.
Note the wrapper size varies slightly by head: 4,280 B at 7a9c0b93, 4,328 B at 1aaf17f5. Same ballpark, different baselines.
Behavior must be preserved exactly: tokio-util gives pre-cancelled tokens priority before the first poll, then polls the inner future first, so a simultaneously-ready value or error wins over cancellation. Cancellation must stay wrapped around every potentially blocking packet read; moving checks to row boundaries only is a separate and much riskier change.
Sub-issue of #247. Raised during review of #275, with prior spike measurements attached so this does not get re-litigated from first principles.
Problem statement
CancelHandle::run_until_cancelled(mssql-tds/src/core.rs:43-57) keepsflive across two distinct suspend points:Because
fcan be awaited in either arm, the generated state machine reserves room for both, costing roughly one extra copy of the inner future. Measured decomposition at thereceive_row_intocall site withW = dyn RowWriter + Send:receive_row_into_internalCancellationToken::run_until_cancelled(inner)CancelHandle::run_until_cancelled(None, inner)The outer wrapper adds 1,424 B on top of tokio-util's. This is on the per-row path, and
#[async_trait]expands toBox::pin(async move { .. })per call, so the size sets the per-row allocation size.Proposed solution
Collapse to a single await point, for example by resolving to one
CancellationTokenup front and using a never-cancelled token whencancel_handleisNone, at the cost of anArcbump on the no-handle path. An alternative that avoids even that: pinfonce before the match and passf.as_mut()intoCancellationToken::run_until_cancelled, keeping the directNone => f.awaitarm.Affected crate
mssql-tds
Alternatives considered
Do it as a standalone performance change. Already prototyped and measured; currently a no-go on its own. The pin-once variant on post-#264
main(7a9c0b93):IntNcolumns.Two other shapes were worse in smoke runs: a unified biased
tokio::select!(+184.8 ns/row no-handle, +35.3 live) and a manual poll loop (+20.7 / +173.1).Assume the allocation shrink pays for itself. It should not be assumed. Shrinking the future shrinks the
Box::pinallocation but does not remove it; removing the per-row allocation is #265's territory, not this one.Additional context
Why this is still worth recording despite the flat result:
Box<dyn TdsTransport>from the row path to unlock writer devirtualization #265 lands, since the caller-visible size is currently hidden byBox<dyn TdsTransport>and#[async_trait].Note the wrapper size varies slightly by head: 4,280 B at
7a9c0b93, 4,328 B at1aaf17f5. Same ballpark, different baselines.Behavior must be preserved exactly: tokio-util gives pre-cancelled tokens priority before the first poll, then polls the inner future first, so a simultaneously-ready value or error wins over cancellation. Cancellation must stay wrapped around every potentially blocking packet read; moving checks to row boundaries only is a separate and much riskier change.