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

util: optimize toASCIILower function using V8's native toLowerCase by mertcanaltin · Pull Request #61107 · nodejs/node · GitHub

/ node Public

util: optimize toASCIILower function using V8's native toLowerCase - #61107

Merged
nodejs-github-bot merged 4 commits into
nodejs:mainfrom
mertcanaltin:mert/perf/mime-toasciilower
Dec 27, 2025
ConversationCommits (4)ChecksFiles changed
Merged

util: optimize toASCIILower function using V8's native toLowerCase#61107
nodejs-github-bot merged 4 commits into
nodejs:mainfrom
mertcanaltin:mert/perf/mime-toasciilower

Conversation

mertcanaltin commented Dec 18, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

benchmark results (mimetype-instantiation.js):

application/ecmascript +54%
text/html charset=gbk +24%
text/html long...=x charset=gbk +21%

nodejs-github-bot added the needs-ci PRs that need a full CI run. label Dec 18, 2025
mertcanaltin force-pushed the mert/perf/mime-toasciilower branch from bf24628 to b391b73 Compare December 18, 2025 08:49
mertcanaltin changed the title perf: optimize toASCIILower function using V8's native toLowerCase util: optimize toASCIILower function using V8's native toLowerCase Dec 18, 2025
mertcanaltin force-pushed the mert/perf/mime-toasciilower branch from b391b73 to 6b08d1a Compare December 18, 2025 08:50
mertcanaltin force-pushed the mert/perf/mime-toasciilower branch from 6b08d1a to 57b817f Compare December 18, 2025 08:55
ChALkeR reviewed Dec 18, 2025
View reviewed changes

ChALkeR left a comment •
edited
Loading

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

This is not identical (and there is a reason why the function checks character ranges)

This function is supposed to do ASCII lowercasing, not Unicode lowercasing

TextDecoder has the same issue for label normalization btw, as it incorrectly uses .toLowerCase at

node/lib/internal/encoding.js

Line 332 in b1c01fc

return encodings.get(trimAsciiWhitespace(label.toLowerCase()));

See spec: https://tc39.es/ecma262/#sec-string.prototype.tolowercase and notes there

As an example, '\u212A'.toLowerCase() === 'k'

toLowerCase can convert non-ascii characters into ascii characters (which makes e.g. current TextDecoder label handling incorrect, among all its other bugs)

text/html charset=gb\u212A should not turn into text/html charset=gbk

ChALkeR commented Dec 18, 2025

Copy link
Copy Markdown
Member

What would be helpful is to move toASCIILower out to some helper lib and to fix TextDecoder label handling to use it instead of .toLowerCase.

mertcanaltin commented Dec 18, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

What would be helpful is to move toASCIILower out to some helper lib and to fix TextDecoder label handling to use it instead of .toLowerCase.

Thanks I will try this idea

ChALkeR reviewed Dec 18, 2025
View reviewed changes
Comment thread lib/internal/mime.js Outdated
mertcanaltin and others added 3 commits December 18, 2025 12:46

codecov Bot commented Dec 18, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.52%. Comparing base (4f24aff) to head (34692cf).
⚠️ Report is 58 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #61107      +/-   ##
==========================================
- Coverage   88.53%   88.52%   -0.01%     
==========================================
  Files         703      703              
  Lines      208546   208548       +2     
  Branches    40217    40224       +7     
==========================================
- Hits       184634   184620      -14     
- Misses      15926    15958      +32     
+ Partials     7986     7970      -16
Files with missing lines Coverage Δ
lib/internal/mime.js 98.48% <100.00%> (+<0.01%) ⬆️

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

mertcanaltin mentioned this pull request Dec 18, 2025
Merged
RafaelGSS added the needs-benchmark-ci PR that need a benchmark CI run. label Dec 19, 2025
RafaelGSS approved these changes Dec 19, 2025
View reviewed changes

RafaelGSS 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. With green CI and confirmed by benchmark CI.

RafaelGSS added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 19, 2025
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 19, 2025

nodejs-github-bot commented Dec 19, 2025

Copy link
Copy Markdown
Collaborator

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

addaleax added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Dec 19, 2025

nodejs-github-bot commented Dec 20, 2025

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Dec 26, 2025

Copy link
Copy Markdown
Collaborator

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

aduh95 added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Dec 27, 2025
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 27, 2025
nodejs-github-bot merged commit ed47077 into nodejs:main Dec 27, 2025
78 checks passed

nodejs-github-bot commented Dec 27, 2025

Copy link
Copy Markdown
Collaborator

Landed in ed47077

aduh95 pushed a commit that referenced this pull request Jan 9, 2026
util: optimize toASCIILower function using V8s native toLowerCase
304fd36 
PR-URL: #61107
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
aduh95 pushed a commit that referenced this pull request Jan 12, 2026
util: optimize toASCIILower function using V8s native toLowerCase
69663d1 
PR-URL: #61107
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Jan 13, 2026
util: optimize toASCIILower function using V8s native toLowerCase
09b5396 
PR-URL: #61107
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
github-actions Bot mentioned this pull request Jan 16, 2026
Merged
aduh95 pushed a commit that referenced this pull request Jan 19, 2026
util: optimize toASCIILower function using V8s native toLowerCase
bfee9d0 
PR-URL: #61107
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
github-actions Bot mentioned this pull request Feb 3, 2026
Merged
aduh95 pushed a commit that referenced this pull request Feb 17, 2026
util: optimize toASCIILower function using V8s native toLowerCase
7367a41 
PR-URL: #61107
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
aduh95 pushed a commit that referenced this pull request Feb 19, 2026
util: optimize toASCIILower function using V8s native toLowerCase
112142c 
PR-URL: #61107
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
aduh95 pushed a commit that referenced this pull request Feb 21, 2026
util: optimize toASCIILower function using V8s native toLowerCase
9c50361 
PR-URL: #61107
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
aduh95 mentioned this pull request Feb 22, 2026
Merged
aduh95 pushed a commit that referenced this pull request Feb 23, 2026
util: optimize toASCIILower function using V8s native toLowerCase
0bd291b 
PR-URL: #61107
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
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

ChALkeR ChALkeR approved these changes
addaleax addaleax approved these changes
cjihrig cjihrig approved these changes
RafaelGSS RafaelGSS 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. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-benchmark-ci PR that need a benchmark CI run. 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.

7 participants

Back | FazBrowse Home | New Git URL