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 For instance,
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...
|
21:50 | <James M Snell> | such that in an object I could do...
|
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
That is simply defined as being equivalent to |
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 |