Skip to content

Conversation

JinhyeokFang
Copy link
Contributor

Description

Optimize checkIsHttpToken performance by using a pre-computed lookup table for strings shorter than 10 characters instead of regex

Benchmark Results

Shows significant performance improvements for short strings, with minimal impact on longer strings:

http/check_is_http_token.js n=1000000 key=':'                                                                         ***    188.25 %       ±1.05% ±1.41% ±1.86%
http/check_is_http_token.js n=1000000 key=':alternate-protocol'                                                       ***     -3.76 %       ±0.30% ±0.40% ±0.52%
http/check_is_http_token.js n=1000000 key='((((())))'                                                                 ***    188.71 %       ±1.01% ±1.35% ±1.79%
http/check_is_http_token.js n=1000000 key='@@'                                                                        ***    187.33 %       ±2.87% ±3.86% ±5.12%
http/check_is_http_token.js n=1000000 key='中文呢'                                                                       ***    108.16 %       ±0.91% ±1.23% ±1.61%
http/check_is_http_token.js n=1000000 key='Accept-Ranges'                                                             ***     -2.18 %       ±0.73% ±0.97% ±1.27%
http/check_is_http_token.js n=1000000 key='alt-svc'                                                                   ***     83.63 %       ±0.46% ±0.61% ±0.80%
http/check_is_http_token.js n=1000000 key='alternate-protocol:'                                                         *     -1.31 %       ±1.19% ±1.61% ±2.13%
http/check_is_http_token.js n=1000000 key='alternate-protocol'                                                        ***     -2.28 %       ±0.45% ±0.60% ±0.79%
http/check_is_http_token.js n=1000000 key='Cache-Control'                                                             ***     -2.47 %       ±0.34% ±0.46% ±0.60%
http/check_is_http_token.js n=1000000 key='Connection'                                                                ***     -1.86 %       ±0.52% ±0.69% ±0.91%
http/check_is_http_token.js n=1000000 key='Content-Encoding'                                                          ***     -2.23 %       ±0.65% ±0.86% ±1.12%
http/check_is_http_token.js n=1000000 key='content-length'                                                            ***     -2.61 %       ±0.98% ±1.32% ±1.74%
http/check_is_http_token.js n=1000000 key='Content-Location'                                                          ***     -2.24 %       ±0.44% ±0.58% ±0.76%
http/check_is_http_token.js n=1000000 key='content-type'                                                              ***     -1.34 %       ±0.37% ±0.49% ±0.64%
http/check_is_http_token.js n=1000000 key='Content-Type'                                                              ***     -1.45 %       ±0.31% ±0.42% ±0.54%
http/check_is_http_token.js n=1000000 key='date'                                                                      ***    145.58 %       ±0.77% ±1.03% ±1.34%
http/check_is_http_token.js n=1000000 key='ETag'                                                                      ***    145.77 %       ±0.56% ±0.75% ±0.98%
http/check_is_http_token.js n=1000000 key='Expires'                                                                   ***     83.44 %       ±0.45% ±0.60% ±0.78%
http/check_is_http_token.js n=1000000 key='Keep-Alive'                                                                ***     -1.36 %       ±0.41% ±0.55% ±0.71%
http/check_is_http_token.js n=1000000 key='Last-Modified'                                                             ***     -2.43 %       ±0.38% ±0.51% ±0.67%
http/check_is_http_token.js n=1000000 key='location'                                                                  ***     71.30 %       ±1.22% ±1.64% ±2.16%
http/check_is_http_token.js n=1000000 key='server'                                                                    ***     99.77 %       ±0.50% ±0.66% ±0.86%
http/check_is_http_token.js n=1000000 key='Server'                                                                    ***     99.28 %       ±0.53% ±0.71% ±0.92%
http/check_is_http_token.js n=1000000 key='status'                                                                    ***     98.86 %       ±0.57% ±0.76% ±0.99%
http/check_is_http_token.js n=1000000 key='TCN'                                                                       ***    224.62 %       ±1.33% ±1.78% ±2.34%
http/check_is_http_token.js n=1000000 key='Transfer-Encoding'                                                         ***     -1.98 %       ±0.48% ±0.64% ±0.84%
http/check_is_http_token.js n=1000000 key='Vary'                                                                      ***    145.11 %       ±0.85% ±1.13% ±1.49%
http/check_is_http_token.js n=1000000 key='version'                                                                   ***     83.58 %       ±0.46% ±0.61% ±0.80%
http/check_is_http_token.js n=1000000 key='x-frame-options'                                                           ***     -2.59 %       ±0.66% ±0.88% ±1.15%
http/check_is_http_token.js n=1000000 key='x-xss-protection'                                                          ***     -2.18 %       ±0.39% ±0.52% ±0.68%

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added http Issues or PRs related to the http subsystem. needs-ci PRs that need a full CI run. labels Sep 9, 2025
Copy link

