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
For every column of every row, the decoder re-derives facts that cannot change within a result set:
a ~40-arm match on data_type to pick the decode shape
an is_plp() probe
an encryption check (crypto_metadata.is_some())
Column metadata is fixed at COLMETADATA time. Over the PoC benchmark (1.5M rows × 48 columns) that is roughly 72M redundant classifications of constant metadata.
Proposed solution
Resolve each column once when COLMETADATA is parsed, into a dense plan, and dispatch on that in the row driver:
Feasibility: ✅ proven for classification and wiring. Prototyped in b0e5132f (7 files, +211/−53), green on all six configs in #247.
Findings
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 a OnceLock<Vec<ColumnPlan>> needs only 7, keeps #[derive(Default)] working so hand-built tokens in tests need just one extra field, and lets the parser fill it eagerly via OnceLock::from(...) so the hot path never touches lazy-init. Recommend the latter.
The plan must reach the resumed row path too.RowPauseState has to carry a cloned copy alongside its existing columns clone, or a row that pauses mid-decode loses its plan.
⚠️ A subtle security-relevant bug the spike hit, and the fix. The first attempt folded encryption into the shape enum, returning Fallback for encrypted columns. 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 DecodeOp::Plp, so the stop_here && op == Plp check never fired and the UnimplementedFeature fail-fast was bypassed.
io::token_stream::tests::ae_paused_plp_streaming_fails_fast caught it (passing count went 1696 → 1695).
The rule this establishes: DecodeOp must describe the bytes on the wire only. Encryption is a separate bool on ColumnPlan. Any PR here needs an explicit test for encrypted-PLP classification.
This also produced a faster path than the buggy version: decode_or_decrypt_column now early-returns on !plan.encrypted (one bool test) instead of probing an Option, and the residual match simplifies from match (meta.crypto_metadata.is_some(), decryptor) to match decryptor.
plan(col) must re-derive on a short or absent plan, not default to Fallback. A wrong default silently changes decode behavior instead of merely costing time.
Clippy pushed toward a better API.too_many_arguments fired at 8 params on drive_row_columns. Since columns, ops, and decryptor always travel together, they became one RowLayout<'a> borrow (Clone, Copy) — which also removes a class of bug where a caller could desynchronize them. drive_row_columns went 8 params → 6, pause_after_column 5 → 3.
⚠️ The spike stops at classification and wiring. The per-shape fast paths are still unwritten and are the bulk of the work. The real design challenge is doing that without creating a third copy of the type switch — decode, decode_into, and a plan-driven path. PR POC: reduce per-row cost in the TDS row decode path #238 does exactly this. Recommend the plan-driven path replacedecode_into rather than sit beside it.
Affected crate
mssql-tds
Alternatives considered
Cache the classification on ColumnMetadata lazily per row. Still pays an Option probe per column and spreads mutable state into a type with 54 construction sites.
Build a closure/vtable per column. Reintroduces indirect calls, which is exactly what #251 and #252 are removing.
Additional context
token_stream.rs is the most contended file in the repo — it collides with the sans-I/O stack (#189–#203). Sequence this item accordingly.
Benchmark caveat: the PoC workload is 39 INT + 9 VARCHAR(6), which exercises 2 of 10 decode shapes. Benchmark against DATETIME2 / DECIMAL / UNIQUEIDENTIFIER before claiming this generalizes.
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<ColumnPlan> annotation or you get E0283. The enum variant is TdsDataType::DateTim4, not DateTime4.
Sub-issue of #247 (axis: Precomputation).
Problem statement
For every column of every row, the decoder re-derives facts that cannot change within a result set:
matchondata_typeto pick the decode shapeis_plp()probecrypto_metadata.is_some())Column metadata is fixed at COLMETADATA time. Over the PoC benchmark (1.5M rows × 48 columns) that is roughly 72M redundant classifications of constant metadata.
Proposed solution
Resolve each column once when COLMETADATA is parsed, into a dense plan, and dispatch on that in the row driver:
Feasibility: ✅ proven for classification and wiring. Prototyped in
b0e5132f(7 files, +211/−53), green on all six configs in #247.Findings
Where the plan lives matters a lot. Hanging it off
ColumnMetadatalooks cleanest but requires touching 54 struct literals repo-wide. Hanging it offColMetadataTokenas aOnceLock<Vec<ColumnPlan>>needs only 7, keeps#[derive(Default)]working so hand-built tokens in tests need just one extra field, and lets the parser fill it eagerly viaOnceLock::from(...)so the hot path never touches lazy-init. Recommend the latter.The plan must reach the resumed row path too.
RowPauseStatehas to carry a cloned copy alongside its existingcolumnsclone, or a row that pauses mid-decode loses its plan.Fallbackfor encrypted columns. That silently disabled the guard indrive_row_columnsthat refuses to stream ciphertext to PLP readers: an encrypted PLP column no longer compared equal toDecodeOp::Plp, so thestop_here && op == Plpcheck never fired and theUnimplementedFeaturefail-fast was bypassed.io::token_stream::tests::ae_paused_plp_streaming_fails_fastcaught it (passing count went 1696 → 1695).The rule this establishes:
DecodeOpmust describe the bytes on the wire only. Encryption is a separateboolonColumnPlan. Any PR here needs an explicit test for encrypted-PLP classification.This also produced a faster path than the buggy version:
decode_or_decrypt_columnnow early-returns on!plan.encrypted(one bool test) instead of probing anOption, and the residual match simplifies frommatch (meta.crypto_metadata.is_some(), decryptor)tomatch decryptor.plan(col)must re-derive on a short or absent plan, not default toFallback. A wrong default silently changes decode behavior instead of merely costing time.Clippy pushed toward a better API.
too_many_argumentsfired at 8 params ondrive_row_columns. Sincecolumns,ops, anddecryptoralways travel together, they became oneRowLayout<'a>borrow (Clone, Copy) — which also removes a class of bug where a caller could desynchronize them.drive_row_columnswent 8 params → 6,pause_after_column5 → 3.decode,decode_into, and a plan-driven path. PR POC: reduce per-row cost in the TDS row decode path #238 does exactly this. Recommend the plan-driven path replacedecode_intorather than sit beside it.Affected crate
mssql-tds
Alternatives considered
Cache the classification on
ColumnMetadatalazily per row. Still pays anOptionprobe per column and spreads mutable state into a type with 54 construction sites.Build a closure/vtable per column. Reintroduces indirect calls, which is exactly what #251 and #252 are removing.
Additional context
token_stream.rsis the most contended file in the repo — it collides with the sans-I/O stack (#189–#203). Sequence this item accordingly.Benchmark caveat: the PoC workload is 39
INT+ 9VARCHAR(6), which exercises 2 of 10 decode shapes. Benchmark againstDATETIME2/DECIMAL/UNIQUEIDENTIFIERbefore claiming this generalizes.Implementation trivia from the spike:
OnceLock<T>derivesClone/Debug/DefaultwhenT: Clone, which is required sinceColMetadataTokenderives all three.collect()into aOnceLock::from(...)argument needs an explicitVec<ColumnPlan>annotation or you getE0283. The enum variant isTdsDataType::DateTim4, notDateTime4.