18:33
<Justin Ridgewell>
Pinged him at https://github.com/legendecas/proposal-async-context/pull/17#issuecomment-1406907267, maybe he doesn't keep Matrix open
18:33
<Justin Ridgewell>
Anyone know how to read Chrome's Pinpoint performance metrics? https://chromium-review.googlesource.com/c/v8/v8/+/4173598
18:44
<James M Snell>

We could make this configurable as an option passed in to addEventListener in order to prevent backwards compatibility issues.

For instance,

eventTarget.addEventListener('foo', () => {}, {
  captureAsyncContext: true,
});

which would essentially be syntactic sugar for manually calling wrap on the handler function

18:45
<James M Snell>
Alternatively, I do wonder if there's a syntax solution here? Similar to what is done with generator functions, or async functions, some way where we can declare that a function should always capture the async context when it is called
18:46
<James M Snell>
I fully understand that syntax additions are less than desirable a lot of the time
18:46
<Justin Ridgewell>
Not a syntax, but we could add a property to the function instance to do it
18:47
<James M Snell>
yeah that's better
18:47
<Justin Ridgewell>
It would be a WebIDL thing that it can check
18:47
<Justin Ridgewell>
I think either behavior is fine, though, so I'm not sure if we need to push it
18:48
<James M Snell>
so the impl of addEventListener would look for that property and if set, capture the async context and restore it when that function is called
18:48
<Justin Ridgewell>
Yoav may have some thoughts on it, though
18:48
<Chengzhong Wu>
Should we create a tracking issue for host integration?
18:49
<Justin Ridgewell>
Yes
18:50
<Chengzhong Wu>
I really need to go to bed now so if any of you would like to do it, it would be great. Or I'll create one when I get up tomorrow morning.
21:46
<James M Snell>
Looking at that I don't see any significant difference in the performance metrics between the two. Is this the patch to handle EnqueueMicrotask?
21:48
<James M Snell>
What's the current plan for dealing with the extends AsyncResource case that we have in Node.js? For instance class Foo extends AsyncResource { doIt() { this.runInAsyncScope(() => {}) } } ... it admittedly is not a great pattern and is difficult to optimize around.
21:48
<Justin Ridgewell>
No, it removes promise handling entirely
21:48
<Justin Ridgewell>
They're trying to see if the continuation stuff causes any perf impact, and it doesn't look like it
21:49
<Justin Ridgewell>
(But I don't really know how to read it, so maybe I missed something)
21:49
<James M Snell>

What we almost need is a way of capturing the reference to the async context frame ... something like a...

class AsyncContextFrame {
  static readonly attribute AsyncContextFrame current;
  run(fn, ...args);
}
21:50
<James M Snell>

such that in an object I could do...

class Foo {
  #frame = AsyncContextFrame.current;
  doSomething() {
    return this.#frame.run(() => {});
  }
}
21:51
<James M Snell>
ah, ok. Yeah from what I can see there it's no perceptible difference in performance
21:51
<Justin Ridgewell>
The initial design for AsyncContext had a mutable static property, and it was shot down
21:52
<Justin Ridgewell>
Is there a reason they can do #run = AsyncContext.wrap((cb) => cb())?
21:52
<Justin Ridgewell>
(This is the higher-order pattern that I kinda dislike)
21:53
<James M Snell>
other than it's really ugly? lol
21:53
<James M Snell>
not a fan of the boilerplate but it does work
21:54
<Justin Ridgewell>
My original design had a Snapshot class that acts like AsyncResource
21:56
<James M Snell>

I would expect the pattern to be common enough to justify making a utility... perhaps something simple like

#run = AsyncContext.snapshot(); 

That is simply defined as being equivalent to AsyncContext.wrap((cb, ...args) => cb(..args))

22:04
<Justin Ridgewell>
That seems fine
22:17
<James M Snell>
I guess alternatively we could have the variation AsyncContext.wrap() (with no arguments passed) be the equivalent to AsyncContext.snapshot() but that seems a bit weird
22:17
<James M Snell>
it would save on API surface tho
23:46
<littledan>
When I suggested switching to wrap, it’s because I thought it looked simpler. I would like to understand more about the use cases for AsyncResource to explain its surface area. Could someone compile a few usage examples?
23:48
<littledan>
(I like to think nothing has been shot down yet, we are just iterating through alternatives and can still come back to things. Anyway, I apologize for being too pushy about some of these details.)
23:50
<littledan>
I think adding punned overloads still counts as surface area. But if something is important, we can go back and add it.
23:57
<James M Snell>
So AsyncResource originally was just an async_hooks thing and didn't have anything to do with AsyncLocalStorage. However, as an artifact of the way Node.js implemented ALS, every AsyncResource captures the async context when it is created and allows functions to enter the context using runInAsyncScope(...). Simply put: it's a way of capturing the async context on object creation.
23:58
<James M Snell>
For resource tracking purposes (async_hooks), AsyncResource is still a thing that will be needed there, but for async context tracking, it's not really needed and it's a bit wasteful