Skip to content

Improve decoder throughput with buffered scalar reads - #299

Draft
Saurabh Singh (saurabh500) wants to merge 2 commits into
mainfrom
dev/saurabh/sync-buffered-reads
Draft

Improve decoder throughput with buffered scalar reads#299
Saurabh Singh (saurabh500) wants to merge 2 commits into
mainfrom
dev/saurabh/sync-buffered-reads

Conversation

@saurabh500

@saurabh500 Saurabh Singh (saurabh500) commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Description

Improves row-decoder throughput by synchronously consuming fixed-width scalars that are already buffered in NetworkTransport. A buffer miss falls back to the existing async TDS packet read/refill path and retries, so packet framing and error handling remain authoritative.

The optimized decoder reads now cover byte, little-endian i16, u16, 24-bit u32, i32, u32, 40-bit u64, i64, f32, and f64. This is a standalone runtime optimization informed by measurements from #269. It does not revive that PR or retry decoder convergence.

Implementation

  • Adds explicit non-consuming-on-miss probes to both TdsPacketReader configurations, with safe None defaults for other readers.
  • Implements direct fixed-width probes in TdsReadBuffer and delegates them from NetworkTransport.
  • Uses one parameterized read_sync_first! macro at every production fixed-width scalar read in datatypes/decoder.rs.
  • Keeps read_tds_packet() as the only refill path for async misses.
  • Leaves big-endian packet/header reads and u64 reads outside the decoder unchanged.

Before and after

Before: every scalar enters the async path

sequenceDiagram
    participant D as Decoder
    participant A as async read_*()
    participant B as TdsReadBuffer
    participant P as Packet I/O

    D->>A: read_*().await
    Note over D,A: Construct and poll a future for every scalar
    A->>B: Enough bytes buffered?
    alt Buffered hit
        B-->>A: Yes
        A->>B: Consume N bytes
        A-->>D: Ready(value)
    else Buffer miss
        B-->>A: No
        A->>P: read_tds_packet().await
        P-->>A: Next framed payload
        A->>B: Retry and consume N bytes
        A-->>D: Ready(value)
    end
Loading

After: synchronous probe before async fallback

sequenceDiagram
    participant D as Decoder
    participant M as sync-first macro
    participant T as NetworkTransport
    participant B as TdsReadBuffer
    participant P as Packet I/O

    D->>M: read_sync_first!
    M->>T: try_read_*() [sync]
    T->>B: Probe complete N-byte scalar
    alt Buffered hit
        B-->>T: Some(value), consume N bytes
        T-->>M: Some(value)
        M-->>D: value
        Note over D,M: No async future constructed or polled
    else Buffer miss
        B-->>T: None, consume zero bytes
        T-->>M: None
        M->>T: read_*().await
        T->>P: read_tds_packet().await
        P-->>T: Next framed payload
        T->>B: Refill, retry, consume complete scalar
        T-->>M: Ok(value)
        M-->>D: value
    end
Loading

N is the scalar's fixed wire width (1, 2, 3, 4, 5, or 8 bytes). Packet framing, cancellation, encryption, and errors remain in the existing async fallback.

Correctness coverage

Targeted tests cover successful buffered reads, zero consumption when any supported scalar is incomplete, and every supported scalar split across real TDS packet boundaries through NetworkTransport. Existing NULL and length-marker handling remains unchanged.

Production-reader benchmarks

Both benchmarks use concrete NetworkTransport, packetized in-memory input, 7,992-byte packet payloads, 30,000 rows per pass, 2 warmups, 9 measured passes per cell, and 8 paired rounds alternating baseline/candidate order. Negative deltas are faster.

Initial change versus clean main (ac1023a1)

Workload Sink Paired median Range Main median Candidate median
INT/VARCHAR discard -6.06% -15.86% to +11.50% 62.25 ms 58.46 ms
INT/VARCHAR materialize -14.32% -21.77% to -11.27% 99.75 ms 85.89 ms
mixed discard -11.04% -22.62% to -1.52% 128.02 ms 112.58 ms
mixed materialize -10.49% -20.84% to +3.20% 172.04 ms 153.39 ms

Expanded fixed-width reads versus the first draft (47c2fb68)

