00:00
<James M Snell>

As Justin suggests, we can easily replace class Foo extends AsyncContext { bar() { this.runInAsyncScope(() => {}) }} with...

class Foo {
  #runInAsyncScope = AsyncContext.wrap((cb, ...args) => cb(...args));
  bar() { this.#runInAsyncScope(() => {});
}

To achieve the use case

00:00
<James M Snell>
The idea with AsyncContext.snapshot() is to just eliminate the extra boilerplate of that wrap tho
00:15
<James M Snell>
While AsyncResource has other use cases relating to async_hooks, the only one that is relevant here is capturing the context on object creation and being able to call into that multiple times
11:01
<Chengzhong Wu>
https://github.com/legendecas/proposal-async-context/issues/19
15:15
<littledan>
Sure, so since this is not expressiveness but rather ergonomics, it would help if someone could point at actual use cases so I could understand why this is worth it
16:05
<James M Snell>
Take a look at the piscina library. That was the origin of Node.js' EventEmitterAsyncResource. It shows a practical use case. Another useful case, look at HTMLRewriter change here https://github.com/cloudflare/workerd/pull/282/commits/0ffd4efd1914b428639499517e0177bd843a6583
16:07
<James M Snell>
That's not using AsyncResource but the effect is the same. We capture the context frame once and enter it each time the registered callbacks are called
16:08
<James M Snell>
Or, we can use AsyncResource to run in the context where the handlers are registered.
16:08
<James M Snell>
It gives a good amount of flexibility
18:02
<Justin Ridgewell>
18:04
<Justin Ridgewell>
I think in this particular case, using this.callback = AsynContext.wrap(callback) would have been appropriate
18:04
<Justin Ridgewell>
But snapshot() would be helpful for capturing the context used for multiple callbacks, instead of wrapping each individually
18:05
<James M Snell>
Yep absolutely. We had no other choices at the time.
18:06
<James M Snell>
That's why I also point at the HTMLRewriter example. It has the multiple callbacks so provides a good contrast
18:15
<James M Snell>
Wrapping each individual callback is also very expensive in node.js' current model since each is a separate AsyncResource that copies the context
18:16
<James M Snell>
That won't the case in the revised model, of course, but currently it's pretty expensive to wrap each individual callback so using AsyncResource is less costly but isn't really quite right in most cases
18:19
<Chengzhong Wu>
I've been poking the current implementation to adopt the revised model, based on the current async_hooks.
18:20
<Chengzhong Wu>
The performance improvements can be significant
18:22
<Chengzhong Wu>
it is still worthwhile to update the current implementation, since we won't get the new v8 apis on older LTS lines
18:24
<James M Snell>
Yeah I think once we get a few tweaks in place we should be able to transition fairly easily while keeping the old releases on the asynchooks model
18:27
<Chengzhong Wu>
yeah, maybe I can submit my micro-benchmarks first -- I still need more time to cleanup my improvements.
18:34
<Chengzhong Wu>
but, right, with the revised model, wrap and snapshot should have no performance differences.