Row decode performance: productionize PoC #238 across four axes (tracking)

[saurabh500]

Problem statement

PoC PR #238 (NivasSA:poc/tds-row-decode-optimizations) reports 47.4% lower wall clock and ~65% lower CPU on a 1.5M-row × 48-column scan. The wins are real and have since been independently reproduced. But it is a proof of concept, and it is not landable as-is.

What it is not: the PoC is often described as "adding sans-I/O". It is not. Its buffered_slice helper documents its own contract as "Callers treat an empty slice as not enough data and fall back to the async path" — the I/O-coupled decoder must still exist as a permanent fallback.

What makes it unlandable in its current form, beyond being +1516/−168 from an external fork with a pending CLA:

• a POC_README.md at the repo root
• PLP validation weakened from 6 checks to 2, with the constants re-declared locally
• a third copy of the decode type switch
• a dead decode_op_into, and an NBCROW optimization that is only half-implemented (the bitmap fix exists only on the buffered fast path)
• pre-rebase benchmark numbers
• zero tests for its main fast path, against a repo that gates on 85% diff coverage

The wins are not one change. They are four independent axes with very different risk profiles, and separating them is the whole point of this issue.

Axis	What is redundant per row	Sub-issues	Status
Dispatch	A heap-allocated future per column read; dyn per column	#251, #252, #257	✅ Landed via PR #264. −61.6% — nearly the entire win
Precomputation	Re-deriving invariant metadata facts every row	#249, #255, #256	#255/#256 landed as designed. #249 built, measured, refuted — closed NOT_PLANNED
Value handoff	Buffers allocated then copied 2–3 times per string/blob	#253	Sign depends on the writer
Buffering	Per-packet async plumbing on already-buffered bytes	deferred	See below

Benchmark arithmetic for the Dispatch axis: 39 IntN + 9 ShortString columns ≈ 96 reads/row × 1.5M rows ≈ ~144M boxed-future allocations in a single scan.

Proposed solution

Land the axes as separate PRs, tracked by the sub-issues below. Every item except #257 has been prototyped and compiles; each sub-issue records its own evidence, blockers, and design decisions so the PR starts from a known-good design rather than a guess.

Sub-issue	Axis	Feasibility	Measured	Notes
#252 — TdsPacketReader → RPITIT	Dispatch	✅ landed (#264)	−61.6%	Nearly the entire win
#253 — RowWriter reserve/commit accumulator	Value handoff	✅ proven	−14% to −30% for contiguous writers; +3% to +13% for DefaultRowWriter	Public trait, 3 language bindings; needs an mssql-js benchmark
#256 — remove per-row tracing overhead	Precomputation	✅ proven	~0%	Lowest risk
#255 — NBCROW null bitmap on the stack	Precomputation	✅ proven	−1.4% on NBCROW only
#251 — remove #[async_trait] from SqlTypeDecode	Dispatch	✅ landed (#264)	~0% (within noise)	Called 1×/column, not 1×/read. Landed for API hygiene and as the decode_boxed recursion break
#249 — fused ColumnDecodeSpec	Precomputation	❌ built, measured, refuted	+12.8% on INT-heavy (non-overlapping); ~0% on mixed	Closed NOT_PLANNED. The modelled −6.0% did not survive contact with the real path. Convergence salvaged as #269
#257 — remove &mut dyn RowWriter per column	Dispatch	✅ landed (#264)	folded into #264's −61.6%
#258 — cleanups surfaced along the way	—	—	—	Independent; some reduce cost of the above

Status as of 2026-08-14. The dispatch axis has landed; the precomputation axis is closed as refuted.

	State
PR #264 — Remove per-row dispatch overhead	✅ MERGED (7a9c0b93). Covers #251 + #252 + #257. Delivered the −61.6%
PR #269 — Converge decode onto decode_into via CaptureWriter	❌ closed, not landed. Neutral on main, but +21.5% on INT-heavy once rebased on #264 — it handed back ~13.5% of #264's gain. A cold-helper fix halved it to +8.8%; still short. See below
#251, #252, #257	closed COMPLETED — landed in #264
#254	closed NOT_PLANNED — superseded by #249
#249	closed NOT_PLANNED — built, measured, refuted. Closes the precomputation axis
#253, #255, #256, #258	open, unstarted

⚠️ Anything measured before #264 must be re-measured on top of it. #269 is the cautionary case: it measured neutral-to-favourable against main and became a +21.5% regression once #264 removed the boxing that had been masking it. #264 shrank the INT-heavy baseline from ~283 ms to ~109 ms, so a fixed per-cell cost that was 2% of the old baseline is ~6% of the new one. Pre-#264 measurements systematically understate cost.

Ordered by measured value. See the benchmark results comment for method, per-commit attribution, and caveats.