The new fixed_scalars workload has 64 columns: Int1, Int2, Int4, Int8, Flt4, Flt8, DateTime, and DateTim4, repeated eight times.

Workload Sink Paired median Range First draft median Expanded median
fixed scalars discard -18.14% -20.76% to -16.08% 95.55 ms 78.48 ms
fixed scalars materialize -9.62% -21.52% to +6.46% 155.71 ms 140.14 ms
INT/VARCHAR discard +0.06% -3.87% to +3.78% 93.63 ms 94.03 ms
INT/VARCHAR materialize +1.01% -10.87% to +13.10% 144.72 ms 145.27 ms
mixed discard -0.95% -4.79% to +2.29% 193.05 ms 189.67 ms
mixed materialize -1.02% -5.07% to +9.28% 264.07 ms 261.60 ms

The existing workloads remain effectively flat relative to the first draft, while the workload exercising the newly migrated scalar types improves materially. Builds used identical lockfiles and isolated source/target directories; the compared binaries had different SHA-256 hashes. The optimized benchmark test executable grew by about 192 KB (1.75%), so code-size impact remains a review consideration.

Validation

  • cargo bfmt
  • cargo bclippy
  • Targeted nextest coverage: 173 passed
  • Full mssql-tds library nextest suite with generated TLS fixtures: 1,752 passed
  • $env:RUSTFLAGS='--cfg fuzzing'; cargo check -p mssql-tds --lib
  • cargo btest was attempted locally but the live SQL integration tests fail with Schannel SEC_E_WRONG_PRINCIPAL; unit/library coverage is green and the prior draft's corresponding remote validation passed on every platform.

Risk

The fast path is limited to already-buffered decoder scalars on NetworkTransport. Other packet readers retain their existing async behavior through default None probes. Misses preserve partial bytes and reuse the existing refill path, including cancellation and encryption behavior. The broader set of inlined probes trades some code size for lower per-value async overhead.

Related Issues

Related to #247.

Historical measurement context: #269.

Checklist

  • cargo bfmt passes
  • cargo bclippy passes
  • cargo btest passes locally (environment-blocked; remote validation pending for the latest commit)
  • New/changed functionality has tests
  • Public API changes are documented

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Optimizes TDS row decoding by synchronously consuming buffered scalar values while retaining the existing asynchronous packet-refill fallback.

Changes:

  • Adds optional scalar probes to TdsPacketReader.
  • Implements buffered byte, u16, and i32 reads for NetworkTransport.
  • Routes decoder hot paths through sync-first reads and adds boundary tests.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
mssql-tds/src/io/packet_reader.rs Defines non-consuming scalar probe methods.
mssql-tds/src/datatypes/decoder.rs Uses sync-first scalar reads in decoder paths.
mssql-tds/src/connection/transport/network_transport.rs Delegates probes and retains packet-refill fallback.
mssql-tds/src/connection/transport/buffers.rs Implements scalar probes and unit tests.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@saurabh500

Copy link
Copy Markdown
Contributor Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@github-actions

github-actions Bot commented Aug 15, 2026

Copy link
Copy Markdown

📊 Code Coverage Report

🔥 Diff Coverage

95%

🎯 Overall Coverage

91.5%

📦 Project: mssql-tds + mssql-odbc + mssql-py-core
ℹ️ Note: diff coverage is reported, not enforced.


Diff Coverage

Diff: main...HEAD, staged and unstaged changes

  • mssql-tds/src/connection/transport/buffers.rs (100%)
  • mssql-tds/src/connection/transport/network_transport.rs (94.4%): Missing lines 1144,1152,1161,1203
  • mssql-tds/src/datatypes/decoder.rs (90.1%): Missing lines 866,976-979,1190-1191,1867,1873
  • mssql-tds/src/io/packet_reader.rs (100%)

Summary

  • Total: 265 lines
  • Missing: 13 lines
  • Coverage: 95%

