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

Do not idle in the event loop if there are pending immediate tasks #17901

Merged
merged 6 commits into from
Mar 7, 2025

Conversation

190n
Copy link
Contributor

@190n 190n commented Mar 4, 2025

What does this PR do?

Fixes #17725. The presence of a setInterval timer would make us idle in us_loop_run_bun_tick on every run of the event loop for a dozen ms or so, but that's not a good idea when we have immediate tasks to run. This PR makes us skip the timeout if there are immediate tasks, similar to what we already do if there is a short timer (in terms of the effect on idling, immediates are like 0ms timers).

Todo:

  • see how much we can benefit JSZip
  • fix Windows failure

How did you verify your code works?

Ran the node:timers tests, and extended our own tests to cover a use case like #17725.

@robobun
Copy link

robobun commented Mar 4, 2025

Updated 9:23 PM PT - Mar 6th, 2025

@190n, your commit 6005944 has 2 failures in Build #12808:


🧪   try this PR locally:

bunx bun-pr 17901

@dylan-conway
Copy link
Member

Is it possible this fixes #11054?

@190n
Copy link
Contributor Author

190n commented Mar 4, 2025

It might, I need to test that.

@Jarred-Sumner Jarred-Sumner merged commit 1d32e78 into main Mar 7, 2025
38 of 48 checks passed
@Jarred-Sumner Jarred-Sumner deleted the ben/setimmediate-perf branch March 7, 2025 04:35
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.

setInterval/setTimeout causes a huge slowdown for setImmediate
4 participants