flowchart TD
    D252["#252 · TdsPacketReader RPITIT<br/>LANDED · −61.6%"]
    D256["#256 · remove per-row tracing<br/>~0%, near-zero risk"]
    D255["#255 · NBCROW bitmap on stack<br/>−1.4% on NBCROW rows"]
    D251["#251 · SqlTypeDecode RPITIT<br/>LANDED · API hygiene"]
    D258["#258 · cleanups<br/>PLP validator, cfg gates"]
    D253["#253 · RowWriter reserve/commit<br/>sign depends on the writer"]
    D249["#249 · fused ColumnDecodeSpec<br/>REFUTED — +12.8%, closed"]
    D269["#269 · converge decode onto decode_into<br/>REFUTED on #264 — +21.5%, closed"]
    D257["#257 · drop &amp;mut dyn RowWriter<br/>LANDED in #264"]
    BUF["Buffering axis<br/>deferred"]

    D258 -->|"trusted Exact length hint"| D253
    D251 --> D257
    D252 --> D257
    D249 -.->|"convergence salvaged, then refuted"| D269
    D253 --> BUF

    style D249 stroke-dasharray: 4 4
    style D269 stroke-dasharray: 4 4

Loading

Ordering rationale, which follows the measurements rather than the PoC's own framing:

1. Dispatch first, specifically Convert TdsPacketReader to RPITIT to remove per-read boxing (row decode perf, axis: dispatch) #252 — done, and it delivered. SqlTypeDecode::decode is called once per column, but TdsPacketReader::read_* is called once per read, so Remove #[async_trait] from SqlTypeDecode (row decode perf, axis: dispatch) #251 moved nothing on its own while Convert TdsPacketReader to RPITIT to remove per-read boxing (row decode perf, axis: dispatch) #252 moved almost everything. Per-commit attribution showed Convert TdsPacketReader to RPITIT to remove per-read boxing (row decode perf, axis: dispatch) #252 alone accounting for −61.6% of a −58.5% total. PR Remove per-row dispatch overhead from the row-decode path #264 has now merged this axis (Remove #[async_trait] from SqlTypeDecode (row decode perf, axis: dispatch) #251 + Convert TdsPacketReader to RPITIT to remove per-read boxing (row decode perf, axis: dispatch) #252 + Investigate removing &mut dyn RowWriter from the per-column path (row decode perf, axis: dispatch) #257) as 7a9c0b93, and an independent harness built for a different purpose corroborated it at −61.4%. Everything downstream must now be measured on top of merged Remove per-row dispatch overhead from the row-decode path #264, not against pre-Remove per-row dispatch overhead from the row-decode path #264 main, or it will be measuring headroom that is already gone — see the Converge decode onto decode_into via CaptureWriter #269 cautionary case above.

2. Value handoff next, but not for its raw number. Add RowWriter reserve/commit accumulator API to remove double/triple buffering (row decode perf, axis: value handoff) #253 is the shared boundary: the writer must serve both a borrowed complete &[u8] and a streamed PLP value without knowing which path produced it. Designing it once, before any buffered path exists, avoids designing it twice. Its measured sign is writer-dependent (see below), so it should land with an mssql-js benchmark rather than a DefaultRowWriter one.

3. Precomputation only pays if it removes work — and Precompute a per-column decode plan (ColumnDecodeSpec) and converge all decode sites on it #249 proved that necessary condition is not sufficient. The first attempt (Precompute a per-column decode plan from COLMETADATA (row decode perf, axis: precomputation) #254, now closed) hoisted wire extent into a plan but left interpretation to the existing ~40-arm type match, so the plan was consulted in addition to the match rather than instead of it. That measured +4.4% — a regression by construction, not by an unfinished implementation, because a Fallback variant guarantees the old match survives to service it. Precompute a per-column decode plan (ColumnDecodeSpec) and converge all decode sites on it #249 corrected exactly that: a fused ColumnDecodeSpec carrying interpretation, no escape hatch, replacing decode_into. It was built, and it still regressed — +12.8% on INT-heavy, non-overlapping. Three candidate causes were each tested and refuted (24-byte spec copy → shrunk to 6 B; async-state growth → futures shrank 77%; a second metadata stream → removed from the hot loop). See the negative-result write-up. The load-bearing conclusion: on a per-cell path this cheap, a precomputed column plan costs more in added dispatch than it saves in removed classification — the classification it removes exists only for string/decimal/date-time columns, while the dispatch it adds is paid by every cell. The precomputation axis is closed.

   The maintainability half of Precompute a per-column decode plan (ColumnDecodeSpec) and converge all decode sites on it #249 was then salvaged as Converge decode onto decode_into via CaptureWriter #269, which defined decode as decode_into + a CaptureWriter and deleted ~430 lines of duplicated type switch. It measured neutral against main — and +21.5% on INT-heavy once rebased on merged Remove per-row dispatch overhead from the row-decode path #264, because Remove per-row dispatch overhead from the row-decode path #264's −61.6% removed the boxing that had been absorbing the cost. Code size was confirmed as a contributor (+13.0% IR lines) and then fully neutralized with a cold #[inline(never)] rare-type helper; the regression halved to +8.8% and stopped there. Converge decode onto decode_into via CaptureWriter #269 was closed rather than landed. The divergence risk it existed to remove — decode and decode_into drifting apart — is better addressed by behaviour-parity tests asserting both paths agree for every type, at zero runtime cost. That is the recommended successor and is now filed as Add exhaustive decode/decode_into behavior-parity tests #289.

#253's sign depends on the writer, and this is the single most misreadable result here. DefaultRowWriter allocates a fresh Vec per value either way, and reserve() must zero-fill — safe Rust cannot hand out uninitialized memory — so the accumulator is added cost there (+3% to +13%). A contiguous writer, which is the shape the FFI bindings actually use, is the opposite (−14% to −30%). Benchmark it against the binding, not the default.

One copy is the floor. The buffered slice points into the transport's working_buffer, which the next packet read overwrites, so the writer must copy. Today a varchar(max) value reaching mssql-js costs three copies; the target is one. True zero-copy would require a visitor-style API incompatible with RowWriter's accumulate-a-row model, and is not proposed.

On the deferred Buffering axis

This axis was originally deferred because it overlapped the in-flight sans-I/O stack (#189–#203). That stack has now been closed and will not be productionized, so the deferral now rests on its own merits, which are stronger:

• The measure pass is not free. A two-pass design pays a pass-1 tax measured at +5.3% / +16.0% / +37.7% depending on schema — most expensive exactly where decoding is cheapest, because a cheap row amortizes the extra walk over less work.
• The headroom has already been taken. The PoC attributes this axis a large share of its win, but that framing predates Convert TdsPacketReader to RPITIT to remove per-read boxing (row decode perf, axis: dispatch) #252. With per-read boxing already gone (−61.6%), what remains for buffering to recover is much smaller than a 16–38% tax can comfortably pay for.
• The PoC's version has a design flaw that is far cheaper to fix before landing than after (below).

If it is revisited, buffering must not be a dead-end bypass. The PoC declines by returning Ok(None) and re-decoding through the async path, which means a row larger than one packet hits the fast path 0% of the time. The decline must instead distinguish "not yet resident, feed me more" from "I cannot handle this type", so a straddling row is retried once more bytes arrive rather than falling off a cliff. That in turn requires a residency cap, which is a security requirement rather than tidiness: pass 1 discovers a row's length only by running past the end of the buffer, so without a cap a hostile server can force unbounded buffer growth.

Feasibility method

Each change was implemented on dev/saurabh/row-decode-perf-feasibility and validated against six configurations:

1. cargo check --workspace --all-targets
2. cargo fmt
3. cargo clippy --workspace --all-features --all-targets -- -D warnings
4. cargo nextest run -p mssql-tds --lib --no-fail-fast
5. cd mssql-py-core; cargo check --all-targets (excluded from the workspace)
6. $env:RUSTFLAGS='--cfg fuzzing'; cargo check -p mssql-tds --lib

All six are green for all six experiments. Tests held at 1703 run / 1696 passed / 7 failed throughout; the 7 failures are certificate_validator (4) and win_tls::validate (3), verified pre-existing on main and unrelated.

Commits: dba753fe (#251), eb64a9d1 (#252), c38e5683 (#253), 56a836f6 (#255 + #256), b0e5132f (the extent-only plan spike, now closed as #254 — still a valid wiring reference for the OnceLock plumbing that #249 needs).

These are throwaway spikes, not merge candidates. The PoC is from an external fork with a pending CLA, so this work will need internal re-landing with attribution.

Affected crate

mssql-tds

Alternatives considered

Take PR #238 as-is. Rejected. Beyond the hygiene problems listed above, it weakens PLP validation from 6 checks to 2, only half-implements its own NBCROW optimization, adds a third copy of the type switch, and ships its main fast path with no tests. The buffering axis in particular has a design flaw — the dead-end decline — that is much cheaper to fix before landing than after.

Wait for the sans-I/O core stack (#189–#203) and build on it. Moot: that stack has been closed. It was 12 draft PRs / ~8,400 lines, none of which reached main. The measurements are the reason it is not needed for this work — #252 is a narrow, mechanical change that captures the dominant win without restructuring the core.

Land it as one PR. Rejected. The axes have independent risk profiles: #255/#256 are near-trivial, #251/#252 are mechanical but wide, #253 changes a public trait across three language bindings, and #249 touches the most contended file in the repo. Bundling them means the riskiest item gates the safest.

Do only #252 and stop. Genuinely defensible. It is −61.6% of a −58.5% total, so almost everything else is rounding error against it. The argument for continuing is that #253 targets a workload #252 does not — wide BLOB/VARCHAR(MAX) traffic through the FFI bindings, where the cost is allocator-bound rather than dispatch-bound — and that #258's PLP hardening is worth doing on its own terms. If appetite is low, ship #252 and #256 and revisit.

Additional context

Benchmark caveat. The reported workload is 39 INT + 9 VARCHAR(6), all nullable. That exercises 2 of ~10 decode shapes and contains no PLP/BLOB data at all. The 29.2% figure the deck attributes to PLP writer sinks comes from a different wide-BLOB workload. Re-benchmark against a schema with wide VARBINARY(MAX)/NVARCHAR(MAX) columns before claiming the #253 win, and against DATETIME2/DECIMAL/UNIQUEIDENTIFIER before claiming #249 generalizes. Test coverage on this work should be driven by the type matrix, not by the benchmark.

Read absolute numbers, not percentages — and prefer driving the real path over modelling it. A supporting harness measured a resolved plan against per-cell classification at −6.0% on the #238 schema, −3.3% on 48 VARCHAR, and −13.8% on 48 IntN. The largest percentage is on the cheapest schema: 48 VARCHAR columns save 291 µs against 35 µs for 48 IntN, an 8.3× gap, which is exactly what a per-cell classification chain predicts. intn48's 13.8% is an artefact of a 34× smaller baseline, and quoting it as a percentage invites the opposite conclusion. None of these modelled figures survived contact with the real decoder — see #249, where the same design measured +12.8% once it drove receive_row_into_internal instead of a stand-in. The harness modelled the classification the plan removes but not the dispatch it adds, which is the term that decided the result. Treat model-derived deltas on this path as hypotheses, not evidence.

⚠️ Which metrics predict decode throughput here — and which don't

Two changes (#249, #269) were each driven through several rounds of diagnosis. Every intuitive proxy for "this made the hot path heavier" was measured, and most of them were wrong. Recording this so the next person doesn't re-derive it:

Proxy	Verdict	Evidence
Future size (size_of the generated future)	❌ wrong, three times	#249 shrank decode_into's future 928 → 216 B (−77%) and got slower. #269's futures were 37–42% smaller than main's (872/712 B vs 1392/1232 B) while running 21% slower.
Forced inlining (#[inline(always)])	❌ no effect	Did not recover #269's regression. Note the attribute lands on the outer async fn, not the generated poll body, so it is a weak instrument here regardless.
Struct size of a per-cell value	❌ insufficient	#249's ColumnSpec went 24 B → 6 B with a const assert. Regression halved, persisted.
Generated code size (IR lines / symbol bytes of the concrete instantiation)	⚠️ partial	#269's hot decode_into grew 5,137 → 5,805 IR lines (+13.0%). Driving it back to 5,101 — below main — recovered only half the regression (+21.5% → +8.8%).
Number of match arms	❌ wrong premise	"40 arms = 40 comparisons" is false. TdsDataType is #[repr(u8)] with sparse wire codes (0x1F–0xF5, ~18% density), so it lowers to a cluster tree with one indirect jump — not a linear chain. The cost of a two-level plan enum is that it adds a second, dependent indirect branch.

Two methodology rules that follow. First, measure by driving receive_row_into_internal over a pre-built in-memory buffer, not by modelling the decode path — every model-derived figure in this issue was later contradicted by the real path. Second, when comparing generic functions, measure the concrete instantiation, not the aggregate: decode_into monomorphized twice will double its llvm-lines total without the hot copy growing at all.

There is a residual nobody has explained. After code size was driven below main's, INT-heavy remained ~6–9% slower. Both #249 and #269 ended the same way: the named quantity reached parity or better, and the regression shrank without vanishing. Anyone attacking this next should expect that, and should not treat a partial recovery as progress toward a full one.

Measured. All six spikes have been benchmarked; see the benchmark results comment for the full table, per-commit attribution, and method. Headline: −58.5% decode time on the PoC's row shape, −69.4% for a contiguous-buffer (FFI-shaped) writer, realised as −61.6% when #264 landed. #249 and #269 have both since been built, measured, and refuted. #253, #255, #256 and #258 remain unstarted; each should carry its own benchmark against its own workload, taken on top of merged #264.

Also out of scope here, tracked separately: the NBCROW null-bitmap Arc allocation (#233, #245), and bypassing the mssql-js string interner for large values.

Repo process reminders for whoever picks these up: draft-first PRs linking the relevant sub-issue; cargo bfmt / cargo bclippy / cargo btest before pushing; CI targets 85% diff coverage; mssql-py-core is excluded from the workspace and needs its own fmt/clippy run.