codecov bot commented Sep 9, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.30%. Comparing base (861d624) to head (b1f8b70).
⚠️ Report is 33 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #59832      +/-   ##
==========================================
+ Coverage   88.26%   88.30%   +0.03%     
==========================================
  Files         701      701              
  Lines      206774   206810      +36     
  Branches    39778    39780       +2     
==========================================
+ Hits       182514   182622     +108     
+ Misses      16298    16210      -88     
- Partials     7962     7978      +16     
Files with missing lines Coverage Δ
lib/_http_common.js 100.00% <100.00%> (ø)

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

Use lookup table instead of regex for strings shorter than 10
characters to improve performance for common short header names
while maintaining compatibility.
Copy link
Contributor

@Ethan-Arrowood Ethan-Arrowood left a comment

Choose a reason for hiding this comment

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

I like it! Before we merge let's run benchmarks and be sure

@daeyeon daeyeon added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Sep 12, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 12, 2025
@nodejs-github-bot
Copy link
Collaborator

@daeyeon
Copy link
Member

daeyeon commented Sep 13, 2025

Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1734/console

08:39:30 http/check_is_http_token.js n=1000000 key=':'                                                            ***    224.81 %       ±8.27% ±11.01% ±14.33%
08:39:30 http/check_is_http_token.js n=1000000 key=':alternate-protocol'                                                  -3.25 %       ±5.76%  ±7.67%  ±9.98%
08:39:30 http/check_is_http_token.js n=1000000 key='((((())))'                                                    ***    236.05 %      ±15.19% ±20.37% ±26.86%
08:39:30 http/check_is_http_token.js n=1000000 key='@@'                                                           ***    220.11 %       ±7.32%  ±9.77% ±12.77%
08:39:30 http/check_is_http_token.js n=1000000 key='Accept-Ranges'                                                        -0.35 %       ±3.43%  ±4.56%  ±5.93%
08:39:30 http/check_is_http_token.js n=1000000 key='alt-svc'                                                      ***     65.58 %       ±5.11%  ±6.83%  ±8.94%
08:39:30 http/check_is_http_token.js n=1000000 key='alternate-protocol:'                                            *     -2.01 %       ±1.79%  ±2.38%  ±3.10%
08:39:30 http/check_is_http_token.js n=1000000 key='alternate-protocol'                                                    1.43 %       ±2.28%  ±3.04%  ±3.96%
08:39:30 http/check_is_http_token.js n=1000000 key='Cache-Control'                                                        -0.14 %       ±3.11%  ±4.14%  ±5.39%
08:39:30 http/check_is_http_token.js n=1000000 key='Connection'                                                            1.48 %       ±2.74%  ±3.64%  ±4.75%
08:39:30 http/check_is_http_token.js n=1000000 key='Content-Encoding'                                                      1.61 %       ±2.93%  ±3.90%  ±5.08%
08:39:30 http/check_is_http_token.js n=1000000 key='content-length'                                                       -1.58 %       ±2.55%  ±3.39%  ±4.43%
08:39:30 http/check_is_http_token.js n=1000000 key='Content-Location'                                                      1.38 %       ±2.52%  ±3.35%  ±4.36%
08:39:30 http/check_is_http_token.js n=1000000 key='content-type'                                                         -0.38 %       ±3.37%  ±4.48%  ±5.84%
08:39:30 http/check_is_http_token.js n=1000000 key='Content-Type'                                                         -0.84 %       ±3.33%  ±4.43%  ±5.76%
08:39:30 http/check_is_http_token.js n=1000000 key='date'                                                         ***    155.93 %       ±6.71%  ±9.01% ±11.90%
08:39:30 http/check_is_http_token.js n=1000000 key='ETag'                                                         ***    167.04 %       ±4.69%  ±6.26%  ±8.19%
08:39:30 http/check_is_http_token.js n=1000000 key='Expires'                                                      ***     66.65 %       ±4.58%  ±6.10%  ±7.95%
08:39:30 http/check_is_http_token.js n=1000000 key='Keep-Alive'                                                           -1.91 %       ±2.57%  ±3.42%  ±4.45%
08:39:30 http/check_is_http_token.js n=1000000 key='Last-Modified'                                                        -1.97 %       ±2.76%  ±3.67%  ±4.77%
08:39:30 http/check_is_http_token.js n=1000000 key='location'                                                     ***     47.84 %       ±4.54%  ±6.07%  ±7.94%
08:39:30 http/check_is_http_token.js n=1000000 key='server'                                                       ***     75.46 %       ±4.14%  ±5.53%  ±7.24%
08:39:30 http/check_is_http_token.js n=1000000 key='Server'                                                       ***     79.92 %       ±5.56%  ±7.44%  ±9.77%
08:39:30 http/check_is_http_token.js n=1000000 key='status'                                                       ***     79.03 %       ±5.31%  ±7.10%  ±9.31%
08:39:30 http/check_is_http_token.js n=1000000 key='TCN'                                                          ***    215.88 %       ±6.17%  ±8.27% ±10.91%
08:39:30 http/check_is_http_token.js n=1000000 key='Transfer-Encoding'                                                    -0.99 %       ±2.16%  ±2.87%  ±3.74%
08:39:30 http/check_is_http_token.js n=1000000 key='Vary'                                                         ***    157.55 %       ±6.81%  ±9.11% ±11.98%
08:39:30 http/check_is_http_token.js n=1000000 key='version'                                                      ***     61.80 %       ±5.12%  ±6.85%  ±8.97%
08:39:30 http/check_is_http_token.js n=1000000 key='x-frame-options'                                                       0.11 %       ±2.85%  ±3.79%  ±4.93%
08:39:30 http/check_is_http_token.js n=1000000 key='x-xss-protection'                                                      2.38 %       ±2.85%  ±3.80%  ±4.94%
08:39:30 http/check_is_http_token.js n=1000000 key='中文呢'                                                       ***    135.25 %       ±5.50%  ±7.37%  ±9.68%

@@ -214,7 +238,19 @@ const tokenRegExp = /^[\^_`a-zA-Z\-0-9!#$%&'*+.|~]+$/;
* @returns {boolean}
*/
function checkIsHttpToken(val) {
return tokenRegExp.test(val);
if (val.length >= 10) {
Copy link
Member

Choose a reason for hiding this comment

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

We can probably safely increase this to 30. There are a fair number of common header names that are over 10 characters (e.g. content-length, authorization, content-type, content-security-policy, etc)

Copy link
Member

Choose a reason for hiding this comment

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

When looking at the benchmark results, it seems that the benefit over a regex is likely zero or negative for more than ten characters.

@BridgeAR BridgeAR added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 14, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 14, 2025
@nodejs-github-bot nodejs-github-bot merged commit c8c6bfa into nodejs:main Sep 14, 2025
64 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in c8c6bfa

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. http Issues or PRs related to the http subsystem. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants