FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fs: dispatch ASCII and Latin1 via simdutf in ReadFileUtf8 by mertcanaltin · Pull Request #63370 · nodejs/node · GitHub

/ node Public

fs: dispatch ASCII and Latin1 via simdutf in ReadFileUtf8 - #63370

Closed
mertcanaltin wants to merge 1 commit into
nodejs:mainfrom
mertcanaltin:mert/readfileutf8-zero-copy-fast-path
ConversationCommits (1)ChecksFiles changed
Closed

fs: dispatch ASCII and Latin1 via simdutf in ReadFileUtf8#63370
mertcanaltin wants to merge 1 commit into
nodejs:mainfrom
mertcanaltin:mert/readfileutf8-zero-copy-fast-path

Conversation

mertcanaltin commented May 16, 2026

Copy link
Copy Markdown
Member

In fs.readFileSync(path, 'utf8'), I dispatch the V8 string creation through simdutf. ASCII and Latin1-fits utf-8 use one-byte V8 strings, multibyte goes through simdutf to UTF-16, invalid utf-8 falls back to V8.

@nodejs/fs, @anonrig @addaleax, @lemire @mcollina

Bench results (gist): https://gist.github.com/mertcanaltin/a0096c3fad387d0bace821938754af44

fs/readfile-utf8-fastpath.js
    latin1     path 4MB     +803%
    latin1     path 256KB   +725%
    utf8_mixed path 4MB     +453%
    utf8_mixed path 256KB   +358%
    latin1     path 16KB    +309%
    latin1     fd   4MB     +238%

nodejs-github-bot commented May 16, 2026

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/performance

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels May 16, 2026

codecov Bot commented May 17, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 57.14286% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.06%. Comparing base (9de9b9f) to head (deaa5e1).
⚠️ Report is 134 commits behind head on main.

Files with missing lines Patch % Lines
src/util.cc 57.14% 2 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #63370      +/-   ##
==========================================
- Coverage   90.07%   90.06%   -0.02%     
==========================================
  Files         714      714              
  Lines      225564   225740     +176     
  Branches    42656    42718      +62     
==========================================
+ Hits       203177   203306     +129     
- Misses      14189    14221      +32     
- Partials     8198     8213      +15
Files with missing lines Coverage Δ
src/util-inl.h 83.38% <ø> (+0.44%) ⬆️
src/util.h 90.98% <ø> (ø)
src/util.cc 86.95% <57.14%> (-0.49%) ⬇️

... and 65 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.

addaleax requested changes May 18, 2026
View reviewed changes

addaleax left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Why would we make this change specifically for fs and not as part of the general ToV8Value() conversion or StringBytes?

src: dispatch ToV8Value(string_view) via StringBytes::Encode
deaa5e1 
Signed-off-by: Mert Can Altin <mertgold60@gmail.com>
mertcanaltin force-pushed the mert/readfileutf8-zero-copy-fast-path branch from cdeaa76 to deaa5e1 Compare May 18, 2026 18:56

mertcanaltin commented May 18, 2026

Copy link
Copy Markdown
Member Author

Why would we make this change specifically for fs and not as part of the general ToV8Value() conversion or StringBytes?

Sure, I applied now,

new benchmark results:

have a one regresion, I solving today,
fs/readfile-utf8-fastpath.js n=3000 source='fd' content='utf8_mixed' size=16384 *** -9.65 % ±2.96% ±3.94% ±5.13%

➜  node git:(mert/readfileutf8-zero-copy-fast-path) ✗ node-benchmark-compare ./result.csv
                                                                                    confidence improvement accuracy (*)    (**)   (***)
