Skip to content

Precompute a per-column decode plan (ColumnDecodeSpec) and converge all decode sites on it #249

Description

Problem statement

Row decoding re-derives per cell what is invariant for an entire result set, and it does so by chasing pointers into a wide, cold ColumnMetadata struct.

Redundant classification. GenericDecoder::decode_string_into (mssql-tds/src/datatypes/decoder.rs:1716-1723) runs three classification calls per cell, before any I/O:

Call Site Work
get_encoding_type(metadata) datatypes/sql_string.rs:159 matches type_info_variant to extract collation, then is_unicode_type(data_type), then collation.unwrap().utf8()
metadata.is_plp() query/metadata.rs:79 matches! on type_info_variant
Self::is_long_len_type(data_type) datatypes/decoder.rs:1701 matches! on data_type

All three depend only on ColumnMetadata, which is fixed for the whole result set. On a 1.5M-row x 48-column benchmark (39 INT + 9 VARCHAR), that is roughly 40M redundant classifications returning identical answers.

The same shape appears throughout the match:

  • read_decimal(reader, metadata) re-derives precision/scale per cell (decoder.rs:1185, :1189).
  • BigVarBinary re-checks metadata.is_plp() per cell (decoder.rs:1226).
  • TimeN, DateTime2N, and DateTimeOffsetN each call metadata.get_scale() per cell and build an ok_or_else error closure around it (decoder.rs:1290, :1309, :1330). Scale is fixed by the column.
  • An encryption check (crypto_metadata.is_some()) per cell.

Cold pointer chases. metadata is a &ColumnMetadata element of a Vec. Every cell dereferences into a fat struct (user_type, flags, data_type, TypeInfo { tds_type, length, type_info_variant }, name, ...) to read one or two fields. A compact per-column plan is a dense array walked sequentially instead.

Dispatch is a secondary effect, but not free. TdsDataType (datatypes/sqldatatypes.rs:15) has explicit sparse discriminants spanning 0x1F..=0xFE with roughly 40 variants (~18% density), so the match metadata.data_type at decoder.rs:1103 does not lower to a single dense jump table; LLVM emits clustered tables plus compare chains. Hot benchmark types straddle clusters (IntN = 0x26 low, BigVarChar = 0xA7 and NVarChar = 0xE7 high), and the string arm at decoder.rs:1195 is an 8-pattern | arm spanning both. In practice the branch predictor handles this well, because column N is always the same type within a result set, so this is a real but minor cost compared with the redundant classification above.

Duplicated type switches that must agree. SqlTypeDecode::decode (produces ColumnValues) and decode_into (drives RowWriter) are two independent ~40-arm switches over the same type set. They must stay in lockstep by convention only, and a divergence is silent wrong data rather than a compile error.

Proposed solution

Introduce a ColumnDecodeSpec enum plus a per-result-set decode plan, computed once from ColMetadataToken and cached (OnceLock<Vec<ColumnDecodeSpec>>), then convert all decode sites to consume it.

This issue supersedes #254. Both propose "precompute a per-column plan", but they are different designs, and the difference is exactly what determines whether the change pays for itself. See Alternatives considered for the measurement.

Naming. The PoC calls this DecodeOp. "Op" implies an executable instruction; this is inert data that is consulted, not executed. ColumnDecodeSpec is declarative and does not imply behaviour, which also keeps it clear of the existing GenericDecoder / StringDecoder types.

Prefer it over ColumnPlan on readability grounds: plan: ColumnPolicy is already a parameter at 10 sites in the row-decode path (io/token_stream.rs:198, 223, 253, 269, 473, 584, 659, 766, 834), including receive_row_into_internal, drive_row_columns, and resume_row_into_internal — the same functions that would carry the decode plan. (The #254 spike compiled green using ColumnPlan, so this is a naming preference, not a compile blocker. That ColumnPolicy parameter is itself misnamed and would be better as policy, but renaming it is out of scope here.)

Three requirements distinguish this from the PoC's version and from the #254 spike:

1. Cover every TdsDataType, so there is no escape hatch. #238 keeps DecodeOp::Generic and the #254 spike keeps DecodeOp::Fallback. Either one forces the existing ~40-arm match to survive indefinitely as the fallback handler, which is why hoisting extent alone cannot pay for itself. Today the residual _ => arm at decoder.rs:1364 falls back to self.decode(...) plus write_column_value; enumerating what currently lands there, and giving each a spec variant, is part of this work.

2. Carry precomputed payloads, not bare tags. The plan must hold the resolved encoding, width, precision/scale, and length-prefix shape, so the per-cell path performs no classification:

enum ColumnDecodeSpec {
    Fixed     { width: u8, kind: FixedKind },     // Int4, Flt8, DateTime, ...
    VarLenU8  { kind: VarU8Kind },                // IntN / FltN / MoneyN / BitN / Guid / date-time-N
    VarLenU16 { kind: VarU16Kind },               // BigVarChar / BigVarBinary, encoding baked in
    LongLen   { encoding: EncodingType },         // Text / NText
    Plp       { encoding: PlpEncoding },
    Decimal   { precision: u8, scale: u8 },
}

3. Both decode sites converge on it. Point decode and decode_into at the same plan so the two ~40-arm switches collapse to one classification source. This is the main durable win: it deletes an entire class of silent divergence between the ColumnValues and RowWriter paths. The plan-driven path must replace decode_into, not sit beside it — otherwise this adds a third copy of the type switch, which is what #238 does.

The length-prefix classes, verified against the current decode_into match rather than inferred from the type table:

MS-TDS class Types Wire shape Site
FIXEDLEN Int1(1) Int2(2) Int4(4) Int8(8) Flt4(4) Flt8(8) Bit(1) Money4(4) Money(8) DateTime(8) DateTim4(4) value only, no prefix decoder.rs:1105-1169, :1250
BYTELEN IntN FltN BitN MoneyN DecimalN NumericN DateTimeN DateN TimeN DateTime2N DateTimeOffsetN Guid u8 length, 0 = NULL decoder.rs:1118, :1264-1341, :1345
USHORTLEN BigBinary, non-PLP BigVarBinary, non-PLP BigChar/BigVarChar/NChar/NVarChar/Char/VarChar u16 length, 0xFFFF (CHARBIN_NULL) = NULL decoder.rs:1210, :1232, :1751
LONGLEN Text, NText u8 textptr length (0 = NULL), textptr, 8-byte timestamp, then u32 length decoder.rs:1723-1749
PARTLEN anything where metadata.is_plp() holds chunked decoder.rs:1226, :1718

Note Guid (0x24) is BYTELEN, not fixed-width: decoder.rs:1345 reads a length byte and then requires it to be exactly 16. It is easy to misfile as FIXEDLEN.

Constraint: do not over-hoist. Anything the wire carries per cell must stay in the decode step, not the plan. Specifically the IntN/FltN/MoneyN/BitN/DateTimeN length byte (decoder.rs:1118) and the 0xFFFF CHARBIN_NULL marker remain per-cell reads. The plan hoists only what ColumnMetadata determines.

Design constraints established by the #254 spike

These were paid for in a working prototype (b0e5132f) and should be treated as settled:

  • Where the plan lives matters a lot. Hanging it off ColumnMetadata looks cleanest but requires touching 54 struct literals repo-wide. Hanging it off ColMetadataToken as OnceLock<Vec<ColumnDecodeSpec>> needs only 7, keeps #[derive(Default)] working so hand-built tokens in tests need one extra field, and lets the parser fill it eagerly via OnceLock::from(...) so the hot path never touches lazy init.
  • ⚠️ Encryption must not be folded into the spec. The spike's first attempt classified encrypted columns as Fallback. That silently disabled the guard in drive_row_columns that refuses to stream ciphertext to PLP readers: an encrypted PLP column no longer compared equal to the Plp variant, so the stop_here && op == Plp check never fired and the UnimplementedFeature fail-fast was bypassed. It was caught only by io::token_stream::tests::ae_paused_plp_streaming_fails_fast. The spec must describe the bytes on the wire only; carry encryption as a separate field alongside it. Any PR here needs an explicit test for encrypted-PLP classification.
  • RowPauseState must carry a cloned plan, or a paused row loses it on resume.
  • A short or absent plan must re-derive, not default to a fallback variant. A wrong default silently changes decode behaviour rather than merely costing time.
  • Clippy's too_many_arguments will push toward a RowLayout<'a> (Clone, Copy) borrow bundling columns / specs / decryptor. Worth doing anyway: it removes a class of desynchronization bug where the arrays disagree.
  • token_stream.rs is the most contended file in the repo. Sequence this against other in-flight work there.

Implementation trivia from the spike: OnceLock<T> derives Clone/Debug/Default when T: Clone, which is required since ColMetadataToken derives all three. collect() into a OnceLock::from(...) argument needs an explicit Vec<...> annotation or you get E0283. The enum variant is TdsDataType::DateTim4, not DateTime4.

Suggested scope for a single PR

  • Add ColumnDecodeSpec (+ FixedKind / VarU8Kind / VarU16Kind) covering all TdsDataType variants, with no fallback variant
  • Add ColMetadataToken::decode_plan() backed by OnceLock<Vec<ColumnDecodeSpec>>, filled eagerly by the parser
  • Carry encryption as a separate field, never as a spec variant; test encrypted-PLP classification explicitly
  • Convert decode_into to consume the plan, replacing the existing match rather than adding beside it
  • Convert decode to consume the same plan
  • Fold get_encoding_type / is_plp / is_long_len_type into plan construction
  • Fold decimal precision/scale into ColumnDecodeSpec::Decimal, and get_scale into the TimeN / DateTime2N / DateTimeOffsetN specs
  • Thread the plan through RowPauseState
  • Tests covering every ColumnDecodeSpec variant, including the types absent from the POC: reduce per-row cost in the TDS row decode path #238 benchmark

The #238 benchmark schema is 39 INT + 9 VARCHAR(6) with all columns nullable, so it exercises only a small fraction of the type matrix. Test coverage here should be driven by the type set, not by the benchmark.

Affected crate

mssql-tds

Alternatives considered

Hoist wire extent only, leaving interpretation to the existing match (the #254 spike shape). Considered and rejected on measurement.

That design is DecodeOp { Fixed(u8), ByteLen, ShortLen, LongLen, Plp, Fallback } — it answers how many bytes a value occupies but not how to interpret them, so the ~40-arm match data_type still runs per cell. Measured against its preceding commit:

Case Before (ms) After (ms) Δ
39 INT + 9 VARCHAR(6) 61.0 63.7 +4.4%
same, every 4th column NULL 50.5 52.6 +4.1%
8 × VARCHAR(512) 26.0 26.5 +2.1%

It is a pure regression by construction, not by accident of implementation: it adds a plan lookup and an indirection per column while removing no per-cell work. The Fallback variant guarantees the old match survives, so the two can never collapse into one. The measurement is also generous — the benchmark builds ColMetadataToken by hand and populates the plan once via OnceLock::get_or_init, so a naive implementation looks worse.

A resolved plan that carries interpretation, by contrast, measures as a win on the same schemas — −6.0% on 39 IntN + 9 varchar and −3.3% on 48 varchar in an isolated harness. The two results are not in tension: they measure two different designs. This issue is the second one.

Split the plan into two parallel arrays: Vec<WireLength> + Vec<ValueDecode>. Considered and rejected on measurement.

The idea was to separate wire extent from value semantics, so that a future two-pass buffered decoder's measure pass would consult only the narrow extent array, making the "pass 1 knows only lengths" property structural rather than conventional. The predicted mechanism was cache density.

The density prediction held, and was in fact understated:

ColumnDecodeSpec=20B   WireLength=2B   ValueDecode=20B   EncodingType=16B
48 columns: fused=960B   split measure array=96B

EncodingType::LcidBased(SqlCollation) is 16 bytes, which makes any spec carrying an encoding 20 bytes. So the split measure array is 10x denser.

It made no difference. Benchmarked with mssql-tds/benches/decode_plan_shape.rs, 2000 rows per iteration, both shapes gated on identical RowWriter traces and on both measure passes returning the true row length:

Group Schema fused split delta
pass 1 (measure only) 39 IntN + 9 varchar 234.4 us 254.9 us split 8.7% slower
varchar48 174.8 us 172.1 us split 1.5% faster
intn48 194.7 us 185.5 us split 4.7% faster
single-pass decode 39 IntN + 9 varchar 1.097 ms 1.118 ms split 1.9% slower
varchar48 3.962 ms 3.978 ms split 0.4% slower
intn48 371.0 us 368.2 us split 0.7% faster
two-pass total 39 IntN + 9 varchar 1.273 ms 1.280 ms split 0.6% slower
varchar48 4.170 ms 4.251 ms split 1.9% slower
intn48 510.7 us 494.8 us split 3.1% faster

The differences are within +/-3% and inconsistent in direction, and the split actually loses on the most realistic schema. The cache argument assumed pressure that does not exist: 960 bytes sits entirely in L1. That cannot change with scale either — SQL Server caps a result set at 1024 columns, so a worst-case fused plan is 1024 x 20B = 20KB, still inside a typical 32KB L1d. The density advantage is unreachable in principle, not merely unmeasured.

Splitting would therefore buy only a type-safety property, at the cost of two arrays, doubled plan construction, and a new class of drift bug if they disagree. Keeping the plan fused.

Take #238 wholesale. #238 bundles four independent axes: dispatch, precomputation (this issue), buffering, and value handoff. It also carries a POC_README.md at the repo root, duplicated PLP constants, a dead decode_op_into, and pre-rebase benchmark numbers, and it adds a third copy of the type switch rather than replacing one.

Leave classification per cell and rely on the branch predictor. This addresses only the dispatch cost, which measurement suggests is the smaller component. It does nothing about the roughly 40M redundant get_encoding_type / is_plp / is_long_len_type calls or the cold-struct pointer chases, and it leaves the two ~40-arm switches duplicated.

Memoize get_encoding_type alone. A narrower fix that captures part of the win, but it leaves is_plp, is_long_len_type, decimal precision/scale, and get_scale per cell, does not give a dense plan array, and does not consolidate decode with decode_into.

Additional context

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions