Skip to content
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

priorityQueue: Prevent same tick setImmediate #1727

Merged
merged 1 commit into from Oct 17, 2020

Conversation

pkarimov
Copy link
Contributor

@pkarimov pkarimov commented Oct 13, 2020

This applies the same technique that was done for queue in PR #1448 to priorityQueue. The change prevents multiple calls to setImmediate for queue processing that originate from calls to push on the same tick. The added overhead of the change is fairly negligible -- one variable assignment and two conditions.

$ perf/benchmark.js --grep priorityQueue
Latest tag is  v3.2.0
Comparing v3.2.0 with current on Node v12.13.1
--------------------------------------
priorityQueue(10) v3.2.0 x 16,503 ops/sec ±2.79% (27 runs sampled), 0.0606ms per run
priorityQueue(10) current x 17,979 ops/sec ±4.99% (28 runs sampled), 0.0556ms per run
current is faster
--------------------------------------
priorityQueue(100) v3.2.0 x 4,261 ops/sec ±1.90% (28 runs sampled), 0.235ms per run
priorityQueue(100) current x 4,749 ops/sec ±0.90% (28 runs sampled), 0.211ms per run
current is faster
--------------------------------------
priorityQueue(1000) v3.2.0 x 509 ops/sec ±1.06% (32 runs sampled), 1.96ms per run
priorityQueue(1000) current x 556 ops/sec ±1.17% (32 runs sampled), 1.80ms per run
current is faster
--------------------------------------
priorityQueue(30000) v3.2.0 x 13.02 ops/sec ±3.95% (24 runs sampled), 76.8ms per run
priorityQueue(30000) current x 14.79 ops/sec ±2.11% (27 runs sampled), 67.6ms per run
current is faster
--------------------------------------
priorityQueue(50000) v3.2.0 x 7.21 ops/sec ±2.82% (14 runs sampled), 139ms per run
priorityQueue(50000) current x 8.93 ops/sec ±3.04% (17 runs sampled), 112ms per run
current is faster
--------------------------------------
current faster overall (182ms total vs. 218ms total)
current won more benchmarks (5 vs. 0)

@aearly aearly merged commit b0bc571 into caolan:master Oct 17, 2020
2 of 3 checks passed
@aearly
Copy link
Collaborator

@aearly aearly commented Oct 17, 2020

Nice, good work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants