Skip to content

src: use simdutf for two-byte string utf8 conversion in utf8 value - #62248

Open
mertcanaltin wants to merge 2 commits into
nodejs:mainfrom
mertcanaltin:mert/use-simdutf-for-two-byte-utf8
Open

src: use simdutf for two-byte string utf8 conversion in utf8 value#62248
mertcanaltin wants to merge 2 commits into
nodejs:mainfrom
mertcanaltin:mert/use-simdutf-for-two-byte-utf8

Conversation

@mertcanaltin

@mertcanaltinmertcanaltin commented Mar 14, 2026

Copy link
Copy Markdown
Member

latin-1 already used simdutf #61696,

I changed WriteUtf8V2 to simdutf for two-byte strings.

➜ node git:(mert/use-simdutf-for-two-byte-utf8) ✗ node-benchmark-compare ./result.csv
confidence improvement accuracy (*) (**) (***)
util/utf8-value.js n=5000000 type='ascii' -0.11 % ±2.64% ±3.69% ±5.18%
util/utf8-value.js n=5000000 type='mixed'*** 49.60 % ±1.90% ±2.67% ±3.78%
util/utf8-value.js n=5000000 type='three_bytes'*** 31.10 % ±2.42% ±3.34% ±4.60%
util/utf8-value.js n=5000000 type='two_bytes'*** 42.75 % ±1.42% ±1.94% ±2.66%
Be aware that when doing many comparisons the risk of a false-positive result increases.
In this case, there are 4 comparisons, you can thus expect the following amount of false-positive results:
0.20 false positives, when considering a 5% risk acceptance (*, **, ***),
0.04 false positives, when considering a 1% risk acceptance (**, ***),
0.00 false positives, when considering a 0.1% risk acceptance (***)

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/performance

@nodejs-github-botnodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Mar 14, 2026
@mertcanaltin
mertcanaltinforce-pushed the mert/use-simdutf-for-two-byte-utf8 branch from f6d6f82 to d459594CompareMarch 14, 2026 08:31
@codecov

codecovBot commented Mar 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.68%. Comparing base (da5843b) to head (19f636f).
⚠️ Report is 1305 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #62248 +/- ##
==========================================
+ Coverage 89.67% 89.68% +0.01% 
==========================================
Files 676 676 Lines 206469 206700 +231 Branches 39537 39584 +47 ==========================================
+ Hits 185157 185388 +231 + Misses 13448 13441 -7 - Partials 7864 7871 +7 
Files with missing linesCoverage Δ
src/util.cc87.64% <100.00%> (+0.20%)⬆️

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

Comment threadsrc/util.cc Outdated
@mertcanaltin
mertcanaltinforce-pushed the mert/use-simdutf-for-two-byte-utf8 branch 4 times, most recently from b8cc5e8 to dcbf493CompareMarch 14, 2026 17:12
@mertcanaltin
mertcanaltinforce-pushed the mert/use-simdutf-for-two-byte-utf8 branch from dcbf493 to fb16b2dCompareMarch 14, 2026 17:12
@mcollinamcollina added the needs-benchmark-ci PR that need a benchmark CI run. label Mar 21, 2026
Comment threadsrc/util.cc Outdated

@lemirelemire 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.

this look fine, see my comment.

@anonriganonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 9, 2026
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 9, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been marked as stale due to 90 days of inactivity.
It will be automatically closed in 30 days if no further activity occurs. If this is still relevant, please leave a comment or update it to keep it open.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++Issues and PRs that require attention from people who are familiar with C++.needs-benchmark-ciPR that need a benchmark CI run.needs-ciPRs that need a full CI run.stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@mertcanaltin@nodejs-github-bot@lemire@gurgunday@mcollina@anonrig