Remove `Box<dyn TdsTransport>` from the row path to unlock writer devirtualization

[saurabh500]

Problem statement

#257 made the row-decode chain generic over the RowWriter (W: RowWriter + Send + ?Sized
through receive_row_into_internal → drive_row_columns → decode_or_decrypt_column →
GenericDecoder::decode_into). Measurement showed that change delivers ~0% in
production, and the reason is structural rather than incidental: the writer's concrete
type is destroyed before it ever reaches that chain.

mssql-js      BinaryRowWriter (concrete)  ─┐
mssql-py-core PyRowWriter     (concrete)  ─┤
                                           v
ResultSet::next_row_into(&mut (dyn RowWriter + Send))     tds_client.rs
  -> TdsClient::get_next_row_into(&mut (dyn RowWriter))
  -> self.transport.receive_row_into(..., writer)
     ^^^^^^^^^^^^^^  transport: Box<dyn TdsTransport>     tds_client.rs:139   <== ERASURE
  -> NetworkTransport::receive_row_into(&mut (dyn ...))
  -> receive_row_into_internal(...)                       token_stream.rs

Both FFI consumers already hold a concrete writer and only coerce to dyn because the
signature demands it. The blocker is one level lower:

pub struct TdsClient { pub(crate) transport: Box<dyn TdsTransport>, ... }   // :139
pub(crate) trait TdsTransport: TdsTokenStreamReader + Send + Sync + Debug   // :20

TdsTokenStreamReader is used as a trait object transitively, as a supertrait of the boxed
TdsTransport. (A grep for dyn TdsTokenStreamReader finds nothing, which is why this is
easy to miss — the boxing is spelled Box<dyn TdsTransport>.) Below that boundary W can
only ever instantiate as dyn RowWriter + Send.

Worth stating plainly: in a release build the box has exactly one possible concrete
type. NetworkTransport is the only non-test implementor — MockTransport is
#[cfg(fuzzing)], TokenReplayTransport and ByteStreamTransport are test-only, and
TestTransport lives inside a private #[cfg(test)] mod tests. The erasure exists for
testability, not for production polymorphism. create_transport even returns a concrete
Box<NetworkTransport> already (network_transport.rs:395); the single erasing cast is at
tds_connection_provider.rs:579.

The measured size of the prize

From the #257 benchmark (20,000-row passes, median of 9, median of 3 runs; full method in
PR #264). The contiguous-buffer writer — the shape mssql-js's BinaryRowWriter uses —
measured twice on each branch, once with the concrete writer (mono) and once with an
explicit as &mut (dyn RowWriter + Send) coercion (dyn). Before #257 the two are
identical by construction, so they serve as a noise control:

Branch	mono (ms)	dyn (ms)	mono advantage
main	140.706	139.141	−1.1% (control)
+#251	140.852	140.605	−0.2% (control)
+#252	45.027	45.889	+1.9% (control)
+#257	34.989	43.759	+20.0%

Three controls land within ±2%; #257 opens a 20.0% gap on a 39 INT + 9 VARCHAR(6) row.
That 20.0% is real and already implemented — it is simply unreachable by any production
caller.

Caveat on what that number is. The harness enters at receive_row_into_internal with
an in-memory reader, i.e. below the transport boundary. That is precisely why it can
isolate the writer gap, but it also means 20.0% is the size of the prize measured at the
layer where it would be realized — not an end-to-end re-measure through
ResultSet::next_row_into. No such re-measure has been done on either spike branch below;
only the structural precondition (concrete writer end-to-end, zero dyn RowWriter in
tds_client.rs) has been verified.

The figure also has a shelf life. It was measured on a control branch against one row
shape (39 INT + 9 VARCHAR(6)). If the row path changes shape before this lands, re-measure
rather than citing the table above — an issue that sits open for months is exactly where a
cached number goes stale unnoticed.

Spike results: both candidate designs built to completion

Both were implemented against #264's head (cfa0dfe7), compiled, and run through the full
test suite.

(A) Generic client — TdsClient<T: TdsTransport = NetworkTransport>, default type
parameter so existing TdsClient spellings keep working.

(B) Enum dispatch — transport: AnyTransport, where AnyTransport is
Network(Box<NetworkTransport>) plus a #[cfg(any(test, feature = "test-util", fuzzing))]
Dyn(Box<dyn TdsTransport>) arm. Row fetch is exposed as inherent generic methods
(receive_row_into_mono<W> / resume_row_into_mono<W>) on both the enum and
NetworkTransport, so TdsTransport itself stays dyn-compatible and every test double
keeps working unchanged through the Dyn arm.

	(A) generic TdsClient<T>	(B) AnyTransport enum
