Skip to content

tls: optimize convertProtocols by using Uint8 and removing reduce #57671

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gurgunday
Copy link
Member

Before:

tls/convertprotocols.js
tls/convertprotocols.js n=1: 12,257.4555973671
tls/convertprotocols.js n=50000: 2,620,556.4730706117

gurgunday@Gurguns-MacBook-Air node % ./out/Release/node benchmark/run.js tls

tls/convertprotocols.js
tls/convertprotocols.js n=1: 11,718.658447980873
tls/convertprotocols.js n=50000: 2,531,944.659589409

gurgunday@Gurguns-MacBook-Air node % ./out/Release/node benchmark/run.js tls

tls/convertprotocols.js
tls/convertprotocols.js n=1: 12,409.56529292779
tls/convertprotocols.js n=50000: 2,641,926.539330387

After:

gurgunday@Gurguns-MacBook-Air node % ./out/Release/node benchmark/run.js tls

tls/convertprotocols.js
tls/convertprotocols.js n=1: 13,706.51607774336
tls/convertprotocols.js n=50000: 2,533,211.4149343553

gurgunday@Gurguns-MacBook-Air node % ./out/Release/node benchmark/run.js tls

tls/convertprotocols.js
tls/convertprotocols.js n=1: 13,179.571663920922
tls/convertprotocols.js n=50000: 2,795,065.7251320067

gurgunday@Gurguns-MacBook-Air node % ./out/Release/node benchmark/run.js tls

tls/convertprotocols.js
tls/convertprotocols.js n=1: 13,528.687582017668
tls/convertprotocols.js n=50000: 2,689,298.5120810554

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/crypto
  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. tls Issues and PRs related to the tls subsystem. labels Mar 29, 2025
Copy link

codecov bot commented Mar 29, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.22%. Comparing base (af75d04) to head (79cebb0).
Report is 49 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #57671      +/-   ##
==========================================
- Coverage   90.23%   90.22%   -0.01%     
==========================================
  Files         630      630              
  Lines      185055   185059       +4     
  Branches    36221    36219       -2     
==========================================
- Hits       166984   166973      -11     
- Misses      11043    11045       +2     
- Partials     7028     7041      +13     
Files with missing lines Coverage Δ
lib/tls.js 95.97% <100.00%> (+0.04%) ⬆️

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

@gurgunday gurgunday added the needs-benchmark-ci PR that need a benchmark CI run. label Apr 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-benchmark-ci PR that need a benchmark CI run. needs-ci PRs that need a full CI run. tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants