-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
napi: reduce gc finalization stress #27085
Conversation
s/finaliation/finalization/ in commit message |
8961ff2
to
1bc8222
Compare
@mscdex thanks for catching that, fixed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in commit log: s/gcc/gc/
ad8201b
to
ad3012c
Compare
nodejs#24494 fixed a crash but resulted in increased stress on gc finalization. A leak was reported in nodejs#26667 which we are still investigating. As part of this investigation I realized we can optimize to reduce amount of deferred finalization. Regardless of the root cause of the leak this should be a good optimization. It also resolves the leak for the case being reported in nodejs#26667. The OP in 26667 has confirmed that he can still recreate the original problem that 24494 fixed and that the fix still works with this optimization
ad3012c
to
881efee
Compare
Landed in ff89670. |
#24494 fixed a crash but resulted in increased stress on gc finalization. A leak was reported in #26667 which we are still investigating. As part of this investigation I realized we can optimize to reduce amount of deferred finalization. Regardless of the root cause of the leak this should be a good optimization. It also resolves the leak for the case being reported in #26667. The OP in 26667 has confirmed that he can still recreate the original problem that 24494 fixed and that the fix still works with this optimization PR-URL: #27085 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
@danbev thanks for landing :) |
@danbev could this please be backported to node10? |
Sorry I don't have time to open a backport PR against 10 this week, but I'll try to do so next week unless someone beats me to it. |
#24494 fixed a crash
but resulted in increased stress on gc finalization. A leak
was reported in #26667 which
we are still investigating. As part of this investigation, I
realized we can optimize to reduce the amount of deferred finalization.
Regardless of the root cause of the leak this should be a
good optimization. It also resolves the leak for the case being
reported in #26667. The OP in #26667 has confirmed that he can
still recreate the original problem that #24494 fixed and that
the fix still works with this optimization
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes