Skip to content

lib: use primordials in freelist.js - #61208

Open
Ayoub-Mabrouk wants to merge 1 commit into
nodejs:mainfrom
Ayoub-Mabrouk:lib-use-primordials-freelist
Open

lib: use primordials in freelist.js#61208
Ayoub-Mabrouk wants to merge 1 commit into
nodejs:mainfrom
Ayoub-Mabrouk:lib-use-primordials-freelist

Conversation

@Ayoub-Mabrouk

Copy link
Copy Markdown
Contributor

Replace native methods with primordials for consistency and security. This improves protection against prototype pollution.

@nodejs-github-botnodejs-github-bot added the needs-ci PRs that need a full CI run. label Dec 29, 2025
@avivkelleravivkeller added the needs-benchmark-ci PR that need a benchmark CI run. label Dec 29, 2025
@Ayoub-Mabrouk
Ayoub-Mabroukforce-pushed the lib-use-primordials-freelist branch from bdbd96d to e1f3326CompareDecember 29, 2025 16:45
Replace native methods with primordials.
@Ayoub-Mabrouk
Ayoub-Mabroukforce-pushed the lib-use-primordials-freelist branch from e1f3326 to 06d364eCompareDecember 29, 2025 17:47
@codecov

codecovBot commented Dec 29, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.53%. Comparing base (05d6b9b) to head (06d364e).
⚠️ Report is 1083 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #61208 +/- ##
==========================================
- Coverage 88.53% 88.53% -0.01% 
==========================================
Files 703 703 Lines 208599 208615 +16 Branches 40229 40252 +23 ==========================================
+ Hits 184685 184692 +7 - Misses 15939 15941 +2 - Partials 7975 7982 +7 
Files with missing linesCoverage Δ
lib/internal/freelist.js100.00% <100.00%> (ø)

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

@aduh95

aduh95 commented Feb 17, 2026

Copy link
Copy Markdown
Contributor

Duplicate of #36565, until proven otherwise we should expect this to perform poorly

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

 confidence improvement accuracy (*) (**) (***)
misc/freelist.js n=100000 *** -71.89 % ±0.70% ±0.95% ±1.26%
Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case, there are 1 comparisons, you can thus
expect the following amount of false-positive results:
0.05 false positives, when considering a 5% risk acceptance (*, **, ***),
0.01 false positives, when considering a 1% risk acceptance (**, ***),
0.00 false positives, when considering a 0.1% risk acceptance (***)

@aduh95aduh95 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Performance regression makes it not worth it

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

Labels

needs-benchmark-ciPR that need a benchmark CI run.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@Ayoub-Mabrouk@aduh95@avivkeller@VoltrexKeyva@nodejs-github-bot