cargo check --lib / --all-targets	0 errors	0 errors
Privacy leaks (private_interfaces/private_bounds)	10 warnings	0
cargo clippy -D warnings	fails	passes
cargo fmt --check	—	passes
--cfg fuzzing leg	—	0 errors
Internal types dragged into public API	6	0
reconnect redesign required	yes (new reconnect_in_place trait method)	no
Turbofish sites forced on callers	5	0
FFI source changes	none	none
Tests	1710 run / 1703 passed / 7 failed	identical
Diff	8 files, +152 / −104	8 files, +417 / −22 (306 = new file)

The 7 failures are pre-existing on main and require a live SQL Server; the failure set is
byte-identical to baseline on both branches.

Recommendation: (B). The decisive axis is the privacy leak. Making a pub struct
generic over a pub(crate) trait drags the internal type graph into the public API —
SslHandler, NetworkWriter, SessionRecoveryData, NegotiatedSettings,
ExecutionContext, and TdsTokenStreamReader all become "more private than" public items.
That is 10 warnings, and since CI runs cargo clippy -D warnings, design (A) does not
build in CI without either sealing the trait or adding six #[doc(hidden)] pubs.

(B) also sidesteps the reconnect trap entirely. check_and_reconnect has 21 call sites
across the query surface; under (A), specializing reconnect to NetworkTransport compiles
locally but fails at the callers, and fixing that virally pins every query method to a
concrete transport. Under (B), reconnect is just an assignment of AnyTransport::Network.

(B)'s one genuine cost is extensibility — a new transport needs a new variant. The Dyn
arm neutralizes this, and it is cfg-gated so a release build is a single-variant enum.

Two claims in the original issue text that the spikes falsified

1. "All three FFI crates name TdsClient, so this ripples into mssql-js,
   mssql-py-core, and mssql-odbc." — False for both designs. A default type
   parameter (T = NetworkTransport) keeps every bare TdsClient spelling valid, and (B)
   does not change the type at all. Both spikes compile mssql-js with zero source
   changes.
2. "Enum dispatch still has to thread a concrete W through receive_row_into, which
   means TdsTransport can no longer be dyn-compatible in that method." — Avoidable.
   Adding the generic row-fetch as an inherent method rather than a trait method keeps
   the trait dyn-compatible, which is what allows the Dyn fallback arm to exist at all.

Also worth recording: switching Box<dyn TdsTransport> → a concrete type produces ~27
E0599 "method not found" errors that look like a broken impl but are not. A dyn Trait
exposes its trait's methods without the trait being in scope; a concrete type does not.
The fix is use ...TdsTransport as _; at the call sites.

Scope correction: this removes the indirect call, not the allocation

Removing the box devirtualizes the writer. It does not remove per-row heap
allocation, and anyone reading this issue as "no more boxing per row" would be wrong.

#[async_trait] desugars to Box::pin(async move { ... }), so today the caller's state
holds only a pointer. Replacing it with a native async fn inlines the whole decode state
machine into every caller — and the repo's own row_fetch_futures_stay_small guard
(tds_client.rs:4418, budget 4096 B) catches it. Measured on spike (B) before the fix:

Future	baseline	unboxed
next_row_cursor	1128	5584
read_row_column	376	4944
drain_rows	1408	5848
get_next_row_into	1160	5600

All four blow the budget; worst is +1215%. The fix is a single Box::pin on the concrete
arm, which restores all four while keeping writer devirtualization. That line is
load-bearing — it costs exactly the one allocation per row that exists today, so it is
parity rather than regression, but it means the allocation survives this change.

Worth noting the guard catches this for free. It was added for #225 — a large local held
across an .await in drain_active_plp — and trips on this unrelated mechanism as a side
effect. Whoever implements this gets a tripwire they did not have to build.

Adjacent finding: ~70% of the row future is cancellation plumbing

Found with cargo +nightly rustc -- -Zprint-type-sizes while investigating the above.
Chain for the DefaultRowWriter instantiation:

Layer	bytes	Δ
receive_row_into_internal (the actual decode)	1328	—
+ tokio_util CancellationToken::run_until_cancelled	2744	+1416
+ core::CancelHandle::run_until_cancelled	4088	+1344
+ tokio::time::Timeout	4200	+112
+ receive_row_into_mono	4416	+216

