Skip to content

net: speed up BlockList.check for strings - #66166

Open
anonrig wants to merge 1 commit into
nodejs:mainfrom
anonrig:cursor/net-blocklist-fast-ead2
Open

anonrig wants to merge 1 commit into
nodejs:mainfrom
anonrig:cursor/net-blocklist-fast-ead2

Conversation

@anonrig

@anonrig anonrig commented Sep 21, 2026

Copy link
Copy Markdown
Member

net.BlockList.check(string) always lowercased the family and went through the slow checkString C++ binding. The documented families are ipv4 / ipv6, so skip toLowerCase() for those, and add a V8 Fast API for one-byte address strings (no heap allocation; stack-copy + uv_ip*_addr).

SocketAddress check() (already Fast API) is unchanged.

This change was implemented with assistance from a closed-source coding agent. I verified the source, official benchmark/net/net-blocklist.js results, and test-blocklist.js / test-blocklist-fast-api.js / test-net-blocklist.js.

Official benches

Three-run medians, n=1e6, operation=check, ruleType=address:

bench main this PR delta
check hit ruleCount=10 13.04M 17.50M +34%
check hit ruleCount=1000 14.66M 18.01M +23%
check miss ruleCount=10 11.13M 13.30M +19%
checkWithSocketAddress hit ruleCount=10 44.3M 44.1M unchanged

Tests

test-blocklist.js, test-blocklist-fast-api.js, test-net-blocklist.js.

@anonrig
anonrig marked this pull request as ready for review September 21, 2026 04:16
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/net

@nodejs-github-bot nodejs-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 Sep 21, 2026
@cursor
cursor Bot force-pushed the cursor/net-blocklist-fast-ead2 branch 4 times, most recently from e010e5d to f7a2973 Compare September 21, 2026 05:23
Skip toLowerCase() for the documented ipv4/ipv6 families and add a
V8 Fast API for one-byte address strings. The FastOneByteString
callback takes FastApiCallbackOptions so V8 can use the fast path
reliably (required for string-shaped Fast API arguments).

Overlong IPv6 zone ids are parsed like uv_ip6_addr (unknown zone
is scope_id 0; address parts 40+ chars stay a miss). IPv4 zone
suffixes stay a miss, matching uv_ip4_addr.

Assisted-by: a closed-source coding agent
Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com>
@cursor
cursor Bot force-pushed the cursor/net-blocklist-fast-ead2 branch from f7a2973 to 8af2ba0 Compare September 21, 2026 05:28
@codecov

codecov Bot commented Sep 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 67.56757% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.28%. Comparing base (dd5dfb5) to head (8af2ba0).
⚠️ Report is 38 commits behind head on main.

Files with missing lines Patch % Lines
src/node_sockaddr.cc 53.84% 10 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66166      +/-   ##
==========================================
+ Coverage   90.27%   90.28%   +0.01%     
==========================================
  Files         790      790              
  Lines      271651   272077     +426     
  Branches    51842    51947     +105     
==========================================
+ Hits       245228   245647     +419     
- Misses      16928    16938      +10     
+ Partials     9495     9492       -3     
Files with missing lines Coverage Δ
lib/internal/blocklist.js 97.20% <100.00%> (+0.05%) ⬆️
src/node_sockaddr.h 51.28% <ø> (ø)
src/node_sockaddr.cc 73.99% <53.84%> (-0.70%) ⬇️

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

@anonrig anonrig added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 21, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 21, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Sign up for free to 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-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants