async_hooks: fast-path for PromiseHooks in JS

[Qard]

This is an attempt to eliminate the GC-tracking of promises needed to emit destroy events to async_hooks if there is nothing listening to those events. I've replicated the PromiseHook handler and PromiseWrap behaviour in JS to allow a fast-path for handling promise-related events when there is no destroy hook present and therefore no need for expensive gc-tracking. This should significantly improve the performance of async_hooks in promise-heavy applications for the typical/suggested case of context management with AsyncLocalStorage.

With this comes a few important changes to consider:

• Promises will no longer emit trace events unless there is a destroy hook
• The resource for a promise event will no longer have the isChainedPromise property (Though there are ideas of how this could be restored, if there are objections)

[jasnell]

/cc @mcollina @addaleax

[puzpuzpuz]

I like the latest commit. I also had an idea that we could use Promise objects as resources without allocating extra wrap objects. I guess, with the current state of this PR it should be ready for initial benchmarking. WDYT?

[puzpuzpuz]

Started a micro-benchmarks build: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/561/

[puzpuzpuz]

Got result for async_hooks/promises.js benchmark (https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/561/):

19:10:28                                                          confidence improvement accuracy (*)   (**)  (***)
19:10:28  async_hooks/promises.js asyncHooks='disabled' n=1000000                -0.44 %       ±3.42% ±4.54% ±5.91%
19:10:28  async_hooks/promises.js asyncHooks='enabled' n=1000000         ***     36.47 %       ±2.94% ±3.93% ±5.15%

Update. I've added a variation of async_hooks/promises.js benchmark with destory hook enabled locally just to compare the old (native) implementation of PromiseHook with the JS one proposed in this PR. Here is the result obtained on my machine (--runs 30):

                                                                       confidence improvement accuracy (*)   (**)  (***)
 async_hooks/promises.js asyncHooks='enabledWithDestroyHook' n=1000000        ***    -13.32 %       ±0.96% ±1.29% ±1.69%

I also got results for async_hooks/async-resource-vs-destroy.js on my machine with --runs 30 setting and autocannon as the benchmarker:

$ cat out-http.csv | Rscript benchmark/compare.R
                                                                                                                                                                    confidence improvement accuracy (*) (**)  (***)
 async_hooks/async-resource-vs-destroy.js n=1000000 duration=5 connections=500 path='/' asyncMethod='async' type='async-local-storage' benchmarker='autocannon'            ***      6.67 %       ±1.77% ±2.40% ±3.20%
 async_hooks/async-resource-vs-destroy.js n=1000000 duration=5 connections=500 path='/' asyncMethod='async' type='async-resource' benchmarker='autocannon'                 ***      6.96 %       ±3.61% ±4.87% ±6.48%
 async_hooks/async-resource-vs-destroy.js n=1000000 duration=5 connections=500 path='/' asyncMethod='async' type='destroy' benchmarker='autocannon'                          *     -3.59 %       ±3.08% ±4.16% ±5.55%
 async_hooks/async-resource-vs-destroy.js n=1000000 duration=5 connections=500 path='/' asyncMethod='callbacks' type='async-local-storage' benchmarker='autocannon'                -0.64 %       ±1.68% ±2.27% ±3.04%
 async_hooks/async-resource-vs-destroy.js n=1000000 duration=5 connections=500 path='/' asyncMethod='callbacks' type='async-resource' benchmarker='autocannon'                     -1.69 %       ±3.59% ±4.85% ±6.45%
 async_hooks/async-resource-vs-destroy.js n=1000000 duration=5 connections=500 path='/' asyncMethod='callbacks' type='destroy' benchmarker='autocannon'                            -0.16 %       ±2.97% ±4.04% ±5.44%

I also tried running it with wrk on my local machine and on CI, but had to cancel those runs as it took 14+h. It seems it gets stuck eventually when wrk is used as the benchmarker.

Update. The above benchmarks were run for the previous version of this PoC. They don't highlight current performance.

[Qard]

@jasnell In regard to the missing trace events we discussed in nodejs/diagnostics#376, do you think it would be reasonable to apply two different wraps depending on if there is a destroy hook? If a destroy hook is present it would apply the full wrap with gc-tracking, and if it's absent it would only apply the simplified wrap. I'm thinking of exposing the wrap class to JS and switching the logic based on the destroy hook presence. Another option would be to have two separate PromiseHook handler functions and switch between them based on destroy handler presence. 🤔

My hope was to be able to separate PromiseHook somewhat, as I'd ultimately like to make it a separate API eventually. Might be difficult though.

[mcollina]

In regard to the missing trace events we discussed in nodejs/diagnostics#376, do you think it would be reasonable to apply two different wraps depending on if there is a destroy hook? If a destroy hook is present it would apply the full wrap with gc-tracking, and if it's absent it would only apply the simplified wrap.

I'm not @jasnell but I can answer this as well. Go ahead with two different wraps depending if there is a destroy hook. It's unlikely we will be ever able to remove the destroy hook, but if we can provide a fast path it would be very handy. I would make sure we emit some debug info when one or the other is turned on (NODE_DEBUG).

[Qard]

Sounds like a plan. Thanks for the input! 😄

[addaleax]

Landed in 13c5a16