Real decode state is ~1.3 KB; cancellation adds ~2.8 KB. Cancellation is applied twice,
because CancelHandle::run_until_cancelled delegates to
CancellationToken::run_until_cancelled. The mechanism is visible in core.rs:50-56 —
f is awaited in both match arms:

Some(handle) => handle.cancel_token.run_until_cancelled(f).await,  // stores f
None => f.await,                                                    // stores f again

Those are separate suspend points in separate arms, so rustc stores both rather than
overlapping them. The arithmetic corroborates it exactly: 2744 + 1328 = 4072 ≈ the measured
4088.

This is likely worth more than this issue — but not for the reason the byte table
suggests. The harness was extended with a paired wrapper ladder to price it, and the
timings invert the size picture:

Mode	median ms	vs bare
bare	32.811	—
run_until_cancelled(None, ..)	34.709	+5.8%
run_until_cancelled(Some(&h), ..)	34.415	+4.9%
+ tokio::time::timeout(..)	46.751	+42.5%

So cancellation is ~70% of the future by size but only ~5% by time, while the timeout is
+112 B but ~36% of the cost. Size and time are anti-correlated here. The per-row
tokio::time::timeout — a timer arm/disarm for every row — is the real overhead, and it is
larger than the 20.0% this issue chases.

The size still matters independently: at ~1.4 KB the row future fits the 4096 B guard
unboxed, which would remove the per-row allocation outright rather than merely preserving
it as this issue does.

Tracked separately in #271 rather than folded in here; the two compose, since this issue
removes an indirect call and that one removes wrapper overhead.

Affected crate

mssql-tds. Under either design the FFI crates need no source changes.

Alternatives considered

Doing nothing. The chain below the transport boundary is already generic after #257, so
this work is purely about removing the erasure — no decoder changes are needed. Leaving it
means permanently forgoing a measured 20.0% on the FFI-shaped writer path.

A third option — splitting row fetch into a separate non-dyn trait — was not spiked, since
(B) achieves the same effect with inherent methods and no new trait.

Additional context

Follow-up to #257 (PR #264), part of #247. Depends on #264 landing first; both spikes were
built against cfa0dfe7.

Four open PRs touch token_stream.rs and will interact with this work: #238 (+441/−4,
ready), #245 (+209/−26), #186 (+142/−28), #215 (+89/−28). #245 is the notable one — its
NBCROW null-bitmap rework is a semantic overlap with the NBCROW benchmark cases, not just
a textual merge conflict.

Implementation notes for whoever picks this up, each of which cost real iterations to
discover: the Box::pin on the concrete arm is required, not stylistic; cargo bclippy
does not set --cfg fuzzing, and that leg catches breakage the default one misses; and
trait methods must be imported with use ... as _; once the receiver stops being a trait
object.

[saurabh500]

Line-ref re-pin against current main (4eafd956), and a heads-up on the guard entry count

Two maintenance notes. Neither changes the proposal.

1. One stale line reference. main has moved since this was filed (same #148 rebase that shifted the refs in #272/#273):

Reference in the body	Filed as	Current main
row_fetch_futures_stay_small guard	tds_client.rs:4418	tds_client.rs:4980
transport: Box<dyn TdsTransport>	tds_client.rs:139	:139 — unchanged
create_transport concrete return	network_transport.rs:395	:395 — unchanged
erasing cast	tds_connection_provider.rs:579	:579 — unchanged

Only the guard moved; the three structural refs are still exact.

2. The guard will have five entries, not four, if #275 lands first.

The body says "All four blow the budget" and lists four futures. PR #275 adds a fifth
to row_fetch_futures_stay_small:

next_row_cursor        1128        drain_rows              1408
read_row_column         376        get_next_row_into       1160
                                   read_active_plp_chunk    216   <- added by #275

That 216 B is small for exactly the reason this issue exists, so expect it to grow like
the other four. TdsTokenStreamReader is #[async_trait] (token_stream.rs:192) and the
client holds Box<dyn TdsTransport>, so today read_active_plp_chunk's future stores a
pointer at the transport boundary rather than the callee's frame. Removing the boxing —
this issue — inlines that frame into the caller, which is the same mechanism that took the
other four from 376–1408 B to 4944–5848 B on spike (B).

So the implementation note stands but the count changes: five Box::pin sites on the
concrete arm, not four. 216 B is not evidence that the PLP path is exempt; it is a
pre-removal number and this issue is the removal.

Nothing here is a blocker — recording it so whoever picks this up isn't surprised by a
fifth assertion failing, and doesn't read the 216 B as headroom.