perf: serialize an RPC request into one buffer

[arthurschreiber]

Problem

RpcRequestPayload writes a small WritableTrackingBuffer per parameter and yields each one in turn: a 20-parameter request reaches the packetizer as ~21 separate buffers. Each buffer and each yield is overhead, and downstream Readable.from(payload) does a pull cycle per chunk.

Change

The payload now writes the whole request — the request header, and every parameter's header, TYPE_INFO and value — into a single WritableTrackingBuffer, and yields its chunks once at the end.

The bytes are unchanged. A large value written by reference is still referenced, not copied: WritableTrackingBuffer keeps buffers of 8 KB or more as their own chunks, so getBuffers() still hands the value out as a distinct by-reference chunk. So this adds no copy — the request just arrives as a few large chunks instead of a small buffer per parameter. Per-parameter error attribution is unchanged: each parameter's writeTypeInfo/writeValue is still wrapped in the InputError that names it.

Measurements

Serialization only, same machine, benchmarks/parameters/scalar-params.js (20 scalar parameters per request):

	before	after
20 scalar params, req/s	~48k	~74–91k
1 MB varbinary(max), req/s	~11.3k	~11.2k
10 MB varbinary(max), req/s	~7.6k	~7.9k

About 1.7x for scalar-heavy requests; binary values, dominated by the value itself, are unchanged.

Validation

• The existing byte-equivalence suite (test/unit/rpcrequest-payload-test.ts: 40 parameter cases across TDS 7.4/7.2, with/without collation) and the large-value by-reference test cover this unchanged.
• Unit suite: 498 passing. Parameterised-statement (140), RPC (33) and TVP (2) integration tests pass against SQL Server 2022.
• Lint and typecheck clean.

Stacked on #1774; retargets to master once that merges. It also sets up the streaming-parameter work to follow: the payload becomes the one place that decides how a request is written out.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Fxv5h4UMCGJEpjgCKcAxug

---

Generated by Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-02T19:48:09.095552Z	687b8e0	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[claude]

Reviewed the diff (src/rpcrequest-payload.ts only — no CLAUDE.md in this repo to check against, so I went by the codebase's own conventions).

Summary: Clean, well-scoped change. Collapsing per-parameter WritableTrackingBuffers into one shared buffer for the whole RPC request is a sensible way to cut yield/buffer overhead, and the by-reference behavior for large values is preserved correctly since WritableTrackingBuffer.writeBuffer only takes the "reference, don't copy" path for chunks ≥ CHUNK_SIZE (8 KB) regardless of when getBuffers() is called relative to when those buffers were pushed.

Correctness

• Verified RpcRequestPayload is only ever constructed and consumed as an Iterable<Buffer> (connection.ts, get-parameter-encryption-metadata.ts) — nothing depends on the old per-parameter generator (generateParameterData) that got replaced by writeParameterData, so renaming it to a plain method and dropping the inner generator is safe.
• Per-parameter error attribution is preserved: writeTypeInfo/writeValue are still called inside the same try/catch that wraps them in InputError naming the parameter, and since the shared buffer is only mutated (never handed out mid-stream) a thrown error simply abandons the whole payload — no partial/corrupt buffer can leak out.
• The existing test/unit/rpcrequest-payload-test.ts byte-equivalence suite (40 param cases × TDS 7.4/7.2 × with/without collation) and the "passes large values through by reference" test both directly cover the behavior this PR depends on, so I'm fairly confident the refactor is byte-for-byte safe. I wasn't able to run the suite myself in this sandbox (no node_modules installed), but the logic checks out from reading writable-tracking-buffer.ts.

Minor/nit

• One behavioral nuance not mentioned in the PR description: previously generateData yielded the request-header buffer immediately, then yielded each parameter's buffer(s) as they were produced (interleaved compute/yield). Now the entire request (aside from large by-reference values) is built synchronously before the first yield fires. This shouldn't matter for Readable.from(payload) correctness, but for requests with a very large number of parameters it does mean slightly more synchronous work happens before any bytes are handed to the consumer. Given the stated goal (fewer yields/allocations) this is an acceptable and probably intended tradeoff — just flagging it as a small behavior shift in case it matters for very large parameter counts under backpressure.
• The new comment in generateData (lines 40-43) and the removed comment in writeParameterData together read fine, but it might be slightly clearer to keep a one-line comment at the writeBuffer call sites in data-type.ts too, since that's the layer that actually decides copy-vs-reference — not required, just a readability thought.

No security or performance concerns beyond what's already covered by the PR's own benchmarks, which look credible (fewer small-buffer allocations and generator yields for scalar-heavy requests, no change for large binary values). Test coverage looks adequate for this internal refactor; nothing new needed given the byte-equivalence suite already exists and covers this path.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 687b8e030d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Preserve streaming between large parameters

When an RPC contains multiple large string or TVP parameters, this loop now serializes every parameter before the first chunk is yielded. WritableTrackingBuffer retains all generated chunks until getBuffers() finishes, so encoded strings and TVP rows for the entire request remain resident simultaneously; previously each parameter's chunks could be consumed and released before the next parameter was generated. This can substantially increase peak memory and block the event loop, potentially causing OOM failures for large multi-parameter requests. Yield or consume completed chunks incrementally rather than buffering the full RPC.

Useful? React with 👍 / 👎.

[arthurschreiber]

Folded into #1777, which already carries this commit (687b8e0) and whose description covers the single-buffer serialization. Keeping a 9-line stepping stone as its own PR only adds a CI cycle to the path to streaming parameters, so this is closed without merging.

---

Generated by Claude Code