async_hooks: move to lazy destroy hook registration in AsyncResource
Adds a check into AsyncResource
constructor, so that registerDestroyHook
only gets called when requireManualDestroy
is false
and there are no active destroy
hooks (previously the check only included requireManualDestroy
value). As below benchmark shows this eliminates a certain overhead related with additional tracking of these objects as a part of GC cycle.
This scenario is important considering AsyncLocalStorage
API which only uses init
hooks.
Important note. Obviously, this PR changes the behavior in the following edge case: previously when an AsyncResource
was created (but not yet destroyed) before enabling the first destroy
hook, the hook would be called for the resource. With this change it's not called.
Related discussion thread: https://github.com/petkaantonov/bluebird/pull/1472#issuecomment-352104689
cc @addaleax @vdeturckheim @Qard @Flarna (sorry for such long CC list, but I think all of you might be interested)
Benchmarks
Before this change:
$ ./node benchmark/async_hooks/gc-tracking.js
async_hooks/gc-tracking.js method="trackingEnabled" n=1000000: 3,338,788.791796874
async_hooks/gc-tracking.js method="trackingDisabled" n=1000000: 79,598,226.36048095
After this change:
$ ./node benchmark/async_hooks/gc-tracking.js
async_hooks/gc-tracking.js method="trackingEnabled" n=1000000: 73,493,075.92334495
async_hooks/gc-tracking.js method="trackingEnabledWithDestroyHook" n=1000000: 2,497,582.277915421
async_hooks/gc-tracking.js method="trackingDisabled" n=1000000: 79,170,025.7848857
Note: trackingEnabledWithDestroyHook
scenario was added as a replacement for the old trackingEnabled
scenario.
Checklist
-
make -j4 test
(UNIX), orvcbuild test
(Windows) passes -
tests and/or benchmarks are included -
documentation is changed or added -
commit message follows commit guidelines