perf: serialize COM_STMT_EXECUTE in a single exact-size pass - #4494
Merged
Conversation
Execute.toPacket serialized every statement twice: a dry run against Packet.MockBuffer() to learn the packet length, then the real pass. Both passes re-encoded every parameter, and since lengthCodedStringLength and writeLengthCodedString each encode internally, a string parameter was UTF-8-encoded four times per execute(). Query.toPacket had the same shape on its attribute path. Every parameter encoder already reports the exact wire length of its value, so the dry run is redundant: parameters are now encoded once, the packet length is summed arithmetically, and the packet is written in a single pass. String-shaped values leave toParameter as encoded bytes and are written with writeLengthCodedBuffer, so each string is encoded exactly once. timeEncoder declared length 13 while writeTime emits 1, 9 or 13 bytes; it now computes the exact length via Packet.timeLength, kept next to writeTime. Since a length/writer drift would send uninitialized allocUnsafe bytes to the server, both serializers throw on offset !== length instead of returning a short-filled packet, and a new unit test pins parameter.length to the bytes each writer emits across the parameter type matrix. Byte equivalence with the previous serializer was verified over 1100 randomized packets: legacy, typed and hinted parameters, utf8/big5/ binary charsets, both flag modes, attributes with multibyte names. Two pre-existing COM_QUERY attribute bugs fell out of the rewrite: a typed NULL attribute kept its declared type so the null bitmap was never set, producing a malformed packet, and the unsigned flag of typed attributes was hardcoded to 0. Both now match the COM_STMT_EXECUTE path, with regression tests that fail against the previous serializer. serialize.js (new): isolated toPacket() benchmark plus an allocation-strategy micro used to evaluate buffer pooling. Interleaved best-of-3: execute with 3 params 6.6x, 10 strings 5.3x, dates 3.9x, typed 3.2x, with attributes 4.5x, without the attribute flag 4.3x, query with attributes 3.0x; the untouched query fast path is unchanged. Pooling send buffers and heuristic sizing were measured and rejected for this path; the data and reasoning are recorded in ANALYSIS.md.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4494 +/- ##
==========================================
+ Coverage 92.45% 92.50% +0.05%
==========================================
Files 93 93
Lines 15855 15909 +54
Branches 2254 2275 +21
==========================================
+ Hits 14658 14717 +59
+ Misses 1197 1192 -5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
- the null-bitmap spill byte on the COM_QUERY attribute path (nine attributes), including read-back of both bitmap bytes - both offset-vs-length drift guards, via a temporarily patched writeDouble that under-reports its advance - writeNothing: the writer of a NULL parameter is now invoked in the length-invariant test and must emit zero bytes lib/packets/query.js is at 100% patch coverage from the unit suite alone; the remaining uncovered lines in execute.js/encode_parameter.js predate this branch.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Execute.toPacket()serialized everyCOM_STMT_EXECUTEtwice: a dry run againstPacket.MockBuffer()to learn the packet length, then the real pass. Both passes re-encoded every parameter, and sincelengthCodedStringLengthandwriteLengthCodedStringeach encode internally, a string parameter was UTF-8-encoded four times perexecute().Query.toPacket()had the same shape on its attributes path (the no-attributes fast path from #4486 is untouched).Every parameter encoder already reports the exact wire length of its value, so the dry run is redundant.
toPacketnow encodes each parameter once, sums the exact packet length arithmetically, allocates once, and writes once. String-shaped values leavetoParameter/lengthCodedEncoderas encoded bytes and are written withwriteLengthCodedBuffer, so each string is encoded exactly once.timeEncoderdeclaredlength: 13whilewriteTimeemits 1, 9 or 13 bytes; it now computes the exact length (Packet.timeLength, kept next towriteTime).Because a length/writer drift would otherwise send uninitialized
allocUnsafebytes to the server, both serializers throw onoffset !== lengthinstead of returning a short-filled packet, and a new unit test pinsparameter.lengthto the bytes each writer actually emits across the parameter type matrix.Bug fixes that fell out of the rewrite
Two pre-existing
COM_QUERYattribute-path bugs, now aligned with theCOM_STMT_EXECUTEpath and covered by regression tests that fail against the previous serializer:TypedParameter.INT(null)) kept its declared type, so the null bitmap was never set and the packet was malformed;Measurements
New isolated benchmark
benchmarks/perf/serialize.js(synctoPacket()loop, no socket), interleaved best-of-3 vs baseline:Buffer pooling and heuristic sizing for send buffers were also evaluated (
serialize.js --alloc) and rejected:allocUnsafecosts 76–286ns under 1KB against ~70ns for a pooled-slab view, whilestream.write()retention (plus TLS, async zlib and >16MB slicing aliasing the buffer) makes recycling unsafe to track for a few percent of best-case gain. Data and reasoning are recorded inbenchmarks/perf/ANALYSIS.md§7.Verification
test-execute-nocolumndeffailure, which also fails on master.