Skip to content

src: write strings to streams without flattening - #67435

Open
araujogui wants to merge 3 commits into
nodejs:mainfrom
araujogui:src-stream-write-no-flatten
Open

araujogui wants to merge 3 commits into
nodejs:mainfrom
araujogui:src-stream-write-no-flatten

Conversation

@araujogui

@araujogui araujogui commented Nov 12, 2026 •

Copy link
Copy Markdown
Member

StreamBase::WriteString() created a String::ValueView twice, the first time in StringBytes::StorageSize() only to read the length. A ValueView flattens the string. So a string built by concatenation, like an HTTP response header, was copied to a new flat string on the V8 heap, then copied again into the write buffer. StreamBase::Writev() did the same for each string chunk.

Now StorageSize() reads the length from the string, and one-byte strings are copied with String::WriteOneByte(), which does not flatten. For UTF-8, the characters go to the last third of the buffer and are converted into the rest: a Latin-1 character takes at most two bytes. Two-byte strings, and UTF-8 strings whose buffer has no room for that, use StringBytes::Write() as before.

WriteString() and Writev() share this, so it covers every HTTP/1.1 response header, with a string body or a Buffer body, and TLS sockets too.

Flat strings written as UTF-8 pay one extra memcpy of at most 65535 bytes. Other StringBytes::Write() callers (fs, Buffer) are not changed.

Copilot AI balanced review requested due to automatic review settings November 12, 2026 14:19

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@nodejs-github-bot nodejs-github-bot added buffer Issues and PRs related to the buffer subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Nov 12, 2026
StreamBase::WriteString() created a String::ValueView twice, the first
time in StringBytes::StorageSize() only to read the length. A ValueView
flattens the string, so writing a string built by concatenation, such
as the header of an HTTP response, allocated a flat copy on the V8 heap
before copying it again into the write buffer.

Read the length from the string in StorageSize(), and copy one-byte
strings that fit the stack buffer with String::WriteOneByte(), which
does not flatten. For UTF-8 the characters go to the last third of the
buffer and are converted into the rest of it: a Latin-1 character takes
at most two bytes.

Assisted-by: Claude
Signed-off-by: Guilherme AraΓΊjo <arauujogui@gmail.com>
@araujogui
araujogui force-pushed the src-stream-write-no-flatten branch from cb1b06e to f89198f Compare November 12, 2026 14:21
Signed-off-by: Guilherme AraΓΊjo <arauujogui@gmail.com>
@codecov

codecov Bot commented Nov 12, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.65517% with 3 lines in your changes missing coverage. Please review.
βœ… Project coverage is 90.70%. Comparing base (933cb11) to head (c8c2b24).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
src/string_bytes.cc 75.00% 2 Missing ⚠️
src/stream_base.cc 95.23% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #67435      +/-   ##
==========================================
+ Coverage   90.69%   90.70%   +0.01%     
==========================================
  Files         801      801              
  Lines      281875   281884       +9     
  Branches    54317    54323       +6     
==========================================
+ Hits       255652   255695      +43     
+ Misses      16348    16312      -36     
- Partials     9875     9877       +2     
Files with missing lines Coverage Ξ”
src/stream_base.cc 76.27% <95.23%> (+0.14%) ⬆️
src/string_bytes.cc 75.57% <75.00%> (-0.18%) ⬇️

... and 35 files with indirect coverage changes

πŸš€ New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • πŸ“¦ JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@araujogui

Copy link
Copy Markdown
Member Author

https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1904/

benchmark/http/end-string.js
                                                                 confidence   improvement   accuracy (*)    (**)   (***)
http/end-string.js duration=5 c=50 len=4 benchmarker='wrk'                       +0.89 %   Β±0.91%  Β±1.21%  Β±1.57%
http/end-string.js duration=5 c=50 len=64 benchmarker='wrk'      ***             +3.42 %   Β±0.70%  Β±0.93%  Β±1.22%
http/end-string.js duration=5 c=50 len=1024 benchmarker='wrk'    ***             +5.66 %   Β±0.64%  Β±0.85%  Β±1.11%
http/end-string.js duration=5 c=50 len=16384 benchmarker='wrk'                   -0.58 %   Β±0.77%  Β±1.03%  Β±1.34%
http/end-string.js duration=5 c=50 len=102400 benchmarker='wrk'                  -0.12 %   Β±0.44%  Β±0.59%  Β±0.76%

                                                                    -6.3%              0%              +6.3%
http/end-string.js duration=5 c=50 len=4 benchmarker='wrk'                           |β–“β–“β–‘β–‘β–‘                +0.89% 
http/end-string.js duration=5 c=50 len=64 benchmarker='wrk'                          |β–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–‘β–‘         +3.42% ***
http/end-string.js duration=5 c=50 len=1024 benchmarker='wrk'                        |β–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–‘β–‘  +5.66% ***
http/end-string.js duration=5 c=50 len=16384 benchmarker='wrk'                   β–‘β–‘β–“β–“|                     -0.58% 
http/end-string.js duration=5 c=50 len=102400 benchmarker='wrk'                    β–‘β–‘|                     -0.12% 

Rates were scaled by 1000x into HdrHistogram (3 significant figures).
Use --scale to adjust precision if needed.

Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case, there are 5 comparisons, you can thus
expect the following amount of false-positive results:
  0.25 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.05 false positives, when considering a   1% risk acceptance (**, ***),
  0.01 false positives, when considering a 0.1% risk acceptance (***)

The stars above are per-benchmark and uncorrected. Adjusting for the size of
this comparison set (Holm-Bonferroni), 2 comparisons remain significant at 5%.

Also skip the flatten in Writev() and for strings larger than the stack
buffer.

Assisted-by: Claude
Signed-off-by: Guilherme AraΓΊjo <arauujogui@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

buffer Issues and PRs related to the buffer subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants