Conversation
This was referenced Sep 25, 2026
Member
|
Duplicate of #5809. Please search existing pull requests before submitting. |
Author
Sorry, I didn't find it because the PR was closed |
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
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.
This relates to...
Every WebSocket message the client sends:
createFrame(binary, ping, close, and text frames that take the general path) andcreateFastTextFrame(text messages).Rationale
Masking allocated a four-element array per frame (
generateMask()) and then XORed the payload one byte at a time, reading the key back from that array withi & 3.writeMask(target, offset)writes the next four bytes of the random pool straight into the frame header, where they belong anyway. It consumes the pool exactly likegenerateMask()did (four bytes per frame, same refill), so no array per frame.maskPayload()keeps the four key bytes in local variables and XORs four payload bytes per step, then the remaining zero to three bytes. Both frame builders use it, so there is one masking loop instead of two.generateMask()stays exported forbenchmarks/websocket/generate-mask.mjs, now built onwriteMask(), so the refill logic exists once.Changes
lib/web/websocket/frame.js: the three items above.test/websocket/frame.js: a round-trip test for payload lengths 0-9 (every tail length), 125, 126, 65535 and 65536, for both builders: unmasking with the frame's own key gives back the payload. A deliberately broken tail fails it.Features
N/A
Bug Fixes
N/A
Breaking Changes and Deprecations
N/A
Verification (this branch vs
328ab843)Headline numbers are standalone: one revision per process, 8 alternating process pairs, minimum of 100 runs per process, with an identical-code control (base against base) run the same way.
The paired runs overstate this case: -69.2% / -67.6% on the full suite, -54.2% / -54.2% focused. With two revisions in one process, shared code sees both revisions' objects, and this case also warms up slowly (its first calls are several times slower than later ones), which widens every paired sample. The standalone number is the one to expect.
Observable surface
createFastTextFramestill masks its input buffer in place, as before. The guard unmasks every frame of the benchmark with its own key and compares with the payload, and the new test does the same for every tail length.generateMaskkeeps its export and return shape (an array of four numbers).writeMaskandmaskPayloadare internal.WPT (
npm run test:wpt: /fetch, /mimesniff, /xhr, /websockets, /eventsource) was run locally on328ab843and on this branch: per test and per case, pass/fail is identical across all 1 282 test files.Note on CI:
Test with Node.js 22 compiled --without-intlalready fails onmainat328ab843, before this change.Reproducing the numbers
The harness (A/B runner, guard, differential, focused and standalone runners) lives in
perf/on the campaign branch: zirkelc/undici@afde9423/perf (the plan and the experiment log with all discarded experiments are in the same directory). It needs Node 24 (it runs.mtsdirectly) and no install beyondnpm ci.A row marked
?has an interquartile band that crosses zero in that run; judge by two runs that agree, and by the standalone numbers.perf/cases.mts, the workloads these numbers come fromStatus
Companion PRs from the same campaign: #5901 (perf(fetch): avoid redundant request state in the Request constructor), #5903 (perf(webidl): check ByteString code units with a native scan), #5904 (perf(cookies): split each cookie pair once in getCookies)