mssql-tds/src/connection/transport/network_transport.rs

  1140     }
  1141 
  1142     #[inline(always)]
  1143     fn try_read_uint32(&mut self) -> Option<u32> {
! 1144         self.tds_read_buffer.try_read_uint32()
  1145     }
  1146 
  1147     #[inline(always)]
  1148     fn try_read_uint40(&mut self) -> Option<u64> {

  1148     fn try_read_uint40(&mut self) -> Option<u64> {
  1149         self.tds_read_buffer.try_read_uint40()
  1150     }
  1151 
! 1152     #[inline(always)]
  1153     fn try_read_int64(&mut self) -> Option<i64> {
  1154         self.tds_read_buffer.try_read_int64()
  1155     }

  1157     #[inline(always)]
  1158     fn try_read_float32(&mut self) -> Option<f32> {
  1159         self.tds_read_buffer.try_read_float32()
  1160     }
! 1161 
  1162     #[inline(always)]
  1163     fn try_read_float64(&mut self) -> Option<f64> {
  1164         self.tds_read_buffer.try_read_float64()
  1165     }

  1199         }
  1200     }
  1201 
  1202     async fn read_float32(&mut self) -> TdsResult<f32> {
! 1203         loop {
  1204             if let Some(value) = self.try_read_float32() {
  1205                 return Ok(value);
  1206             }
  1207             self.read_tds_packet().await?;

mssql-tds/src/datatypes/decoder.rs

  862         let value: ColumnValues = match byte_len {
  863             1 => ColumnValues::TinyInt(read_sync_first!(reader, try_read_byte, read_byte)),
  864             2 => ColumnValues::SmallInt(read_sync_first!(reader, try_read_int16, read_int16)),
  865             4 => ColumnValues::Int(read_sync_first!(reader, try_read_int32, read_int32)),
! 866             8 => ColumnValues::BigInt(read_sync_first!(reader, try_read_int64, read_int64)),
  867             0 => ColumnValues::Null,
  868             _ => {
  869                 return Err(crate::error::Error::from(Error::new(
  870                     std::io::ErrorKind::InvalidData,

  972         // Read 8-byte header
  973         let layout_format_byte = read_sync_first!(reader, try_read_byte, read_byte);
  974         let layout_version_byte = read_sync_first!(reader, try_read_byte, read_byte);
  975         let dimension_count = read_sync_first!(reader, try_read_uint16, read_uint16);
! 976         let base_type_byte = read_sync_first!(reader, try_read_byte, read_byte);
! 977         let _reserved1 = read_sync_first!(reader, try_read_byte, read_byte); // Reserved
! 978         let _reserved2 = read_sync_first!(reader, try_read_byte, read_byte); // Reserved
! 979         let _reserved3 = read_sync_first!(reader, try_read_byte, read_byte); // Reserved
  980 
  981         // Validate header using enum conversions
  982         let _layout_format = VectorLayoutFormat::try_from(layout_format_byte)?;
  983         let _layout_version = VectorLayoutVersion::try_from(layout_version_byte)?;

  1186             // === Fixed-length float types ===
  1187             TdsDataType::Flt4 => {
  1188                 writer.write_f32(
  1189                     col,
! 1190                     read_sync_first!(reader, try_read_float32, read_float32),
! 1191                 );
  1192             }
  1193             TdsDataType::Flt8 => {
  1194                 writer.write_f64(
  1195                     col,

  1863             // Allocates buffer and reads data
  1864             // Creates SqlString with appropriate encoding type
  1865             // NULL handling works (textptr_len = 0)
  1866             // LCID-based decoding implemented (see sql_string.rs)
! 1867             let text_ptr_len = read_sync_first!(reader, try_read_byte, read_byte) as usize;
  1868 
  1869             let length = if text_ptr_len > 0 {
  1870                 const TIMESTAMP_BYTE_COUNT: usize = 8;
  1871                 reader.skip_bytes(text_ptr_len).await?;

  1869             let length = if text_ptr_len > 0 {
  1870                 const TIMESTAMP_BYTE_COUNT: usize = 8;
  1871                 reader.skip_bytes(text_ptr_len).await?;
  1872                 reader.skip_bytes(TIMESTAMP_BYTE_COUNT).await?;
! 1873                 read_sync_first!(reader, try_read_uint32, read_uint32) as usize
  1874             } else {
  1875                 // text_ptr_len == 0 means NULL value
  1876                 return Ok(ColumnValues::Null);
  1877             };


🔗 Quick Links

View Azure DevOps Build · Coverage Report

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 2bf6f59e-c587-4687-93bd-201a9f26681a

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.


use super::row_writer::{RowWriter, write_column_value};

macro_rules! read_sync_first {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can use some comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants