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

### 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 | 🔄 **reopened with a corrected design.** The measured variant handed back `&mut Vec`, which cannot write into consumer-owned memory |
| **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` sink API | Value handoff | 🔄 **reopened, redesigned** | Prior `&mut Vec` variant: `DefaultRowWriter` −2.0% to −4.9% on wide PLP but **+20.9% at 128 B**; FFI binary −5.3/−6.8%; **FFI nvarchar −21.6/−23.4%** | Redesigned around `*_destination -> Option<&mut [u8]>` with default impls, so `DefaultRowWriter` is excluded by construction. Needs re-measurement on current `main` |
| #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 | **reopened** — redesigned as an opt-in sink API for external consumers (PostgreSQL FDW); `DefaultRowWriter` explicitly out of scope |
| #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](https://github.com/microsoft/mssql-rs/issues/247#issuecomment-5276270006) for method, per-commit attribution, and caveats.

```mermaid
flowchart TD
D252["#252 · TdsPacketReader RPITIT
LANDED · −61.6%"]
D256["#256 · remove per-row tracing
~0%, near-zero risk"]
D255["#255 · NBCROW bitmap on stack
−1.4% on NBCROW rows"]
D251["#251 · SqlTypeDecode RPITIT
LANDED · API hygiene"]
D258["#258 · cleanups
PLP validator, cfg gates"]
D253["#253 · RowWriter sink API
reopened, redesigned"]
D249["#249 · fused ColumnDecodeSpec
REFUTED — +12.8%, closed"]
D269["#269 · converge decode onto decode_into
REFUTED on #264 — +21.5%, closed"]
D257["#257 · drop &mut dyn RowWriter
LANDED in #264"]
BUF["Buffering axis
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
```

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

1. **Dispatch first, specifically #252 — done, and it delivered.** `SqlTypeDecode::decode` is called once per *column*, but `TdsPacketReader::read_*` is called once per *read*, so #251 moved nothing on its own while #252 moved almost everything. Per-commit attribution showed #252 alone accounting for −61.6% of a −58.5% total. **PR #264 has now merged this axis** (#251 + #252 + #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 #264**, not against pre-#264 `main`, or it will be measuring headroom that is already gone — see the #269 cautionary case above.
2. **Value handoff — attempted, refuted, and reopened with a corrected design.** The first attempt replaced the issue's original `reserve(col, n) -> &mut [u8]` shape with `begin_value(..) -> Option<&mut Vec>` at design review, on the grounds that a slice destination forces a zero-fill. **That rejection was the error.** The driving consumer is a PostgreSQL FDW that allocates a `varlena` via `palloc` — uninitialised, so there is no zero-fill — and needs the decoder to write into *that* memory; a `&mut Vec` on the Rust heap structurally cannot. The measured design was therefore incapable of delivering the goal, and its failure does not refute the slice-based one. #253 now carries the corrected design: `*_destination -> Option<&mut [u8]>` plus borrowed `write_*_ref`, every method with a default impl. The [decision rule](https://github.com/microsoft/mssql-rs/issues/253#issuecomment-5303846419) and [results](https://github.com/microsoft/mssql-rs/issues/253#issuecomment-5304059024) from the refuted attempt remain on the issue.
3. **Precomputation only pays if it removes work — and #249 proved that necessary condition is not sufficient.** The first attempt (#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. #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](https://github.com/microsoft/mssql-rs/issues/249#issuecomment-5283595345). **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 #249 was then salvaged as **#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 #264**, because #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. **#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 **#289**.

**The first #253 attempt was refuted, and both the mechanism this tracker gave for it and the design it measured were wrong — worth reading before re-implementing.** The earliest note claimed the accumulator must cost `DefaultRowWriter` (+3% to +13%) because "`reserve()` must zero-fill." That holds only for a `Vec`-backed writer; the consumer motivating this axis writes into `palloc`-ed memory, where nothing is zeroed. The implemented `&mut Vec` design removed the zero-fill outright and `DefaultRowWriter` **still** did not gain: **−2.0% to −4.9% on 64 KiB PLP** (the −2.0% cell overlapped in 4/4 rounds with mixed signs) and **+20.9% on 24×128 B PLP**, non-overlapping in all four rounds. The structural reason: **`take_row()` is `std::mem::take`, so every `Vec` is moved out to the caller each row; `commit_value` must therefore `mem::take` its scratch buffer, which starts at capacity 0 and reallocates on every value.** No cross-row reuse is reachable without a breaking `next_row()` ownership change — which is why the reopened design excludes `DefaultRowWriter` by construction instead of trying to make it gain. Findings that survive and must carry forward: the PoC's −14% to −30% for contiguous writers **did not generalize** — FFI *binary* came in at only −5.3/−6.8% while FFI *nvarchar* reached −21.6/−23.4%, isolating that win to **removing the transcode staging copy**; a Windows allocator probe showed the `alloc_zeroed` vs `with_capacity` advantage **peaks around 8–64 KiB (−34% to −41%) and collapses to −1.9% by 256 KiB**, so 64 KiB benchmarks sit near the top of that curve and overstate it; and a **+4.1% regression on the untouched non-PLP FFI control** — which added no accumulator calls at all, and which `DefaultRowWriter` did not show (−0.7%) — was never isolated and **must be reproduced or ruled out** by any new implementation. **One separable win remains unbuilt:** `wants_values() -> false` alone drove the discard cells to −10.2%/−15.7%; it needs a single trait method with a default impl and leaves `DefaultRowWriter` unchanged.

**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 #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](https://github.com/microsoft/mssql-rs/issues/247#issuecomment-5276270006) 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.** #255, #256 and #258 remain unstarted, and #253 has been reopened with a corrected design; 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.

Treat this as a tracker rather than a self-contained change: read the benchmark results comment and the current states of #253, #255, #256, and #258, alongside merged PR #264. Start by measuring any selected axis on current main; done means the corresponding sub-issue has an agreed design, current benchmark evidence, tests where required, and a separate landed change.

Written by the indexing model from the issue text.