-
Notifications
You must be signed in to change notification settings - Fork 31k
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
timers: recent performance regressions #41219
Labels
performance
Issues and PRs related to the performance of Node.js.
process
Issues and PRs related to the process subsystem.
regression
Issues related to regressions.
timers
Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout.
Comments
To mitigate this, we can traverse Line 161 in c8f5dd6
Line 426 in c8f5dd6
|
RaisinTen
added a commit
to RaisinTen/node
that referenced
this issue
Dec 18, 2021
The additional objects that were getting added and deleted from the activeTimersMap object were slowing down the rest of the timers code, so this change falls back to traversing the timerListMap and outstandingQueue objects to count the active timers inside process.getActiveResourcesInfo(). Fixes: nodejs#41219 Signed-off-by: Darshan Sen <[email protected]>
PR: #41231 |
And if we can't fix it, we can always just leave it documented as an API limitation. |
RaisinTen
added a commit
to RaisinTen/node
that referenced
this issue
Dec 31, 2021
The additional objects that were getting added and deleted from the activeTimersMap object were slowing down the rest of the timers code, so this change falls back to using the ref counts to count the active timers inside process.getActiveResourcesInfo(). Fixes: nodejs#41219 Signed-off-by: Darshan Sen <[email protected]>
nodejs-github-bot
pushed a commit
that referenced
this issue
Jan 2, 2022
The additional objects that were getting added and deleted from the activeTimersMap object were slowing down the rest of the timers code, so this change falls back to using the ref counts to count the active timers inside process.getActiveResourcesInfo(). Fixes: #41219 Signed-off-by: Darshan Sen <[email protected]> PR-URL: #41231 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
targos
pushed a commit
that referenced
this issue
Jan 14, 2022
The additional objects that were getting added and deleted from the activeTimersMap object were slowing down the rest of the timers code, so this change falls back to using the ref counts to count the active timers inside process.getActiveResourcesInfo(). Fixes: #41219 Signed-off-by: Darshan Sen <[email protected]> PR-URL: #41231 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
danielleadams
pushed a commit
that referenced
this issue
Jan 31, 2022
The additional objects that were getting added and deleted from the activeTimersMap object were slowing down the rest of the timers code, so this change falls back to using the ref counts to count the active timers inside process.getActiveResourcesInfo(). Fixes: #41219 Signed-off-by: Darshan Sen <[email protected]> PR-URL: #41231 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
Linkgoron
pushed a commit
to Linkgoron/node
that referenced
this issue
Jan 31, 2022
The additional objects that were getting added and deleted from the activeTimersMap object were slowing down the rest of the timers code, so this change falls back to using the ref counts to count the active timers inside process.getActiveResourcesInfo(). Fixes: nodejs#41219 Signed-off-by: Darshan Sen <[email protected]> PR-URL: nodejs#41231 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
danielleadams
pushed a commit
that referenced
this issue
Feb 1, 2022
The additional objects that were getting added and deleted from the activeTimersMap object were slowing down the rest of the timers code, so this change falls back to using the ref counts to count the active timers inside process.getActiveResourcesInfo(). Fixes: #41219 Signed-off-by: Darshan Sen <[email protected]> PR-URL: #41231 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
performance
Issues and PRs related to the performance of Node.js.
process
Issues and PRs related to the process subsystem.
regression
Issues related to regressions.
timers
Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout.
0d9f3bd introduced a noticeable performance regression with various timers benchmarks. For example:
timers/immediate.js type="clear"
performance dropped 77%timers/timers-cancel-pooled.js
performance dropped 55%timers/timers-timeout-pooled.js
performance dropped 41%and many other timers benchmarks/benchmark configs saw double digit drops.
While having a public "get active requests"/"get active handles" API would be nice, I don't think it is worth this kind of penalty if we can't find some way to remedy the performance regressions.
/cc @RaisinTen
The text was updated successfully, but these errors were encountered: