Skip to content

async_hooks: use new v8::Context PromiseHook API

Rodrigo Muino Tomonari requested to merge github/fork/Qard/jsfunction-promise-hook into master

I've been working on an alternative to the existing PromiseHook API in V8 using split JSFunctions per event type rather than the old way which routes everything through a single C++ function even though it's just going right back to JS in the end. The old way has many, many costly boundary jumps and deopts in many cases, so we really need a better way to track promise lifecycle events. 😅

The corresponding Chromium Review is here: https://chromium-review.googlesource.com/c/v8/v8/+/2466783 https://chromium-review.googlesource.com/c/v8/v8/+/2759188

As the change has not yet landed in V8, I'm opening this as a draft PR just to show my progress and give people something to look at. I've got some nice benchmarks to share too!

Before:

❯ ./node-master benchmark/async_hooks/promises.js
async_hooks/promises.js asyncHooks="enabled" n=1000000: 271,514.4761845998
async_hooks/promises.js asyncHooks="enabledWithDestroy" n=1000000: 187,109.67499761964
async_hooks/promises.js asyncHooks="enabledWithInitOnly" n=1000000: 315,364.83307747904
async_hooks/promises.js asyncHooks="disabled" n=1000000: 1,668,525.7547478643

After:

❯ ./node benchmark/async_hooks/promises.js       
async_hooks/promises.js asyncHooks="enabled" n=1000000: 1,250,448.989340235
async_hooks/promises.js asyncHooks="enabledWithDestroy" n=1000000: 194,382.33020796857
async_hooks/promises.js asyncHooks="enabledWithInitOnly" n=1000000: 1,071,082.1953050198
async_hooks/promises.js asyncHooks="disabled" n=1000000: 1,673,424.8290965108

It's getting about 3-4x better performance. Not quite on-par with disabled yet, but getting close. There is still much that can be done to improve those numbers though.

The current changes do not allow the optimizer to do its magic in certain cases so I'm hoping to dig a bit more into getting those cases to optimize too. For reference though, the existing PromiseHook API bails out too, so this is quite a bit faster already. I'm also investigating moving the PromiseWrap used when there's a destroy hook to using this model and wrapping from JS instead, which I think should still be an overall performance improvement despite the hop back to native for the initial wrap.

cc @nodejs/diagnostics @nodejs/v8

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

Merge request reports