Skip to content

Reduce per-call cost of the JS/C++ boundary - #1300

Open
nigrosimone wants to merge 3 commits into
uNetworking:masterfrom
nigrosimone:perf-boundary-costs
Open

Reduce per-call cost of the JS/C++ boundary#1300
nigrosimone wants to merge 3 commits into
uNetworking:masterfrom
nigrosimone:perf-boundary-costs

Conversation

@nigrosimone

@nigrosimone nigrosimone commented Aug 19, 2026

Copy link
Copy Markdown

Every string argument crossing from JS to C++ pays two full passes (Utf8Length + WriteUtf8). Since V8 12.5, String::ValueView lets one-byte ASCII strings (header keys and values, status, typical bodies) cross with a single scan + memcpy. Older V8 keeps the old path.

Smaller items with the same goal:

  • header keys and method strings are created kInternalized: they repeat identical on every request and JS uses them as object keys, so the property store skips the re-internalization
  • onAborted stores the callback in an internal field of res: one global handle per request instead of two, and the capture now fits MoveOnlyFunction inline storage (same for onData/onDataV2/onWritable)
  • tryEnd returns via bulk Array::New instead of two Object::Set
  • v8-fast-api-calls.h does not exist on V8 14 (Node 26), the include and the unused fast-call code are guarded (is dead code??)

Numbers (Node 26 win-x64, min of 5 interleaved runs, ns/op): getHeader 96 -> 81, onAborted install 127 -> 67, forEach collected into a headers object 2599 -> 1999, method dispatch 52 -> 48. End to end (status + 8 headers + 1KB body, pipelined keep-alive, 9 runs): mean 102k -> 106k req/s, median 102k -> 108k. Note: iterating headers without using the keys gets ~5% slower from the interning, the object building case is what Express layers do on every request.

Functional checks pass on both builds: forEach pairs unchanged, abort fires and invalidates res, tryEnd shape unchanged.

@nigrosimone

Copy link
Copy Markdown
Author

I don't know if CI failure is my fault, but I've seen it fail other times too.

@nigrosimone
nigrosimone marked this pull request as ready for review August 19, 2026 08:21
@webcarrot

webcarrot commented Aug 19, 2026

Copy link
Copy Markdown

AFAIK It (kInternalized) will allow DDOS via OOM by spamming "random" headers names.

@uNetworkingAB

uNetworkingAB commented Aug 19, 2026 via email

Copy link
Copy Markdown
Contributor

@nigrosimone

Copy link
Copy Markdown
Author

Both objections deserved a proper answer rather than a claim, so I have pushed tests/strings.js and wired it into build.yml next to smoke.js.

On the ValueView history. You are right, and it is the reason the test exists. #1262 took strView.data8() and used it as utf-8; a V8 one-byte string is Latin-1, so ä went out as the single byte 0xE4 and #1280 got str<?>ngar back. This PR only takes the fast path when the string is one-byte and no byte has the high bit set, which is ASCII, where Latin-1 and utf-8 are the same bytes. Anything else falls through to the existing path.

The test compares bytes rather than decoded text, since decoded text hides a wrong encoding behind a replacement character. Cases: ASCII, the exact string from #1280, ÿ as the Latin-1 edge, BMP, astral pairs, mixed, empty, and a 200 KB body that leaves the 128 KB pool in alloc and takes the malloc fallback. Bodies and header values both ways, plus forEach, method, url, query and the tryEnd return shape.

I checked the test can actually fail rather than only pass: simulating what the reverted code did, a Latin-1 copy of the one-byte buffer, three cases go red, the #1280 string, ÿ and the 200 KB one. ASCII and empty stay green, correctly, because those bytes are identical in both encodings, and the two-byte cases never reached that branch.

On interning and memory. I could not reproduce an OOM. V8's string table holds weak references, so an internalized string nobody points at is collected. Measured on Node 26: one million unique property keys, which is how V8 internalizes, take 43 MB and the heap returns to its exact starting value after a collection, zero retained. The test carries a smaller version of that, 5000 unique header names over 500 requests, and asserts the heap grows less than 8 MB across a gc; it measures about 1 MB here.

What is fair in the objection is the cost, not the leak: interning hashes and looks up the table per distinct name, so random header names are more expensive than before, and the table grows between major collections. That is also why the PR body says iterating headers without using the keys gets about 5% slower. If that trade reads wrong for a server that cannot choose its clients, dropping kInternalized from the header keys and keeping it only for the method is a one-line change and the ValueView work stands on its own.

One caveat on where this runs: both workflows trigger on pushes to master and clone upstream, so the new test cannot run in this PR's CI. Locally it is make && cd tests && npm install ws && node --expose-gc strings.js. Against the released 20.69.0 it is green.

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.

3 participants