| 04:40 | <Steve Hicks> | In terms of events that we wanted to set to dispatch-context from the start, there was mention of same-window postMessage (I assume it's nonsense to talk about preserving context when messaging a different window?). Would MessageChannel also be covered here? (the reason I ask is that I'm working on a userland aroundEach for Jasmine - it mostly works, but only if I polyfill context propagation for MessagePort.prototype.onmessage) |
| 11:58 | <Chengzhong Wu> | agree, these tasks are pretty similar and I think we should apply the same policy on them. Contexts should not be preserved across agents. But if the async context variable is explicitly passed to a different window, I don't see it to be a problem to preserve the context. |
| 11:58 | <nicolo-ribaudo> | In terms of events that we wanted to set to dispatch-context from the start, there was mention of same-window |
| 11:59 | <nicolo-ribaudo> | agree, these tasks are pretty similar and I think we should apply the same policy on them. Contexts should not be preserved across agents. But if the async context variable is explicitly passed to a different window, I don't see it to be a problem to preserve the context. |
| 12:00 | <Andreu Botella> | Part of the reason why we focused on same-window postMessage is because that is sometimes used as a scheduler |
| 12:01 | <Andreu Botella> | but yeah, it makes sense to extend that to other windows in the same agent, or to MessageChannel |
| 16:46 | <snek> | i've been playing around with async context and disposables, and i realized they don't really work well together because we don't restore the context around suspend points (except in async generators?). curious if anyone else has thought about this at all. |
| 16:48 | <Chengzhong Wu> | async context variable disposable needs https://tc39.es/proposal-async-context/#sec-generatorresume to play well with generator/yield |
| 16:54 | <snek> | more specifically i was hoping these two examples would behave the same, but the second one doesn't work because you'd have to restore the context "inside" await, after it sets the adds the promise reaction and before it returns to the caller. https://gist.github.com/devsnek/2eee5001144f7e39513e3694ca6b3e8d |
| 18:40 | <Steve Hicks> | I'm not sure I quite understand your example. You've written
or are you thinking of something else entirely? |
| 18:49 | <Steve Hicks> | That said, we're currently thinking about the impact of whether
|
| 18:51 | <Steve Hicks> | and how exactly one sets the boundaries on where mutations do affect |
| 19:01 | <nicolo-ribaudo> | snek I'm also not fully understanding your question, but it seems similar to a discussion we had in the past:
Is answering what the second |
| 19:03 | <snek> |
|
| 19:04 | <snek> | except this doesn't work with async functions because they don't restore the scope on awaits, you're forced to wrap the function. |
| 19:13 | <nicolo-ribaudo> | Async functions capture the scope right before pausing and restore them when resuming:
|
| 19:14 | <snek> | that's not exactly what i'm talking about |
| 19:16 | <snek> | its about code that effectively uses enterWith |
| 19:18 | <snek> | i'm referring to specifically when the async function is suspended, it does not restore the context. additionally, as you noted, the promise reactions capture and restore their context. i'm basically suggesting that async function body evaluation should do the same. |
| 19:18 | <nicolo-ribaudo> | Oh you mean that before pausing they restore the context that was active before that the code in the async function run? |
| 19:19 | <snek> | yes |
| 19:19 | <snek> | this is only relevant if you can "mutate" async variables though, which the proposal currently does not allow |
| 19:19 | <nicolo-ribaudo> | Ok -- just within the current AsyncContext itself this seems to be only editorial right? It's not observable, and it only becomes observable if somebody builds a new API that behaves like async context and uses the same propagation mechanism |
| 19:20 | <snek> | idk if i'd call it editorial but yes the functionality would be something the host could build on top of, not something the js api currently exposes |
| 19:22 | <Justin Ridgewell> | There are various changes to the spec we’ll need to make if we ever add using support. Right now they’re not necessary to make. |
| 20:54 | <Steve Hicks> | But if we're contemplating either (1) making yield bring in the dispatch-context, or (2) exposing enterWith, then it's possible that we're already painting ourselves into a corner where using is a non-starter. |
| 20:54 | <Steve Hicks> | To be clear, this I'm not convinced that this is a problem, but I do think it's worth at least some thought |
| 20:56 | <Steve Hicks> | In particular, if you have enterWith then presumably you can implement using semi-reasonably in userland. |
| 20:56 | <snek> | using can be done in terms of enterWith, but you will get behavior that is probably not what you want unless the context is restored properly at suspend points |
| 20:56 | <nicolo-ribaudo> | I'd really prefer that, if we do one of those two in the language, we do `using`. It's much harder to make mistakes with it |
| 20:57 | <snek> | i am generally a fan of what using can enable with this, for example using span = tracer.createSpan(). all i'd ask for at this point is that we don't prevent that from happening in the future. |
| 21:00 | <Steve Hicks> | IIRC, The concern with using today was that it wasn't sufficiently hermetic. |
| 21:00 | <Steve Hicks> | i.e. what happens if you write tracer.createSpan()[Symbol.enter]() and then never dispose it? |
| 21:01 | <Steve Hicks> | Of course, that's less concerning if enterWith is already a possibility |
| 21:01 | <Steve Hicks> | (effectively the above is enterWith IIUC) |
| 21:02 | <snek> | yes i would say this is equiv to providing enterWith |
| 21:02 | <snek> | though if we start to seal up some of the things like context leaking out of suspends, the "never dispose" problem becomes less and less of a danger to callers |
| 21:02 | <nicolo-ribaudo> | I did have concerns with `using` because the proposal makes it trivial to just forget `using`. However, `using` with some check that you actually get Symbol.dispose would be much better than enterWith, since it makes it hard to *accidentally* forget to close the context |
| 21:03 | <Steve Hicks> | but it still leaves the question of what happens if you write
? |
| 21:04 | <Steve Hicks> | though if we start to seal up some of the things like context leaking out of suspends, the "never dispose" problem becomes less and less of a danger to callers enterWith could (depending on spec) cause a problem |
| 21:05 | <snek> | yes i mean if this functionality exists |
| 21:05 | <ljharb> | as a separate topic, i don't really grok the details of the proposal personally, but it seems like a number of node core collaborators are unhappy with the design of the proposal (in ways i also don't understand). i don't think it would be appropriate to advance an implementation that isn't also useful in node, so before seeking advancement, can yall please ensure their concerns are explained and hopefully addressed? |
| 21:06 | <nicolo-ribaudo> | As far as I understand, the "problem with Node.js" is that they have two APIs for setting the context, and this proposal is only defining one leaving the second one as a follow-up proposal |
| 21:06 | <Steve Hicks> | I did have concerns with `using` because the proposal makes it trivial to just forget `using`. However, `using` with some check that you actually get Symbol.dispose would be much better than enterWith, since it makes it hard to *accidentally* forget to close the context using so that it was syntactically guaranteed to be correct - but Ron Buckton had an issue with composability. snek's example of tracer.createSpan() exactly demonstrates the composability problem, since createSpan would need to call [Symbol.enter] and [Symbol.dispose] directly rather than syntactically. |
| 21:07 | <nicolo-ribaudo> |
|
| 21:08 | <nicolo-ribaudo> | I haven't thought about it enough to say how |
| 21:13 | <Steve Hicks> | Independently, I think snek's original question was how do the following two snippets work?
and
|
| 21:14 | <snek> | well i know how they work given the current spec text |
| 21:15 | <Steve Hicks> | they don't work at all under the current spec text |
| 21:15 | <Steve Hicks> | because enter isn't a thing |
| 21:15 | <snek> | yes i mean assuming that such a thing existed |
| 21:16 | <Steve Hicks> | gotcha - I think introducing enter would absolutely require substantial changes to account for it |
| 21:17 | <Steve Hicks> | and there's a few different options for what those changes look like |
| 21:19 | <snek> | i think its pretty simple in terms of the spec, store context on the asyncfn/generator object when started and swap back to it whenever a suspend happens. |
| 21:20 | <snek> | but we don't need to deal with it right now, i was just more curious about previous thoughts in this space and ensuring that we don't prevent this from happening in the future |
| 21:22 | <Steve Hicks> | I don't quite understand what you mean by "whenever a suspend happens" - I would have expected to swap back to it when it resumes? But the conversation yesterday was motivated by iterator helpers - if we decide that they should behave like synchronous events then it would be more consistent for generators to not swap back to the initiating context on resume - for one thing, that would make it impossible to write Iterator.prototype.map as a generator, which seems surprising. |
| 21:23 | <Steve Hicks> | There was a side benefit that removing the context swap on yield makes the spec simpler and is also probably easier and more performant for implementations |
| 21:24 | <snek> | when resuming, the context is already handled by promise reaction jobs. |
| 21:24 | <Steve Hicks> | not for generators |
| 21:24 | <nicolo-ribaudo> | It would probably be clearer @devsnek:matrix.org if you remove the top-level await from your example |
| 21:24 | <nicolo-ribaudo> | That's what confused me |
| 21:25 | <nicolo-ribaudo> | Just delete the await keyword |
| 21:25 | <Steve Hicks> | but then I'm also that much more confused because you're concerned with what the context will be outside the asyncfn body? |
| 21:25 | <snek> | well its interesting both with and without the await keyword, i guess. but those are different things. |
| 21:27 | <Steve Hicks> | (my understanding is that it gets restored to whatever it was immediately before the body was entered - which is either the same thing if it's the first entry, or else it's the empty/top context from the microtask queue) |
| 21:27 | <snek> | the current semantics are this https://gc.gy/f5b45fb1-6bd7-4357-ba4f-95ee1e20c464.png |
| 21:27 | <Steve Hicks> | but maybe that's what you're getting at - the first entry wouldn't restore anything? |
| 21:28 | <snek> | get/setAsyncContext being exactly equiv to agent.[[AsyncContext]]=whatever |
| 21:29 | <Steve Hicks> | and you're saying you want it to be undefined? |
| 21:30 | <snek> | i think that's what i would expect it to be, yes |
| 21:31 | <snek> | (updated above to a slightly more rigorous example) |
| 21:32 | <Justin Ridgewell> | Updated what? |
| 21:32 | <Steve Hicks> | I agree, I think that makes the most sense, but I wonder if that would require an unacceptable amount of function call overhead? |
| 21:32 | <snek> | i can't speak for all engines but in v8 at least its a single cpu instruction |
| 21:32 | <snek> | Updated what? |
| 21:33 | <Justin Ridgewell> | There is a change to await that needs to happen to prevent the using scope from leaking out in:f() synchronous execution ends at the await, and we need to cleanup any using mutations before resuming past the f() call. |
| 21:33 | <Justin Ridgewell> | sorry, i meant the above screenshot |
| 21:34 | <snek> | ~13 messages up. i also posted a more abstract gist much further above |
| 21:35 | <Chengzhong Wu> | For disposable in an async function, only mutations before the first await need to be restored when exiting the function frame. Not all await tho. |
| 21:36 | <snek> | oh yeah i was thinking about that before. all subsequent mutations happen within microtasks so they're already restored right? |
| 21:36 | <Chengzhong Wu> | they will be overridden in the next task anyway |
| 21:37 | <Justin Ridgewell> | Wow, Cinny is missing a huge part of the conversation |
| 21:37 | <snek> | i was also playing with this, not that i intend to merge it but if anyone is curious https://chromium-review.googlesource.com/c/v8/v8/+/6020635 |
| 21:37 | <snek> | sent an image. |
| 21:38 | <shu> | haha love2matrix |
| 21:40 | <snek> | at the very least, it seems we're all on the same page that changes would be needed for this. and that there are some concerns around those changes. |
| 21:42 | <Chengzhong Wu> | yes, more changes additional to the current spec are needed |
| 21:42 | <Chengzhong Wu> | it should be a follow-up proposal |
| 22:51 | <Steve Hicks> | My concern with a follow-up proposal is that it would send polyfills back to square 1 - i.e. you couldn't leverage the working earlier-spec'd AsyncContext and get the new behavior. And worse, the polyfill would be impossible without instrumenting every function in the entire program, rather than just async/generators. I know we don't want to block on polyfill feasibility, but that's potentially a pretty big problem for me at least. |
| 22:51 | <Steve Hicks> | (As it is, I'm already at a bit of a loss w.r.t. how I'm going to polyfill the context-propagation in MessageChannel...) |