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
#[instrument] on an async fn wraps the returned future in Instrumented<F>. That has two costs beyond log formatting:
It enlarges the async state machine.
It enters and exits a span on every poll — not once per call.
On a per-row entry point that is millions of span transitions per scan. This matters because it means level filtering alone does not help — the span machinery runs regardless of whether any subscriber is listening. The attribute itself has to go.
The per-row and per-token info! / debug! statements are a smaller, separate cost.
Proposed solution
Remove #[instrument] from per-row entry points.
Downgrade the per-row/per-token log statements to trace! rather than deleting them.
Feasibility: ✅ proven. Prototyped in 56a836f6, green on all six configs in #247.
Findings
There are 5 sites, not the 3 the deck claims:
Site
What
tds_client.rs:3076
#[instrument]
tds_client.rs:3943
#[instrument]
tds_client.rs:3133
info!("Row Received")
tds_client.rs:3400
info!("Row Received")
token_stream.rs:371
debug!("Parsing token type")
token_stream.rs:597
debug!("Parsing token type")
PR POC: reduce per-row cost in the TDS row decode path #238 deletes these outright. Recommend downgrading instead. Moving info!/debug! → trace! keeps the diagnostics available under RUST_LOG=trace at effectively zero cost when disabled. Losing per-token tracing would be a real debuggability regression for protocol work, and it is the first thing anyone reaches for when debugging a malformed token stream.
Only the #[instrument] attributes must be removed outright, since their cost is not level-gated.
Affected crate
mssql-tds
Alternatives considered
Keep #[instrument] and rely on level filtering. Does not work — see above. Instrumented<F> costs are paid at poll time regardless of subscriber state.
Delete the log statements entirely (what #238 does). Rejected — the cost of a disabled trace! is negligible, and per-token visibility is genuinely useful.
Additional context
This is the lowest-risk item in #247 and a reasonable first PR for whoever picks up this work.
Nothing here is benchmarked. The deck attributes 0.390s to this change; confirm independently.
Sub-issue of #247 (axis: Precomputation).
Problem statement
#[instrument]on anasync fnwraps the returned future inInstrumented<F>. That has two costs beyond log formatting:poll— not once per call.On a per-row entry point that is millions of span transitions per scan. This matters because it means level filtering alone does not help — the span machinery runs regardless of whether any subscriber is listening. The attribute itself has to go.
The per-row and per-token
info!/debug!statements are a smaller, separate cost.Proposed solution
#[instrument]from per-row entry points.trace!rather than deleting them.Feasibility: ✅ proven. Prototyped in
56a836f6, green on all six configs in #247.Findings
There are 5 sites, not the 3 the deck claims:
tds_client.rs:3076#[instrument]tds_client.rs:3943#[instrument]tds_client.rs:3133info!("Row Received")tds_client.rs:3400info!("Row Received")token_stream.rs:371debug!("Parsing token type")token_stream.rs:597debug!("Parsing token type")PR POC: reduce per-row cost in the TDS row decode path #238 deletes these outright. Recommend downgrading instead. Moving
info!/debug!→trace!keeps the diagnostics available underRUST_LOG=traceat effectively zero cost when disabled. Losing per-token tracing would be a real debuggability regression for protocol work, and it is the first thing anyone reaches for when debugging a malformed token stream.Only the
#[instrument]attributes must be removed outright, since their cost is not level-gated.Affected crate
mssql-tds
Alternatives considered
Keep
#[instrument]and rely on level filtering. Does not work — see above.Instrumented<F>costs are paid at poll time regardless of subscriber state.Delete the log statements entirely (what #238 does). Rejected — the cost of a disabled
trace!is negligible, and per-token visibility is genuinely useful.Additional context
This is the lowest-risk item in #247 and a reasonable first PR for whoever picks up this work.
Nothing here is benchmarked. The deck attributes 0.390s to this change; confirm independently.