fs/readfile-utf8-fastpath.js n=3000 source='fd' content='ascii' size=1024                           0.11 %       ±4.46%  ±5.99%  ±7.89%
fs/readfile-utf8-fastpath.js n=3000 source='fd' content='ascii' size=16384                          2.08 %       ±5.10%  ±6.80%  ±8.88%
fs/readfile-utf8-fastpath.js n=3000 source='fd' content='ascii' size=262144                        -3.75 %       ±3.75%  ±5.00%  ±6.50%
fs/readfile-utf8-fastpath.js n=3000 source='fd' content='ascii' size=4194304               ***      4.98 %       ±2.36%  ±3.14%  ±4.08%
fs/readfile-utf8-fastpath.js n=3000 source='fd' content='ascii' size=64                             0.11 %       ±3.45%  ±4.60%  ±6.00%
fs/readfile-utf8-fastpath.js n=3000 source='fd' content='latin1' size=1024                          1.61 %       ±4.25%  ±5.68%  ±7.45%
fs/readfile-utf8-fastpath.js n=3000 source='fd' content='latin1' size=16384                 **      6.43 %       ±3.72%  ±4.96%  ±6.46%
fs/readfile-utf8-fastpath.js n=3000 source='fd' content='latin1' size=262144               ***     27.97 %       ±4.38%  ±5.84%  ±7.63%
fs/readfile-utf8-fastpath.js n=3000 source='fd' content='latin1' size=4194304              ***    221.38 %       ±4.45%  ±5.97%  ±7.87%
fs/readfile-utf8-fastpath.js n=3000 source='fd' content='latin1' size=64                            1.45 %       ±2.80%  ±3.73%  ±4.85%
fs/readfile-utf8-fastpath.js n=3000 source='fd' content='utf8_mixed' size=1024                      3.00 %       ±5.51%  ±7.37%  ±9.67%
fs/readfile-utf8-fastpath.js n=3000 source='fd' content='utf8_mixed' size=16384            ***     -9.65 %       ±2.96%  ±3.94%  ±5.13%
fs/readfile-utf8-fastpath.js n=3000 source='fd' content='utf8_mixed' size=262144           ***     17.65 %       ±3.20%  ±4.26%  ±5.55%
fs/readfile-utf8-fastpath.js n=3000 source='fd' content='utf8_mixed' size=4194304          ***    126.65 %       ±3.25%  ±4.34%  ±5.68%
fs/readfile-utf8-fastpath.js n=3000 source='fd' content='utf8_mixed' size=64                        0.33 %       ±4.22%  ±5.64%  ±7.38%
fs/readfile-utf8-fastpath.js n=3000 source='path' content='ascii' size=1024                         2.40 %       ±2.93%  ±3.95%  ±5.23%
fs/readfile-utf8-fastpath.js n=3000 source='path' content='ascii' size=16384                        0.24 %       ±4.38%  ±5.83%  ±7.60%
fs/readfile-utf8-fastpath.js n=3000 source='path' content='ascii' size=262144              ***     10.85 %       ±1.06%  ±1.42%  ±1.86%
fs/readfile-utf8-fastpath.js n=3000 source='path' content='ascii' size=4194304             ***     27.92 %       ±3.49%  ±4.68%  ±6.18%
fs/readfile-utf8-fastpath.js n=3000 source='path' content='ascii' size=64                           0.79 %       ±2.69%  ±3.62%  ±4.79%
fs/readfile-utf8-fastpath.js n=3000 source='path' content='latin1' size=1024               ***     29.48 %       ±3.18%  ±4.27%  ±5.65%
fs/readfile-utf8-fastpath.js n=3000 source='path' content='latin1' size=16384              ***    264.74 %       ±5.63%  ±7.57% ±10.00%
fs/readfile-utf8-fastpath.js n=3000 source='path' content='latin1' size=262144             ***    575.46 %       ±7.93% ±10.65% ±14.06%
fs/readfile-utf8-fastpath.js n=3000 source='path' content='latin1' size=4194304            ***    823.55 %       ±4.83%  ±6.46%  ±8.47%
fs/readfile-utf8-fastpath.js n=3000 source='path' content='latin1' size=64                          3.02 %       ±3.61%  ±4.86%  ±6.44%
fs/readfile-utf8-fastpath.js n=3000 source='path' content='utf8_mixed' size=1024           ***     18.36 %       ±3.05%  ±4.11%  ±5.43%
fs/readfile-utf8-fastpath.js n=3000 source='path' content='utf8_mixed' size=16384          ***    136.45 %       ±2.53%  ±3.37%  ±4.38%
fs/readfile-utf8-fastpath.js n=3000 source='path' content='utf8_mixed' size=262144         ***    260.71 %       ±4.50%  ±5.99%  ±7.80%
fs/readfile-utf8-fastpath.js n=3000 source='path' content='utf8_mixed' size=4194304        ***    330.51 %       ±2.51%  ±3.38%  ±4.48%
fs/readfile-utf8-fastpath.js n=3000 source='path' content='utf8_mixed' size=64                      3.01 %       ±3.61%  ±4.86%  ±6.43%

Be aware that when doing many comparisons the risk of a false-positive result increases.
In this case, there are 30 comparisons, you can thus expect the following amount of false-positive results:
  1.50 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.30 false positives, when considering a   1% risk acceptance (**, ***),
  0.03 false positives, when considering a 0.1% risk acceptance (***)
➜  node git:(mert/readfileutf8-zero-copy-fast-path) ✗

mertcanaltin commented May 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

-9.65 % on fd utf8_mixed 16 KiB is addressed in #63385 it collapses the StringBytes UTF-16 path to a single simdutf pass for buffers ≤ 1 MiB. Both PRs together remove the regression @addaleax fyi

mertcanaltin mentioned this pull request May 18, 2026
Merged
mertcanaltin requested a review from addaleax May 18, 2026 21:44
addaleax added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels May 19, 2026
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 19, 2026

nodejs-github-bot commented May 19, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/73552/

gurgunday approved these changes May 20, 2026
View reviewed changes

gurgunday left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Lgtm

nodejs-github-bot commented May 26, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/73736/

addaleax pushed a commit that referenced this pull request May 27, 2026
src: dispatch ToV8Value(string_view) via StringBytes::Encode
770385a 
Signed-off-by: Mert Can Altin <mertgold60@gmail.com>
PR-URL: #63370
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Daniel Lemire <daniel@lemire.me>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>

addaleax commented May 27, 2026

Copy link
Copy Markdown
Member

Landed in 770385a

addaleax closed this May 27, 2026
aduh95 pushed a commit that referenced this pull request May 28, 2026
src: dispatch ToV8Value(string_view) via StringBytes::Encode
31268b8 
Signed-off-by: Mert Can Altin <mertgold60@gmail.com>
PR-URL: #63370
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Daniel Lemire <daniel@lemire.me>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
github-actions Bot mentioned this pull request May 30, 2026
Merged
aduh95 pushed a commit that referenced this pull request May 30, 2026
src: dispatch ToV8Value(string_view) via StringBytes::Encode
2906fa8 
Signed-off-by: Mert Can Altin <mertgold60@gmail.com>
PR-URL: #63370
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Daniel Lemire <daniel@lemire.me>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
aduh95 added dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. dont-land-on-v24.x PRs that should not land on the v24.x-staging branch and should not be released in v24.x. labels Aug 6, 2026
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

lemire lemire approved these changes
addaleax addaleax approved these changes
gurgunday gurgunday approved these changes

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. dont-land-on-v24.x PRs that should not land on the v24.x-staging branch and should not be released in v24.x. fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL