Skip to content

lib: optimize priority queue - #60039

Merged
aduh95 merged 1 commit into
nodejs:mainfrom
gurgunday:pq-opt
Oct 4, 2025
Merged

lib: optimize priority queue#60039
aduh95 merged 1 commit into
nodejs:mainfrom
gurgunday:pq-opt

Conversation

@gurgunday

@gurgundaygurgunday commented Sep 27, 2025

Copy link
Copy Markdown
Member

Further improves performance and fixes leftover issues from my last PR

Benchmarks:

main

node % ./node benchmark/run.js --filter priority util
util/priority-queue.js
util/priority-queue.js n=100000: 6,676,867.584962973

branch

node % ./node benchmark/run.js --filter priority util
util/priority-queue.js
util/priority-queue.js n=100000: 6,854,890.535966754

@nodejs-github-botnodejs-github-bot added the needs-ci PRs that need a full CI run. label Sep 27, 2025
@codecov

codecovBot commented Sep 27, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.45%. Comparing base (79f1999) to head (f980885).
⚠️ Report is 37 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #60039 +/- ##
==========================================
+ Coverage 88.43% 88.45% +0.01% 
==========================================
Files 703 703 Lines 207805 207800 -5 Branches 40026 40022 -4 ==========================================
+ Hits 183780 183801 +21 + Misses 16012 15979 -33 - Partials 8013 8020 +7 
Files with missing linesCoverage Δ
lib/internal/priority_queue.js100.00% <100.00%> (+0.80%)⬆️

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

@gurgundaygurgunday added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Sep 27, 2025
@gurgunday

gurgunday commented Sep 28, 2025

Copy link
Copy Markdown
MemberAuthor

constpos=++this.#size;
heap[pos]=value;
if(heap.length===pos)
heap.length*=2;

This was dead code before, the check never worked

Example coverage form another PR: https://app.codecov.io/gh/nodejs/node/pull/60060/blob/lib/internal/priority_queue.js?dropdown=coverage#L32

@BridgeAR

Copy link
Copy Markdown
Member

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

@BridgeARBridgeAR added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 29, 2025
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 29, 2025
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

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

@gurgundaygurgunday added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 29, 2025
@gurgundaygurgunday changed the title lib: priority_queue pre-grow array and avoid holey readslib: increase coverage in priority_queueSep 30, 2025
Comment threadlib/internal/priority_queue.js Outdated
@gurgunday

Copy link
Copy Markdown
MemberAuthor

@aduh95 PTAL, it's actually faster without the manual array resizing in modern node (V8 does it anyway)

So we just removed the dead code and made it packed (assigning to arr[length] keeps it packed)

@gurgunday

Copy link
Copy Markdown
MemberAuthor

@BridgeAR you can run the priority queue benchmark again by the way

This was faster for me locally

@aduh95

aduh95 commented Oct 3, 2025

Copy link
Copy Markdown
Contributor

Started a benchmark including the upcoming V8 update: https://github.com/aduh95/node/actions/runs/18221639411

EDIT: seems to be (significantly) faster across the board 🚀

@aduh95

Copy link
Copy Markdown
Contributor

Can you update the commit message? I don't think this has anything to do with coverage, instead you can use something like lib: optimize priority queue.

@gurgundaygurgunday changed the title lib: increase coverage in priority_queue@gurgunday lib: optimize priority queueOct 3, 2025
@gurgundaygurgunday changed the title @gurgunday lib: optimize priority queuelib: optimize priority queueOct 3, 2025
@gurgundaygurgunday added the performance Issues and PRs related to the performance of Node.js. label Oct 3, 2025
@gurgunday

gurgunday commented Oct 4, 2025

Copy link
Copy Markdown
MemberAuthor

I need a rerun of macOS and an approval

@aduh95

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@aduh95
aduh95 merged commit e978a63 into nodejs:mainOct 4, 2025
52 of 53 checks passed
@aduh95

Copy link
Copy Markdown
Contributor

Landed in e978a63

@gurgunday
gurgunday deleted the pq-opt branch October 4, 2025 20:17
targos pushed a commit that referenced this pull request Oct 6, 2025
PR-URL: #60039
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
aduh95 pushed a commit that referenced this pull request Oct 22, 2025
PR-URL: #60039
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.needs-ciPRs that need a full CI run.performanceIssues and PRs related to the performance of Node.js.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@gurgunday@BridgeAR@nodejs-github-bot@aduh95@juanarbol