2023-03-08 [10:20:35.0419] We have spec text! https://tc39.es/proposal-async-context/ [10:21:55.0814] I think the only surprising change is the addition of `[[Construct]]` on wrapped callbacks: ``` const Foo = AsyncContext.wrap(class Foo {}); // The wrapper will invoke construct on the wrapped function new Foo(); ``` [10:22:15.0862] This comes from the behavior of `Function.p.bind` [12:25:31.0724] yeah I am OK with including or omitting the [[Construct]] behavior [12:42:18.0394] the spec would probably be clearer if we could use whatwg infra algorithms [12:42:28.0516] (for the mapping) [12:43:39.0074] we should probably think about exposing reusable algorithms (for web specs) for get and run, but this is an editorial thing [12:44:08.0692] it might be nicer editorially if we avoided SameValueZero; SameValue or simply = is enough. (But the nicest would be to say that it's a mapping...) [12:44:31.0584] anyway I don't see any bugs in the spec; looks good for Stage 2 to me [12:48:11.0181] IIRC the `setTimeout` infrastructure in HTML jumps off-thread to sleep, and then queues a task on the event loop [12:48:31.0000] a wrapped `setTimeout` would have to "send" a snapshot off-thread [12:48:36.0678] is that fine, as long as the values are not read? [12:49:05.0180] I'm not sure if it should be phrased like that [12:49:23.0828] I hope that you can work with ms2ger and Yoav on a proposed wording here [12:50:01.0165] I think queued tasks would often carry asynccontext snapshots with them, but that snapshot shouldn't ever logically leave the main thread [12:50:35.0272] * a `setTimeout` that wraps the callback would have to "send" a snapshot off-thread [12:51:04.0549] in any case, you'll want to have clean abstract algorithms for creating an AsyncContext, and get and run, so that the embedding spec doesn't need to call the actual JS functions 2023-03-09 [11:49:40.0549] We really need to execute on improving the motivation section of the async context readme, especially for the web use cases. Here's a summary of use cases I have in mind: - Prioritizing threads of control, along the lines of https://github.com/WICG/scheduling-apis , and inheriting these priorities across callbacks and async/await (both the built-in mechanism and JS-level mechanisms) - Collecting performance information across logical asynchronous threads of control, including both timings and with OpenTelemetry. See this project which can only work with async/await if you use a custom transpiler: https://open-telemetry.github.io/opentelemetry-js/classes/_opentelemetry_context_zone_peer_dep.ZoneContextManager.html - There's some other use case with React Cache but I don't fully understand it. - We have other use cases within Bloomberg in the Terminal to track different logical "applications" which run in the same JS heap, but I'm not sure if anyone else runs into these issues. (We also want to make more use of OpenTelemetry) [11:49:51.0437] Every time I talk with a web person about this proposal, they are confused for this exact reason [11:50:22.0838] Can someone take the action to make a PR to the async context readme on this topic? If not, please let me know and I'll try to eventually do it (but might not have time before this meeting) [11:51:31.0492] * We really need to execute on improving the motivation section of the async context readme, especially for the web use cases. Here's a summary of use cases I have in mind: - Prioritizing threads of control, along the lines of https://github.com/WICG/scheduling-apis , and inheriting these priorities across callbacks and async/await (both the built-in mechanism and JS-level mechanisms) - Collecting performance information across logical asynchronous threads of control, including both timings and with OpenTelemetry. See this project which can only work with async/await if you use a custom transpiler: https://open-telemetry.github.io/opentelemetry-js/classes/\_opentelemetry\_context\_zone\_peer\_dep.ZoneContextManager.html - Yoav's various use cases [as a consistent mechanism which works within browsers and fundamentally needs the same mechanics] - There's some other use case with React Cache but I don't fully understand it. - We have other use cases within Bloomberg in the Terminal to track different logical "applications" which run in the same JS heap, but I'm not sure if anyone else runs into these issues. (We also want to make more use of OpenTelemetry) [11:51:45.0779] also: Are we putting AsyncContext on the agenda for this TC39 meeting? We need to decide by tomorrow and add it if so. [11:52:55.0989] Chengzhong mentioned in the WinterCG meeting that the idea was to propose it for stage 2 for this TC39 plenary [11:53:43.0367] Good, let's put it on the agenda then [11:54:05.0972] it will need to have this web motivation better documented for this plenary [11:54:49.0766] This will also be important to have written down significantly before the March 16th WebPerf WG call which will discuss AsyncContext [11:56:51.0465] is there some link for the react use case? [11:57:19.0461] (I only know of the description in Justin's slides... IMO the others are sufficient to start with) [11:57:55.0101] ok, I'll make a PR for that [11:58:16.0750] Thanks! [13:31:16.0431] What’s WebPerf WG? [14:05:26.0961] https://www.w3.org/webperf/ [14:05:46.0221] I think a lot of the strongest use cases for AsyncContext in the web are around performance metrics, so we're going to discuss this there. [15:08:00.0698] If we (non-champions) have additional use cases to suggest, should we open issues on the proposal repo? Or sketch them here? [15:19:38.0577] Either an issue, here, or an explainer PR would all be good and helpful! [15:19:47.0850] Whatever works for you 2023-03-10 [18:59:21.0489] > <@littledan:matrix.org> also: Are we putting AsyncContext on the agenda for this TC39 meeting? We need to decide by tomorrow and add it if so. Thanks for the reminder! Agenda item submitted: https://github.com/tc39/agendas/pull/1334 [19:01:20.0946] > <@benjamn:matrix.org> If we (non-champions) have additional use cases to suggest, should we open issues on the proposal repo? Or sketch them here? whatever works for you is welcomed! [21:53:02.0070] Working on slides now [23:26:27.0911] https://docs.google.com/presentation/d/1LLcZxYyuQ1DhBH1htvEFp95PkeYM5nLSrlQoOmWpYEI/edit#slide=id.p [06:31:43.0567] Great, thanks for writing slides, Justin, these look really good! Some notes: - IMPORTANT: right before Slide 2, insert a slide which quickly lists some use cases (including at least 2 client-side use cases, plus a third thing which is internal infrastructure within web standards [Yoav's thing]); hopefully this can also link to Andreu's more detailed description at the top of the README [The rest of these comments are nits, but the motivation is key.] - In Slide 4, note explicitly that this is what Promise .then, setTimeout, etc would do. [Or: Consider consolidating slides 3 and 4 into something which just shows the interface, saving the pseudocode implementation for a bonus slide. I am undecided on whether we should do this] - Write somewhere on the slides that we consider all of the questions raised to be things that we aim to resolve between Stage 2 and Stage 3. Emphasize this especially for the web platform integration and unhandled rejection context--these must be spelled out, and we plan to work with Yoav Weiss on this, who was already thinking in this direction for the previously mentioned internal use case. - The constructor parameters idea is really superficial; should it really be the first slide? Maybe we should start with the important issues. [Also IMO the parameter order should be reversed but that's a comment I should make in the issue] - The web platform doesn't have a lot of cases to consider because it's weird--this is just an inherent difficulty with AsyncContext wherever it's done. I think "case by case" might make it sound too bad--we'll have some organizing principles, we just don't have a proposal yet. The proposal here will be a precondition for Stage 3. (BTW: Avoid saying "a Stage $n question" because people use this term both to mean, a precondition to enter $n or something to discuss during $n) - Focus on explaining next steps towards resolution when presenting each of these slides. [More notes to come, have to go] [08:24:06.0825] speaking of constructor parameters, I really like the `defaultValue` idea, though I suppose you could subclass `AsyncContext` to add functionality like that if you wanted it [08:24:56.0847] could you? since you'd need to wrap all uses of the `AsyncContext` in a `run`, I think [08:25:25.0719] oh, by overriding `get` I guess [08:28:01.0561] I've also been asked about the possibility of setting up a construction-time default value but really don't think it's a good idea. I imagine that it would have the same basic effect as `asyncLocalStorage.enterWith(...)`, modifying the state of the current async context unless it is defined as "use this value as the default if the current storage for this is undefined" [08:29:40.0661] if that's the semantic, then I can get on board with it, but if setting the value on the constructor actually modifies the async context, definitely not [08:35:04.0504] I'm not familiar with the `AsyncLocalStorage` API, but I don't think I understand the difference between those two options [08:35:10.0229] would it lead to any observable difference? [08:35:39.0615] oh, is this about snapshot? [08:35:56.0518] * oh, is this about wrap? [08:36:57.0377] The default value would only be returned if the mapping can’t be found in the global state [08:37:17.0588] You wouldn’t be able to modify the context instance after the fact [08:37:32.0511] So just modifying the get method’s last step [08:38:49.0939] So I guess the key question then is what happens in the `AsyncLocalStorage.exit(...)` case, which Node.js defines as exiting the context scope but we've implemented as being equivalent to `als.run(undefined, () => {})` [08:39:42.0930] Setting to an explicit undefined isn’t the same as no found mapping, so you’d get undefined [08:40:07.0987] e.g. what would you expect in the following: ``` const als = new AsyncLocalStorage({ defaultValue: 'foo' }); als.run(123, () => { als.exit(() => { console.log(als.getStore()); // undefined or foo? } }) ``` [08:41:42.0488] Undefined, but it could really be either. AsyncContext doesn’t have an exit method, so this is really just Node [08:42:24.0627] if this is only something that is a concern when implementing `AsyncLocalStorage` in terms of `AsyncContext`, I'm not sure that's enough to block this addition [08:42:40.0310] it can surely be implemented anyway with an additional layer of indirection, right? [08:43:10.0374] yeah, I think I just wanted to make sure that adding the default value does not actually modify the current storage context [08:43:23.0307] if it's local to the `AsyncContext` instance itself, then +1 [09:24:32.0511] > <@abotella:igalia.com> is there some link for the react use case? If we don't include the React or Bloomberg Terminal use cases, it seems like the only non-browser-internal use cases we have logging, timing and OpenTelemetry [09:25:55.0755] the browser-internal use cases are relevant, of course, but it'd be great to have more use cases for having `AsyncContext` as an actual JS built-in [10:00:40.0253] https://github.com/tc39/proposal-async-context/pull/30 [12:32:33.0693] here are a dozen use cases (some overlapping) I pulled from some old notes: https://gist.github.com/benjamn/f901cdc634a2d6e29542c32330208a87 [12:34:20.0254] I'd love some guidance on which of those sound more/less interesting, or need clarification/elaboration, so I can prioritize which ones I turn into issues [13:06:31.0659] fwiw, the use case I care most about is https://gist.github.com/benjamn/f901cdc634a2d6e29542c32330208a87#file-topological_dependency_tracking-md [13:17:10.0945] I will add much more explanatory text to any public issues 2023-03-13 [04:04:53.0654] Aren't the snapshot requirements on `HostMakeJobCallback` and `HostCallJobCallback` a bit too loose? [04:05:15.0254] we don't want hosts to populate `[[AsyncContextSnapshot]]` with any snapshot whatsoever [04:42:39.0205] I opened https://github.com/tc39/proposal-async-context/pull/31 to tighten those requirements [04:52:20.0744] > <@abotella:igalia.com> I opened https://github.com/tc39/proposal-async-context/pull/31 to tighten those requirements This PR LGTM but I think we should really refactor the PR to make abstract operations for run and get. Then, this PR would say, the AsyncContext must be created by run [04:52:20.0994] (This implies a tighter host limitation, otherwise hosts could do anything run does) [04:52:21.0164] I would also mention JS-implemented scheduling and, delete the last bullet point (access limitation) as I think a lot of people will be skeptical of it and it will throw off the discussion [04:52:21.0337] Sorry I didn’t review Andreu’s PR earlier. Let’s avoid the term “asynchronous callstack” as it is confusing and misleading (doesn’t make it clear that run has to be explicit). Instead, we are talking about the asynchronous flow of control. [04:58:14.0203] Also would be good if the spec had a sort of introduction that linked to the readme [04:58:35.0858] Maybe it should begin with the definition of the AsyncContext class and then go into how it works [04:58:46.0877] (Again, apologies for the delayed review ) [05:14:22.0423] Huh, it looks like `Atomics.waitAsync`'s `HostResolveInAgent` leaves *everything* to the host, rather than using `HostMakeJobCallback` and `HostCallJobCallback` [05:15:59.0429] should it be up to the host whether the callback gets wrapped there? [05:17:38.0915] Let’s leave any proposed refactorings there for later (since we will learn what html wants as this proposal evolves, and no need to duplicate the debate) [05:18:13.0158] sure [05:18:55.0733] We can just note that this state of the spec might mean that TC39 might not hold us to a very meaningful layering [05:18:56.0567] I just have been learning a bit about low-level concurrency and was trying to make sense of the atomics and memory model parts of the spec, and I noticed this slightly intersected with AsyncContext [05:19:15.0834] Hopefully the layering can be expressed more by the structure of the spec and less by requirements [05:19:33.0690] Requirements end up being really unclear [05:25:13.0732] It is great that you noticed this connection btw, even if it is not actionable right now [07:10:55.0615] the way the spec is written right now, `AsyncContext.wrap` doesn't check if the argument is a function, but it does check if it's a constructor [07:11:13.0528] in order to figure out whether to make the wrapped function a constructor or not [07:11:26.0114] do we want this behavior? [07:12:40.0654] also, `AsyncContextWrappedFunctionCreate` is defined to take a function object as its first argument, which isn't the case since `AsyncContext.wrap` doesn't check that [07:13:02.0862] I guess we should eagerly check for callability, as this is what .bind does [07:13:37.0555] btw if Function.prototype.bind doesn't work for class declarations (just old-style classes), does that mean its [[Construct]] behavior is thought of as "legacy"? In this case, should we not bother replicating it? [08:55:41.0991] > <@littledan:matrix.org> btw if Function.prototype.bind doesn't work for class declarations (just old-style classes), does that mean its [[Construct]] behavior is thought of as "legacy"? In this case, should we not bother replicating it? How does `Function.prototype.bind` not work? You can `new` the result, but you can't subclass it without fixing `.prototype` first: ```js class C {} let C1 = C.bind(null); new C1() instanceof C; // true class D extends C1 {} // TypeError: Class extends vlaue does not have a valid prototype property C1.prototype; // undefined C1.prototype = C.prototype; class D1 extends C1 {} // ok ``` [08:55:56.0128] > <@littledan:matrix.org> btw if Function.prototype.bind doesn't work for class declarations (just old-style classes), does that mean its [[Construct]] behavior is thought of as "legacy"? In this case, should we not bother replicating it? * How does `Function.prototype.bind` not work? You can `new` the result, but you can't subclass it without fixing `.prototype` first: ```js class C {} let C1 = C.bind(null); new C1() instanceof C; // true class D extends C1 {} // TypeError: Class extends value does not have a valid prototype property C1.prototype; // undefined C1.prototype = C.prototype; class D1 extends C1 {} // ok ``` [11:48:42.0905] oh well Function.prototype.bind throws on things which are not callable. I forgot about how classes are callable and they just throw. [12:01:33.0943] it's so baffling to me that the call/construct slots were basically made useless with things like that [12:08:33.0557] yes, it makes me feel like it was a mistake to have a [[Construct]] hook in the first place [12:19:33.0380] i mean it'd be super useful to have both slots actually mean what they're called, but short of that having two is silly [12:51:46.0829] I'd love for a "call constructor" to be valid in the language. It'll be possible to write a user-space `@Callable` decorator, I think, but you would have to replace the constructor with a regular `function` to do so. A native `@Callable` that does the same thing could just replace the `[[Construct]]` slot. [12:54:49.0046] What's weird to me is that `class` has a `[[Call]]` that throws, but `() => {}` doesn't have a `[[Construct]]` at all (even one that throws). 2023-03-14 [19:59:16.0276] > <@abotella:igalia.com> the way the spec is written right now, `AsyncContext.wrap` doesn't check if the argument is a function, but it does check if it's a constructor Fixing this in https://github.com/tc39/proposal-async-context/pull/32. Thanks for catching this! [04:02:46.0454] Do we want values in the async context mapping to be ECMAScript language values? [04:03:30.0390] I was thinking that maybe Yoav Weiss's task attribution could be defined in terms of storing an HTML spec-level value on the mapping for a symbol that's not associated to any `AsyncContext` instance [04:03:38.0569] * I was thinking that maybe Yoav Weiss's task attribution handling could be defined in terms of storing an HTML spec-level value on the mapping for a symbol that's not associated to any `AsyncContext` instance [04:04:23.0216] `AsyncContext.prototype.get` could then assert that the gotten value is an ECMAScript language value [04:07:01.0121] * I was thinking that maybe Yoav Weiss's work on task attribution handling could be defined in terms of storing an HTML spec-level value on the mapping for a symbol that's not associated to any `AsyncContext` instance [05:26:43.0282] yes, I think this would be a good direction for part of the refactoring patch which also exposes abstract operations for get, run, and the AsyncContext constructor. 2023-03-16 [03:23:10.0825] Don't know about you, but I thought the idea was to implement task attribution *on top of* AsyncContext, rather than the other way around [03:23:24.0752] but implementing it on top of task attribution seems to be what Yoav is proposing [03:23:38.0144] * but implementing AsyncContext on top of task attribution seems to be what Yoav is proposing [03:29:22.0276] I wonder what the folks from server-side V8-based runtimes think about that [07:08:17.0882] so the way we handle things in workers... What we actually store using the v8 API is an embedder object wrapped in a js object. We call this embedder object an "Async Context Frame" (or frame for short). That frame maintains a mapping of `StorageKey` to JS Value. Every `AsyncLocalStorage` instance (equivalent to `AsyncContext`) represents one `StorageKey`. Whenever `als.run(...)` is called, we create a new frame as a clone of the current and set the new value associated with that key. "Capturing" the current context just means holding a reference to whatever frame is current [07:09:15.0060] What's nice about this design is that we can (and do) have `StorageKey` instances that are not associated with a specific `AsyncLocalStorage` instance [07:11:13.0516] the values actually stored are also ECMAScript language values [07:11:36.0784] but those could be externals (js objects wrapping embedder objects) [07:39:54.0995] Hm, I guess if task attribution gets spec'd in the web platform, and it is used for the scheduler API, that would fall under WinterCG [07:40:26.0752] and no matter how that gets implemented in V8, V8-based WinterCG runtimes could handle it the same way as Chromium [07:42:34.0686] so it might not matter that the current async context snapshot is not managed by V8 itself [07:43:11.0286] Andreu Botella: What is the difference? Is there some conversation happening somewhere that I'm missing? [07:43:37.0295] > <@littledan:matrix.org> Andreu Botella: What is the difference? Is there some conversation happening somewhere that I'm missing? https://docs.google.com/document/d/16HstQ6yHWMhFHflrpnKTeH0FDGxpQ8fKWqbA1mos3Ag/edit [07:44:18.0455] was this linked from somewhere? [07:45:13.0674] oh he shared it with us [07:46:10.0104] oh, rereading that I now realized that the doc was framing AsyncContext as a web API rather than a JS built-in, so it wouldn't actually be implemented in V8 [07:46:17.0470] I guess I read it too quickly earlier [07:46:29.0877] * oh, rereading that I now realized that the doc was framing AsyncContext as a web API rather than a JS built-in, so it wouldn't actually be implemented in V8 if that is accepted [07:47:15.0983] yeah from our perspective (workers) it won't matter to us so long as we can keep using that v8 api to propagate the context [07:48:27.0170] I rather like the way the API works currently because it makes things super simple for us [07:51:59.0241] in terms of spec layering (which might not correspond at all to how V8/Chromium implement it), if AsyncContext is still spec'd in TC39 rather than in the web specs, task attribution would have to depend on it [08:03:16.0833] How did the WebPerf WG meeting to, Chengzhong Wu ? [08:43:11.0034] btw the slides say addEventLitener [08:43:15.0117] missing an s [08:43:28.0801] also I think they should reference OpenTelemetry (maybe with a link to the zone.js library) [12:53:36.0804] Both the slides and the explainer make several references to "asynchronous callstack". I would like to discourage folks from using that term, as I think it confusingly implies that we're doing something more similar to Yoav's TaskAttribution. We're not saving a callstack with the whole asynchronous history or building a derivation tree of everything; we're just preserving stuff stashed in .run() through an asynchronous flow of control. [12:57:19.0528] So I'd suggest saying, "asynchronous flow of control", or especially better if we say something more concrete, e.g., "Annotating logs with information related to a particular logically related series of operations, even with callbacks and promises" (or, maybe there's a better way to put that, idk) [12:58:30.0434] in particular: the logical "async stack" can sometimes grow in an unbounded way by accident by just chaining operations. Yoav's system handles this in a lossy way--things can fall off a circular buffer. We're specifically hoping to avoid this by making .run() explicit. [12:59:00.0722] [though I guess we do need to decide what to do on stack overflow/excessive growth, which can still happen with .run()] 2023-03-17 [19:46:06.0083] > <@littledan:matrix.org> How did the WebPerf WG meeting to, Chengzhong Wu ? as far as I can tell, the initial feedback is positive (slides of the talk: https://docs.google.com/presentation/d/1cmAUbNUsqkuO6gMjhPaEcVGHuCJFYluntw7AIPETGaI/edit?usp=sharing) [20:08:12.0421] Are there notes? [20:09:34.0492] It should be published shortly at https://w3c.github.io/web-performance/meetings/ [20:10:47.0381] Not published yet. [20:12:04.0046] The use cases there are very interesting. Maybe it would be useful to link that deck from the readme [20:12:32.0195] And possibly go into it at TC39 as well if there is skepticism raised [20:13:21.0604] (Could be pasted on as bonus slides?) [20:24:35.0578] yeah, we can bring the use case slides up when needed. [20:25:42.0760] > <@littledan:matrix.org> So I'd suggest saying, "asynchronous flow of control", or especially better if we say something more concrete, e.g., "Annotating logs with information related to a particular logically related series of operations, even with callbacks and promises" (or, maybe there's a better way to put that, idk) Agreed. "async stack" sounds like we are preserving the exact call stacks but this is not accurate. [01:26:05.0522] Rough minutes are at https://docs.google.com/document/d/1wO7mbGr6f3tDnEWAaMSVI0bMeEHIdcLbAKeOQe-Zh4U/edit#heading=h.1sx1vjpi65co [01:28:08.0050] The document is not public -- access requested [01:28:38.0462] apologies! should be now [01:34:52.0268] If I can try to summarize the feedback: RUM providers would be very interested in being able to keep track of operations that are a result of a certain AsyncContext [01:35:33.0479] e.g. "This element timing entry is a result of a user click on element X" is something they'd love to be able to surface to their customers [01:36:12.0512] It was unclear from the conversation if the current API enables them to achieve that without over-instrumentation of their customers' code (which they prefer not to do) [01:38:26.0156] But maybe magicaly surfacing the AsyncContext data in PerfObserver entries can help them do that if they wrap addEventListener callbacks [01:50:17.0667] > <@yoavweiss:matrix.org> It was unclear from the conversation if the current API enables them to achieve that without over-instrumentation of their customers' code (which they prefer not to do) Yeah, this can be another interesting topic that we can discuss about. Node.js provides experimental [`diagnostics_channel`](https://nodejs.org/api/diagnostics_channel.html#diagnostics-channel) for library providers to expose certain events, for example: https://github.com/nodejs/undici/blob/main/docs/api/DiagnosticsChannel.md. [01:54:19.0326] I don't think async context is the exact only tool helping in this problem. [01:56:40.0201] > <@yoavweiss:matrix.org> It was unclear from the conversation if the current API enables them to achieve that without over-instrumentation of their customers' code (which they prefer not to do) * Yeah, this can be another interesting topic that we can discuss about. Node.js provides [`diagnostics_channel`](https://nodejs.org/api/diagnostics_channel.html#diagnostics-channel) for library providers to expose certain events, for example: https://github.com/nodejs/undici/blob/main/docs/api/DiagnosticsChannel.md. [02:00:55.0724] https://docs.google.com/document/d/16HstQ6yHWMhFHflrpnKTeH0FDGxpQ8fKWqbA1mos3Ag/edit#heading=h.9g3dvhcn9qro as I jotted down what I think a JS side impl may look like from my perspective [02:01:09.0854] (dunno much about the implementation complexity of actually doing that in V8) [04:43:21.0548] Is `AsyncContext.wrap` only needed in userland JS with things like non-builtin promise libraries? [04:44:14.0441] I was mostly thinking of it from an implementation perspective where you have to wrap when enqueuing promise jobs and other tasks [04:45:00.0954] * I was mostly thinking of it from an implementation perspective where you have to wrap when enqueuing promise jobs and other tasks, so I hadn't really thought about how it'd be used in userland [04:45:11.0404] but I guess it's only really useful at that same level of abstraction [04:54:40.0675] Yoav Weiss: I think what you describe there matches what we're doing in workers. We do have cases where we gave to grab a strong reference the current frame (taskscope in your proposal) and enter it manually (for instance, when queuing unhandled rejection events, or setTimer/setInterval... We also use it for tracing internally). If we had to implement task attribution apis in the runtime I don't think that's too much of a problem tho I would need to see more of what's involved. [06:01:38.0291] > <@abotella:igalia.com> and no matter how that gets implemented in V8, V8-based WinterCG runtimes could handle it the same way as Chromium I don't think this is the case, because Node.js at least needs to be able to host Chrome at the same time. So there's value in the "multiplexing" of different variables being at the V8 level, rather than the blink level. Is that right, James M Snell ? [06:01:59.0732] oh, that's right, Electron [06:06:48.0825] Yep that's exactly it. As long as embedders can capture a reference to the entire frame at a point in time when necessary. [06:07:14.0869] Having v8 manage it for us is best [10:26:12.0921] > <@yoavweiss:matrix.org> It was unclear from the conversation if the current API enables them to achieve that without over-instrumentation of their customers' code (which they prefer not to do) I'm also not certain how they collect their data. If it's anything like OTEL, then we should be pretty good. [10:28:06.0415] > <@abotella:igalia.com> Is `AsyncContext.wrap` only needed in userland JS with things like non-builtin promise libraries? It's also useful when firing batches of handlers with a framework level event listener. Eg, I don't want to install a click listener on every element, I just want one at the root and it'll fire the handlers for the specific element that was clicked. [10:29:10.0288] right, any time you have an API which takes a callback and the API logically "schedules" that callback, it might be a good idea to use .wrap() [10:34:09.0267] > <@littledan:matrix.org> So I'd suggest saying, "asynchronous flow of control", or especially better if we say something more concrete, e.g., "Annotating logs with information related to a particular logically related series of operations, even with callbacks and promises" (or, maybe there's a better way to put that, idk) I disagree with this. I think explaining this in terms of the async callstack is the perfect way, because people already have a understanding of how that propagates [10:34:59.0678] I don't think the we imply that we're capturing the actual function calls anywhere [10:44:08.0635] Particularly for cases like `await promiseFromAnotherContext`, is the context after that await the one from the promise? That would follow control flow, but describing it as the the callstack clears up that confusion [10:44:31.0187] And, it's easy to explain why the context contains a certain value, just poke up the callstack in the dev tools [11:13:19.0130] > <@jridgewell:matrix.org> I don't think the we imply that we're capturing the actual function calls anywhere Huh, what does the term callstack mean to you? [11:14:10.0163] Anyway I am just trying to help you make things are clear, it’s your call how to present it 2023-03-22 [17:30:53.0389] I had some quick chats today at TC39 with dminor and bakkot, everyone seemed cautiously positive though a little uncertain about use cases. I am optimistic about the presentation but we might consider adding bonus slides that go in more depth there (eg just taking what was in legendecas’s webperf wg slides) [17:31:50.0503] I didn’t get the feeling that the complexity around what things are wrapped was considered a very bad issue for either (just something to work out) but maybe I misunderstood them [17:32:02.0818] Good luck this week on proposing for Stage 2! [08:16:26.0038] So, here's a question. fetch has the `Response` object. This can be created using a `ReadableStream` object, which uses a number of internal promises to manage state. In workerd, we have a method that handles received requests and can return a `Response` object specifying the response to an http request. So, imagine a case like: ``` const als = new AsyncLocalStorage(); // or AsyncContext export default { async fetch(req) { const readable = als.run('abc', () => new ReadableStream({ pull(c) { c.error(new Error(`boom ${als.getStore()}`)); } })); return als.run(123, () => new Response(readable)); } } ``` The actual pipe from the readable happens by the code that calls the `fetch` function handler here, which is running in the root async context where `als` store will be `undefined`. What value would you expect `als.getStore()` to return when the error is constructed? [08:17:37.0762] Keep in mind that the pull function can be called at different times depending on the value of the `highWaterMark` configured for the readablestream [08:23:18.0220] specifically, pull might be called when the ReadableStream is created, and any time after while it is being read [08:52:15.0163] I don't have a good answer. Just from the use case I recognize that it must be one of the two (and not `undefined` just because the `Response` object is leaked out of the fetch), but I don't have a good idea of which one of these contexts is the "registration" context [08:53:59.0839] In my experience, the contexts are the same. I create the readable and the response in the same overall context, so the distinction doesn't matter [08:56:02.0429] That's fair. What I suspect is that for web apis, once AsyncContext is integrated, we'll need a way of identifying in webidl which types are expected to capture and propagate the async context [08:56:38.0210] It's going to be needed regardless of where AsyncContext ultimately gets defined (tc39 or whatwg, etc) [10:51:00.0200] Here’s my take: if/once `AsyncContext` becomes an official part of the language, utilities like `ReadableStream` could (backwards compatibly!) begin to support a contract whereby the context in effect at `ReadableStream` construction time is automatically restored each time `pull` and other callbacks are called [10:51:20.0626] In the meantime (potentially for a long time), you might need to take matters into your own hands using `AsyncContext.wrap`: [10:51:26.0955] ```
export default { async fetch(req) { const readable = als.run('abc', () => new ReadableStream({ pull: AsyncContext.wrap((c) => { c.error(new Error(`boom ${als.getStore()}`)); }), })); return als.run(123, () => new Response(readable)); } } ``` [10:51:51.0207] With this code, the `als` with `abc` is bound to the `pull` callback function so `als.getStore()` can return `abc` instead of undefined for the Error [10:52:21.0500] Of course, this `AsyncContext.wrap` binding means any other `AsyncContext` values coming from the caller of the `pull` callback (some code in the `ReadableStream` implementation) will now be ignored, but that’s probably safe as long as you don’t expect/care `ReadableStream` to be manipulating `AsyncContext` [10:52:25.0332] * Of course, this `AsyncContext.wrap` binding means any other `AsyncContext` values coming from the caller of the `pull` callback (some code in the `ReadableStream` implementation) will now be ignored, but that’s probably safe as long as you don’t expect `ReadableStream` to be manipulating `AsyncContext` [10:52:43.0903] Is that helpful? [10:55:27.0151] Unfortunately it doesn't exactly help. If I call c.error() within the pull, and there's a queued read, that queued read is likely not necessarily going to be rejected immediately when the `c.error` is called. It might end up getting processed from a different async scope, which means if it results in an unhandledrejection event, it will propagate the wrong context [10:56:22.0432] going to have to play with this case a bit more to really tease out all the various cases [10:57:10.0760] There are a variety of situations that make this complicated. [10:57:49.0983] whatever code is scheduling/queueing those read jobs needs to be using something like `AsyncContext.wrap` to preserve the context from one job to the next, I think? [10:58:04.0171] are you trying to get this to work without modifying library code that does this kind of scheduling? (seems hard) [10:58:06.0394] for instance, I may have multiple reads queued, all called from separate async contexts, but a mismatched number of pulls, where one pull triggered by one context fulfills multiple reads [10:58:22.0268] yeah, we're not in control of the ReadableStream implementations [10:58:47.0300] ok I think I see the rub you were pointing out [11:00:06.0090] a single invocation of `pull` can represent multiple different contextual branches [11:00:40.0716] * a single invocation of `pull` can handle/inherit multiple different contextual branches [11:02:26.0654] do you think the solution is somehow to merge the branches into one context, or to keep the different branches distinct so `pull` can create the errors with their correct (not merged) contexts? [11:03:17.0428] > <@jasnell:matrix.org> That's fair. What I suspect is that for web apis, once AsyncContext is integrated, we'll need a way of identifying in webidl which types are expected to capture and propagate the async context This differs from how Yoav ended up implementing a similar feature in Blink. It turned out that the WebIDL layer approach was too slow. [11:03:32.0329] instead, it's more like, you propagate the context when queueing a task [11:09:05.0258] so in this case it's a bit complicated. "when queing a task" ... for a readable stream, a pending read can be viewed as a queued task [11:09:21.0620] when a read is received, the stream may or may not call pull to ensure that the read can be fulfilled [11:09:43.0362] and a single pull can cause multiple pending reads to be fulfilled [11:10:04.0388] so, within that pull, should it use the context of the read that immediately triggered it? [11:10:19.0640] or should it use the context that was current when the stream was created? [11:11:23.0871] and if that pull causes an error, causing all subsequent reads to reject with unhandled rejections, should the unhandled rejection events be called from the async context for the individual read or the context that was active when the pull errored [11:13:15.0375] is there some mechanism currently for the `pull` callback to tell why an error occurred, in a way that points back to one particular read? [11:13:28.0021] no [11:14:07.0364] whether it throws or calls `controller.error(...)` the effect is the same, the stream is put into an errored state such that all pending and future reads are rejected with that reason [11:14:47.0568] all pending reads would be rejected within the same async context and not the frame that was current when the read was scheduled [11:15:17.0233] so the unhandledrejection handler would see that context and the context that scheduled the read in the first place would be lost [11:17:17.0474] Here's another example: ``` const als = new AsyncLocalStorage(); const readable = als.run(123, () => new ReadableStream({ pull(c) { c.error(`boom ${als.getStore()}`); } }, { highWaterMark: 0 })); const response = als.run('abc', () => new Response(readable); const reader = als.run('xyz', () => response.body; const read = als.run(321, () => reader.read(); await Promise.all([ als.run('???', async () => await read), als.run('???', async () => await read) ]); ``` [11:17:43.0396] * Here's another example: ``` const als = new AsyncLocalStorage(); const readable = als.run(123, () => new ReadableStream({ pull(c) { c.error(`boom ${als.getStore()}`); } }, { highWaterMark: 0 })); const response = als.run('abc', () => new Response(readable); const reader = als.run('xyz', () => response.body; const read = als.run(321, () => reader.read(); await Promise.all([ als.run('???', async () => await read), als.run('!!!', async () => await read) ]); ``` [11:18:25.0537] sorry that's a bit off... one sec [11:18:26.0671] I'm not sure this works as you'd hope: `const reader = als.run('xyz', () => response.body)` [11:18:57.0248] * Here's another example: ``` const als = new AsyncLocalStorage(); const readable = als.run(123, () => new ReadableStream({ pull(c) { c.error(`boom ${als.getStore()}`); } }, { highWaterMark: 0 })); const response = als.run('abc', () => new Response(readable); const reader = als.run('xyz', () => response.body; const read1 = als.run(321, () => reader.read(); const read2 = als.run(567, () => reader.read(); await Promise.all([ als.run('???', async () => await read1), als.run('!!!', async () => await read2) ]); ``` [11:19:23.0332] that's thrown in just to make it more complex. I know it wouldn't do anything :-) [11:19:26.0871] * I'm not sure this works as you'd hope: `const reader = als.run('xyz', () => response.body)`, since `reader.read` won't automatically restore the `xyz` context value when called [11:19:41.0583] * Here's another example: ``` const als = new AsyncLocalStorage(); const readable = als.run(123, () => new ReadableStream({ pull(c) { c.error(`boom ${als.getStore()}`); } }, { highWaterMark: 0 })); const response = als.run('abc', () => new Response(readable); const reader = als.run('xyz', () => response.body); const read1 = als.run(321, () => reader.read()); const read2 = als.run(567, () => reader.read()); await Promise.all([ als.run('???', async () => await read1), als.run('!!!', async () => await read2) ]); ``` [11:19:52.0217] * Here's another example: ``` const als = new AsyncLocalStorage(); const readable = als.run(123, () => new ReadableStream({ pull(c) { c.error(`boom ${als.getStore()}`); } }, { highWaterMark: 0 })); const response = als.run('abc', () => new Response(readable)); const reader = als.run('xyz', () => response.body); const read1 = als.run(321, () => reader.read()); const read2 = als.run(567, () => reader.read()); await Promise.all([ als.run('???', async () => await read1), als.run('!!!', async () => await read2) ]); ``` [11:23:50.0828] would it be acceptable for the unhandledrejection error to report a set of contexts rather than just one? or is that a nonstarter? [11:24:32.0723] it's called once per each unhandled promise rejection, there's only one context associated with each [11:24:35.0183] I have been thinking we need to extend AsyncContext itself to handle a set of context [11:24:39.0986] reporting a set would not make sense there [11:25:05.0777] how does `pull` know which `read` was to blame, and so which context to report? [11:26:00.0151] so that the context user can gather the values associated with both the registration and the trigger paths [11:26:23.0061] it doesn't. Currently pull would be called within the async context of whatever read triggered it. It could also be called following the completion of the start algorithm microtask (the async context where the stream is constructed). [11:28:13.0023] we can break this down to a few specific questions: 1. Because the various "algorithms" associated with a stream are considered part of its internal implementation (e.g. start, pull, close), should those always be executed in the async context frame in which the stream itself was created [11:29:37.0205] 2. If a read promise rejects and is unhandled, should the unhandledrejection error be called in the async context in which the read promise was rejected (which is likely different than the context that scheduled the read), or the async context that was current when the read was scheduled [11:30:08.0182] * 2. If a read promise rejects and is unhandled, should the unhandledrejection event be called in the async context in which the read promise was rejected (which is likely different than the context that scheduled the read), or the async context that was current when the read was scheduled [11:30:36.0177] * we can break this down to a few specific questions: 1. Because the various "algorithms" associated with a stream are considered part of its internal implementation (e.g. start, pull, close), should those always be executed in the async context frame in which the stream itself was created or should they propagate the context that triggered the algorithm to be called [11:34:11.0291] Mark Miller brought up a point a while back (when `AsyncContext` was last proposed), about what to do with an `AsyncContext.wrap`-bound function that gets called in a context where some `AsyncContext` instance that was previously bound with one value happens to have a different value in the new calling context [11:34:33.0070] Keeping in mind also that people can do silly things like.. ``` let controller; const readable = als.run(123, () => new ReadableStream({ start(c) { controller = c; } })); const read = als.run('abc', () => readable.getReader().read()); als.run(321, () => controller.error('boom')); ``` [11:36:15.0616] Are you expecting the context to be propagated somehow by the assignment `controller = c`? Whatever context is in effect in `start` is pretty clearly lost by this code, right? [11:36:34.0050] I don't expect `controller.error` to have access to anything beyond the `321` context, there [11:37:13.0387] nope, it's definitely lost. so if we always called the algorithm in the context captured when the readablestream was created, this would escape that context and would cause the reads to be rejected in the context where the value is 321 [11:37:17.0995] * Mark Miller brought up a point a while back (when `AsyncContext` was last proposed), about what to do with an `AsyncContext.wrap`-bound function that gets called in a context where some `AsyncContext` instance that was previously bound with one value (for some `AsyncContext`) happens to have a different value in the new calling context [11:37:46.0305] but what should be the context for the unhandledrejection associated with the read [11:37:51.0361] 'abc' or '321'? [11:39:39.0698] I can imagine a `Promise` implementation that has a chance of preserving `abc` (in this code) in the final context associated with the rejected `read` promise [11:41:49.0485] but that's assuming/imagining `Promise` objects capture context at creation time (when `readable.getReader().read()` is called), which is yet to be decided or implemented [11:42:49.0397] Why would a promise ever need to capture the creation time? I only see it the resolution and registration time as being useful [11:43:06.0261] for unhandledrejection error attribution, mostly, I think? [11:43:07.0134] * Why would a promise ever need to capture the creation time? I only see the resolution and registration times as being useful [11:43:23.0932] we've previously decided that, for promises, the context would be captured at the moment the continuation was attached. So either when then is called or we await [11:43:36.0092] that's the rejection time that's useful, not the promise creation time [11:43:42.0871] * for unhandledrejection error attribution/context, mostly, I think? [11:43:59.0203] and that in the deferred promise case, we'd propagate the context that is current when either resolve() or reject() is called [11:44:14.0610] I'm fine with that [11:44:25.0097] but in the ReadableStream case, it's *most* useful to treat `read()` itself as a scheduled task [11:44:26.0157] so it's the context when `reject()` is called that matters? [11:44:47.0263] * so it's the context when `reject()` is called that matters (determines the context reported by the unhandledrejection error)? [11:44:51.0600] since the read promise itself might be rejected from a completely unrelated context [11:46:27.0906] wouldn't the onus be on the `read()` implementation to reject the promise from the context in which `read()` was called if the rejection is related to the read implementation. [11:47:05.0137] yeah, it's not specific to the promise itself [11:47:31.0717] should the read operation itself capture the current context and make sure that whenever that queued promise is resolved or rejected the captured context is entered [11:48:40.0702] or should it always be resolved in the context the stream was created... or should it always be resolved in the context that happens to be current whenever start/pull or whatever mechanism internal to the stream causes the promise to be rejected/resolved [11:48:48.0618] Here is a question: if the user aborts, which causes the read to be rejected, what context matters? the context in which read was started, or the one in which the operation was aborted. I believe the latter is the meaningful one [11:49:55.0468] I agree. but in the unhandledrejection handler for an individual read, does the same answer apply? [11:51:53.0882] I can see arguments for both. For instance, if it's the runtime that ends up aborting the stream, my unhandledrejection handler might want to log request specific details of the read that was canceled, but I've lost that context because the abort came from the runtime in the root async context where no data was captured [11:52:51.0067] I can see how losing track of the call site could be problematic in some use cases, but that's basically amounting to say that we have 3 contexts that can be meaningful: the operation start, the operation resolution, the registration for the result of the operation. [11:52:53.0466] whereas within the cancel algorithm inside the stream itself, I may not care so much about which context was actually propagated [11:53:45.0187] fwiw, I'm perfectly fine with us maintaining that it's the context that called the abort which is important [11:54:06.0158] if someone *really* wants to preserve the context for the rejection then they need to explicitly handle the rejection [11:54:52.0026] I have use cases where I'd like to be able to recover the context of the promise resolution [11:55:16.0823] I can very well imagine some other people may have use cases where they need to recover the context of the promise creation [11:55:28.0749] for my original case, however, the `Response` object is created in association with a context but used outside of that context and the user has no means of attaching rejection handlers to the internal promises so I think having the `Response` capture the context and propagate it makes the most sense [11:55:39.0030] (to be clear, that on top of having the context of the promise handler registration) [11:57:14.0886] I can see this being expensive to implement however 2023-03-23 [20:30:08.0017] I talked through what AsyncContext was with Jordan and he said he understands it better now than previously. I focused on how it was used; he said he found the pseudo-implementation confusing (sorry for the bad recommendation there!) When presenting, I recommend you spend a bunch of the time reviewing what the proposal is, somehow in terms a little different from last time, and what the use cases are, before going ahead. The open questions to answer during Stage 2 should probably go faster without dwelling too much. [20:31:03.0386] Maybe we could have a break in the middle to go through the queue and answer questions about what the proposal is and why we want it, before going on [20:34:25.0179] I think this lack of understanding all of the basics is a common pattern in proposals and doesn’t really reflect on the quality of your first presentation, or on Jordan. I heard similar lack of understanding of the motivation for a couple other proposals whose motivation had been extensively discussed. [22:21:59.0317] Chengzhong Wu: I added you as an editor of the slide deck, if you want to add some slides on OTEL [22:23:21.0899] You can use https://carbon.now.sh/aXSZjJ58QreRZMzMLEfP to generate code images (use 17 lines of code to get optimal size for slides) [22:24:18.0867] I can explain a fair bit of usage and usecases between slides 2 and 3 [22:31:25.0589] I think having a before and after might be helpful. Eg, normal use code that might be written, and how the code has to change (or rather doesn’t have to change) if we add OTEL [22:32:11.0389] A bit like the logs example I used in the last deck, user code doesn’t need to be written with telemetry in mind, it just gets added seamlessly [23:00:32.0098] Thanks, I'll add the slides on OTEL [00:09:15.0004] Note, code images aren’t accessible by themselves but you can fix this by putting the code in the speakers notes [08:31:53.0197] I wonder if the presentation should mention this layering question (about whether it should be a web API) and our plan to investigate it (by trying out implementing it in Chrome) [08:32:15.0277] Chrome has put in a lot of time to articulate that concern, so we should probably speak to it somehow [08:32:26.0391] (I am happy to explain this part if people want) [08:56:19.0526] whether AsyncContext should be a web API ≠ whether Chrome should implement it as a web API [08:56:44.0606] but yeah, it might be good to discuss that as well [08:58:30.0931] * whether AsyncContext should be a web API ≠ whether Chrome (or browsers in general) should implement it as a web API [09:00:03.0090] > <@abotella:igalia.com> whether AsyncContext should be a web API ≠ whether Chrome (or browsers in general) should implement it as a web API Yes, I think this is a good point to make in the presentation as well [09:00:25.0860] But I believe Yoav if he says it is easier for him to implement and expose it as a web API [09:00:36.0606] Slides updated, and I added the code as speaker notes [09:00:59.0903] * But I believe Yoav if he says it is easier *for him* to implement and expose it as a web API [09:01:37.0601] > <@littledan:matrix.org> I wonder if the presentation should mention this layering question (about whether it should be a web API) and our plan to investigate it (by trying out implementing it in Chrome) I'm going to mention this in my foreword [09:43:07.0529] I for one would prefer seeing this in the language itself [09:46:37.0970] While I think I'd prefer language level also, I'm sympathetic to the web API approach and it's likely easier for us to implement that way anyway. Specifically, even if it were in the language we'd want v8 to treat it largely as an embedder API anyway to make it easier for us to integrate with other embedder use cases [09:49:52.0537] Could you be more specific about those other embedder use cases? [10:00:36.0893] > <@mhofman:matrix.org> I for one would prefer seeing this in the language itself Maybe this is something you could articulate (with justification) in a queue item after the presentation? [10:01:22.0294] > <@jasnell:matrix.org> While I think I'd prefer language level also, I'm sympathetic to the web API approach and it's likely easier for us to implement that way anyway. Specifically, even if it were in the language we'd want v8 to treat it largely as an embedder API anyway to make it easier for us to integrate with other embedder use cases Could you elaborate on this point? It might feed into the design of the V8 API, to ensure it meets your use cases (if we go that way at all). [10:04:15.0755] I need to articulate it better, but I think what I'll need is to extend the current API to support capturing both the registering context AND the resolving context, mostly to support join type use case like `Promise.all`. [10:10:19.0899] > <@mhofman:matrix.org> I need to articulate it better, but I think what I'll need is to extend the current API to support capturing both the registering context AND the resolving context, mostly to support join type use case like `Promise.all`. huh, this is a big request; I'm looking forward to you articulating it more. [10:11:35.0065] I don't believe it's that big, we already need to capture the rejection context for unhandled rejections [10:14:31.0435] when a context is captured, how are we imagining it can be accessed later? [10:14:41.0611] maybe it's an `AsyncContext.wrap`-bound function that runs other functions in that bound context? [10:15:11.0951] I have a sketch I'll try to clean up [10:16:33.0975] are you hoping these registering/resolving contexts will be preserved separately, or merged into one context? [10:18:16.0822] I just need to be able to get the list of my values associated to the different executions if there are multiple. Aka `asyncContext.getAll(): Array` [10:19:28.0557] that's nice because it lets consuming/application code decide how to merge the multiple values [10:20:08.0465] I don't understand in what situations we'd provide multiple contexts, and how we'd expose them. It seems to complicate what model we should use for embedders. So I'm looking forward to more concreteness and use cases here. [10:20:16.0416] Instead of introducing a new API, why can't the context be used with an array of values? [10:20:34.0915] Let's focus right now on what we need to get to Stage 2, and then open this question up more aftewards [10:20:36.0771] `AsyncContext` -> `AsyncContext` [10:21:04.0332] ``` const p = new Promise(resolve => { asyncContext.run(123, resolve); }); asyncContext.run(456, () => p.then(() => console.log(...asyncContext.getAll()))); // Prints 456, 123 ``` [10:21:32.0287] say a function is bound with `AsyncContext.wrap` in one context and then called in another context, where some of the `AsyncContext` values in the two contexts are not the same (a "merge conflict" if you will) [10:22:08.0528] * say a function is bound with `AsyncContext.wrap` in one context and then called in another context, where some of the `AsyncContext` values in the two contexts are not the same [10:22:38.0644] Yes, I'm not trying to ask to change anything now, but it's something I'd like us to consider at stage 2 [10:22:51.0520] I think we've been referring to those as "snapshots" or "maps", rather than "contexts" [10:23:14.0581] OK, good, I'm looking forward to discussing this during Stage 2 once you're able to articulate the motivation [10:23:20.0071] I'm _all for_ avoiding the naked word "context" since it has so many overloads 😅 [10:23:41.0415] I like "snapshots" here [10:23:48.0449] maybe we should make, say, monthly AsyncContext calls which would give an easy slot for this discussion [10:23:53.0392] The motivation is join like execution flows, of which the prominent example is `Promise.all` [10:24:40.0364] * I'm all for avoiding the naked word "context" since it has so many overloads 😅 [10:25:09.0554] It will be good to hear what problem you're trying to solve concretely, just to give an example. But I'm not asking for an answer right now. [10:25:53.0758] when you do Promise.all(...).then, the execution context is clear (it's inherited from where Promise.all is called, and has nothing to do with the promises passed into Promise.all) [10:25:57.0411] > <@littledan:matrix.org> maybe we should make, say, monthly AsyncContext calls which would give an easy slot for this discussion 👍 We can establish the regular call after the plenary [10:27:00.0182] Also, the context that's active when the all'd promise's `resolve()` is invoked is only the last promise item to resolve. [10:27:26.0022] Sure but this is also not used in the `.then` [10:27:40.0736] ongoing/open question: what is the best data structure to represent the snapshots? [10:28:06.0893] Right, I said it's the most obvious case, but basically it boils down to I need to learn the data associated with the context that resolved the promise [10:28:54.0860] Yeah, I just am missing how this applies at all. So I'm looking forward to an example application, maybe just concretely what motivates you to start thinking about this (even if that's not a relevant motivation for everyone) [10:31:25.0856] I'm actually surprised no one wants this. It's invaluable for diagnostics to understand the 2 causality paths on how you got where you are. Sure the registration path is often the most important, but the resolving path is also relevant. [10:32:05.0609] OK, good, we're getting concrete: diagnostics. [10:32:26.0152] That is one of my use cases yes [10:32:58.0473] OK, looking forward to the writeup giving the set of your use cases; sorry for the distraction from Shu's presentation. [10:33:49.0161] I think I share/want this use case, though I need more details for alignment [10:34:01.0392] looking forward to digging into use cases during stage 2 [10:34:20.0812] This feels like something which shouldn't be shared to me? I don't expect that resolving a promise exposes anything to consumers of the promise except the value of the promise [10:34:35.0899] the async context which I was in when I did the resolving isn't theirs to see, it's mine [10:34:53.0301] (My intuition matches bakkot's. I'm really baffled by this whole thread. That's why I'm asking for the motivation.) [10:35:26.0463] to clarify, you can only learn information if you associated a value through your own `asyncContext.run` [10:36:06.0912] my general motivation is that it should be feasible/cheap to keep around (potentially many) snapshots of past context, so you can inspect them for diagnostic purposes later (in development, especially) [10:36:39.0986] `wrap` is the capability to snapshot contexts [10:36:42.0050] I'm not sure exactly what snapshots should be preserved/exposed/reported for promises specifically [10:37:10.0671] you might not want to allocate a function for every snapshot you capture? [10:37:43.0758] It's frequently been posited that `wrap` may be less efficient than something like AsyncResource. I think this is something we should investigate in the context of an actual implementation during Stage 2. [10:37:44.0844] * I'm less opinionated about exactly which snapshots should be preserved/exposed/reported for promises [10:37:54.0903] It's clear that whatever mechanism we adopt here has to be "efficient enough" [10:38:31.0341] this still doesn't seem to relate to the idea of multiple inheritance of AsyncContext stuff [10:39:39.0217] > <@littledan:matrix.org> It's frequently been posited that `wrap` may be less efficient than something like AsyncResource. I think this is something we should investigate in the context of an actual implementation during Stage 2. to be clear, I mean: if `wrap` is too inefficient, then we should definitely switch to a different API, maybe one more shaped like AsyncResource. [10:40:07.0185] Mathieu Hofman [moving off thread for readability]: also if you want to pass data from the promise-resolver to the consumer-of-the-resolved-promise, you can just... stick it in the promise? that seems like a very very different thing than the async contexts proposal [10:40:26.0711] I think there are some other valid critiques of `wrap`, but it's possible to build many of the patterns you might want in userland, using only the current proposal [10:40:59.0361] I agree, I think the promise should be responsible for passing data [10:41:05.0205] I do not control intermediary code that creates and resolved these promises [10:41:07.0870] > <@benjamn:matrix.org> you might not want to allocate a function for every snapshot you capture? yeah I was specifically responding to this: you can't rebuild performance in JS [10:41:15.0276] * I do not control intermediary code that creates and resolves these promises [10:41:24.0094] but `wrap` captures the expressiveness of snapshotting [10:41:26.0227] Hence I cannot pass data throughout [10:41:32.0712] And in most cases, because you own the `AsyncContext` that you invoked `.run()` on, it should be possible for you to chain onto the promise and pass the value [10:41:35.0012] That's what async context is for in the first place [10:41:51.0640] wouldn't async context Just Work for what mathieu's talking about tho? shouldn't every promise spun off of "one that has the context" also have it? [10:42:00.0042] As I mention in the thread, I do no control intermediary promises [10:42:14.0303] * As I mention in the thread, I do not control intermediary promises [10:42:20.0920] i'd assume that "adopting the state of another promise" also adopts/merges its context map or whatever the term is [10:42:22.0539] No, but you control the promise as it's returned from your `.run()` [10:42:57.0517] You're assuming the promise returned by run is related? [10:43:11.0258] another issue is that `wrap` always ignores the current calling context of the function, when you might potentially want to merge the originally bound snapshot with the calling snapshot [10:43:34.0246] Yes? If it doesn't escape the run, then the context should be whatever is currently in the context. [10:43:46.0832] `run` creates an execution flow. the program can store / share promises of their own creation between flows [10:43:48.0475] If it does escape the run, then you control it. [10:44:02.0092] yeah, this is what I'd like to understand better: the use cases for merging. E.g., do you need to merge over all variables, or just one or two that you're thinking about. [10:44:54.0192] I don't control promises resolved from that promise [10:45:35.0906] This discussion is just too abstract for me to understand it. Let's pause and continue once we have this written example, as Mathieu Hofman has already promised. [10:45:52.0816] I'm baffled by the request to explicitly attach information to promises given the whole premise of this proposal is based on the fact it's pretty much impossible to do that when you don't control the program [10:48:49.0648] This "merging" is exactly what I have in mind [10:48:55.0067] I'm hoping it's the latter, so merging can be cheap [10:49:07.0885] (I just still don't understand what's being discussed) [10:49:09.0110] if the merge is lazy, so the system does not have to eagerly merge all the variables up-front, you could potentially wait to see which variables are accessed, and resolve merge conflicts then [10:52:23.0526] (yeah still confused; looking forward to concrete applications) [10:58:56.0179] I guess basically my thought process is: - if it's a promise you make/resolve yourself, you can thread state through - if it's not, the person doing the resolving isn't expecting to give the consumer any information other than the promise value itself; promises are supposed to be dumb wrappers for values maybe I'm missing something because we don't have a concrete example in front of us though. [11:03:29.0266] right but isn't the point of asynccontext that the person doing the resolving doesn't have to *know* to pass stuff down, it's just ambiently available as long as you have the key? [11:04:00.0753] in react, context's entire purpose is to replace prop drilling in a way that still avoids globals by requiring the context key, and this seems similar to me [11:10:20.0743] the thing which gets passed down is created when you create the callback, not when you resolve the promise [11:10:47.0699] capturing state when you resolve the promise is a very different thing than capturing state when you create the callback [11:11:14.0953] though again hard to talk about without something concrete [11:28:18.0681] +1 for a description (like symbols have) [11:29:00.0282] default value could be a getter method - so there could be a choice to either return a value, or throw a customized Error [11:43:25.0474] > <@phryneas:matrix.org> default value could be a getter method - so there could be a choice to either return a value, or throw a customized Error +1, this can avoid unexpected modification on the default values somehow [11:45:45.0036] while `defaultValue` is super convenient, you could implement a subclass of `AsyncContext` that takes a `defaultValue` in the constructor and overrides the `get` method to return that default [11:47:23.0331] true. [11:47:34.0739] eek... I'm not sure I'm a fan of this at all, especially with the values being just an array like this. It also greatly complicates the implementation which currently allows each context frame to be independent of all others, including the frames they inherited from [12:12:41.0447] ljharb: agreed it needs to be easy, but I also believe we need ways of opting out (which `snapshot()` and `wrap()` help with). [12:14:38.0533] congrats on stage 2!! 🥳 [12:15:57.0597] Yay congrats!!! [12:16:09.0903] The next steps for next meeting are clear: Focus completely on use cases [12:16:19.0566] 🎉 [12:16:33.0078] > <@rbuckton:matrix.org> ljharb: agreed it needs to be easy, but I also believe we need ways of opting out (which `snapshot()` and `wrap()` help with). totally agree; i'd prefer the default be "include everything" and the methods to be for opting out [12:16:50.0275] In our new regular meetings, we can dig down towards proposed answers to the many questions Justin explained. I think we demonstrated to committee that we are looking into details, but I think we should be proposing answers at the same time as we explain the issues. [12:17:07.0709] so some time *later* than next meeting (where we focus on use cases) we can present details on those. [12:27:24.0559] If someone really wanted multiple values like this they could easily do something like this with the current API... ``` const ac = new AsyncContext(); ac.run([], () => { const p = new Promise(resolve => { ac.get().unshift(123); resolve(); }); ac.get().unshift(456); await p; console.log(ac.get()); // 456, 123 }); ``` [12:34:51.0227] I do not control the sites where resolve is called, my example was an simplification [12:38:44.0082] aka the `run` call is where I invoke the user program, which can do all sorts of things with promises. The important part is that the program will be invoked with different `run` contexts, and is thus able to "join" these. When they call back into my supervisor, I need to be able to look up all the associated run values, not just one (which usually would be the last in a join) [12:39:23.0402] > <@ljharb:matrix.org> totally agree; i'd prefer the default be "include everything" and the methods to be for opting out I don't think "capture context at declaration" makes sense. At some point you will execute the function, and you want to use the current context to do it. If you want to capture the context at the time of declaration, that's what `wrap()` is for. [12:41:09.0848] "capture context at declaration" would also make `snapshot` completely useless. One of the benefits of `snapshot()` is that you can capture a "clean/empty" environment from which to start fresh, much like how some code today captures primordials early. [12:50:21.0441] Unless I am misunderstanding your point about capturing context [13:00:38.0338] i mean like, `arr.map(() => { /* context should be available here no matter when or where this function is called */ })` [13:01:29.0745] * i mean like, `arr.map(() => { /* context should be available here no matter when or where this function is called */ })` or replace `arr.map(` with `someAPI(` [13:01:49.0251] > <@rbuckton:matrix.org> "capture context at declaration" would also make `snapshot` completely useless. One of the benefits of `snapshot()` is that you can capture a "clean/empty" environment from which to start fresh, much like how some code today captures primordials early. Indeed. Something that people might be missing is, the innermost time setting the context "wins". So if all closures captured the context, there'd be no overriding it. [13:03:57.0361] > <@ljharb:matrix.org> i mean like, `arr.map(() => { /* context should be available here no matter when or where this function is called */ })` or replace `arr.map(` with `someAPI(` I disagree. That is specifically what `.wrap()` is intended to address. This whole feature is related to async flow through call stacks. A function declaration doesn't have a call stack. [13:05:25.0346] JS has deep dep trees, and code i don't control will be wrapping my functions, and/or calling them, all the time - it seems really unfortunate if i have to `.wrap()` manually in order to ensure my context works [13:06:36.0115] > <@ljharb:matrix.org> JS has deep dep trees, and code i don't control will be wrapping my functions, and/or calling them, all the time - it seems really unfortunate if i have to `.wrap()` manually in order to ensure my context works This should really not be the case; `.wrap` is pretty obscure and things should inherit correctly for the most part. The exception is if you're somehow making your own system for an event loop/queueing/batching, for callbacks (promises are already handled comprehensively) [13:07:05.0695] AsyncContext inherits automatically over deep dependency trees [13:07:36.0910] should host environments use `AsyncContext.wrap` to wrap `setTimeout` and `setInterval` callbacks? [13:07:50.0688] * should host environments use `AsyncContext.wrap` to wrap `setTimeout` and `setInterval` callbacks? (since they're the ones implementing those scheduling tools) [13:07:52.0701] > <@benjamn:matrix.org> should host environments use `AsyncContext.wrap` to wrap `setTimeout` and `setInterval` callbacks? (since they're the ones implementing those scheduling tools) They should use the same underlying algorithm at some point, yes [13:15:42.0158] > <@littledan:matrix.org> This should really not be the case; `.wrap` is pretty obscure and things should inherit correctly for the most part. The exception is if you're somehow making your own system for an event loop/queueing/batching, for callbacks (promises are already handled comprehensively) I think this is exactly the right take. For 99% usecase, it happens correctly, automatically. If you're not explicitly making your own queueing/batching system, you won't even need to think about this. [13:16:36.0042] if that's actually the case then that's great! [13:17:04.0258] i look forward to being convinced by more examples in the future :-) perhaps a temporal-like "cookbook" would help? [13:21:48.0431] > <@ljharb:matrix.org> i look forward to being convinced by more examples in the future :-) perhaps a temporal-like "cookbook" would help? good idea! Maybe it could be derived from these excellent slides by Chengzhong Wu https://docs.google.com/document/d/1wO7mbGr6f3tDnEWAaMSVI0bMeEHIdcLbAKeOQe-Zh4U/edit#heading=h.71r4ayj7zowk [13:24:33.0152] is `AsyncContext.wrap` expected to be free if there's no active contexts? [13:24:41.0275] or as free as like a `.bind` or whatever, at least [13:25:02.0306] I would hope so but just want to confirm [13:25:12.0094] (otherwise we probably want a "is there an active context" so you can conditionally wrap) [13:25:43.0678] I expect implementations could make it free [13:26:04.0395] say, make it an optional weak map [13:26:12.0684] * say, make it an optional/nullable weak map [13:26:53.0124] It should be as fast as bind, but it could be optimized for memory to return a blank wrapper function if there's not context [13:27:10.0622] But that could be done as an impl optimization, not something we need to explicitly design in the spec. [13:27:15.0369] * say, make the snapshot an optional/nullable weak map [13:28:15.0330] Note that the pre/post steps of the wrapper need to run, setting the global state to the empty context, so that can't go away [14:32:18.0813] are we considering dependency injection as a potential application of `AsyncContext`? [14:33:08.0427] > <@benjamn:matrix.org> are we considering dependency injection as a potential application of `AsyncContext`? If you have thoughts about how/whether it would be useful, it'd be really helpful to have a writeup on that. I would expect it to be controversial, however. [14:33:16.0562] that doesn't seem like a good strategic choice to market it that way [14:34:27.0403] People will definitely use it that way, after having done that for half a decade with a React Feature of the same name :) Although, technically, "DI" is the wrong term for it ^^ [14:35:03.0917] another technical term with almost as many flavors as "context" [14:35:13.0870] hehe look you've already been called wrong two different ways [14:35:51.0667] I had a twitter argument (I was calling it DI) with someone making very good arguments for "if you have to call out for it, it's not injection" - I still keep using DI though :) [14:37:01.0722] cool, I appreciate that kind of feedback [14:37:36.0601] * I had a twitter argument (I was calling it DI) with someone making very good arguments for "if you have to call out for it, it's not injection" - I still keep using the term DI though :) [14:37:38.0584] DI in JS is just function arguments, and i like it that way :-) [14:38:31.0607] sometimes! [14:39:10.0392] I like how static the parameter decorators for DI could be, compared to the more dynamic version you'd get with `AsyncContext` [14:39:24.0144] * I like how static(ally analyzable) the parameter decorators for DI could be, compared to the more dynamic version you'd get with `AsyncContext` [16:29:34.0559] > <@bakkot:matrix.org> is `AsyncContext.wrap` expected to be free if there's no active contexts? Not if it can't ensure calling it later won't adopt a different context. This is necessary to escape out of a context later, which is necessary to create isolation [16:31:26.0983] > <@ljharb:matrix.org> DI in JS is just function arguments, and i like it that way :-) I mean, parameter decorators just tell the DI container what arguments to put where. [16:33:15.0981] I think DI patterns are far better for improving testability than hacking the module loader like Jest does to perform mocking. 2023-03-24 [17:30:16.0617] sure. low bar, though. [22:27:03.0448] A lesson: don’t talk about DI with respect to TC39 proposals [15:54:27.0990] Overall, really great job in the presentation, Justin Ridgewell and Chengzhong Wu! And great work supporting this proposal all around from everyone else. I think the next steps are: - For next meeting, we should have a presentation which focuses on use cases, being both broad and concrete. Maybe Chengzhong Wu could give this talk, based on his past presentation. [If we make progress on any of the below items before next meeting, we could *briefly* explain that, but the focus would be the overall motivation, which we still haven't fully gotten across to the committee.] - Chengzhong Wu will schedule a regular call to discuss this proposal and its evolution. (Should this be monthly? Biweekly?) We can check up on progress on various work items, and talk through what resolution we should take on the various items that Justin presented to the committee, and once we come to a resolution, assigning different spec-writing tasks to different people. This can also be where we dig into the "merging" idea that Ben Newman (Apollo, @benjamn on GH) and Mathieu Hofman have raised. Remember that, at Stage 2, it remains OK for us to land PRs without waiting for committee consensus. - On the host integration: Andreu, Yoav, ms2ger and Chengzhong to work on a draft "processing model" for HTML which explains how task attribution/AsyncContext works. This might involve iterating on the proposed layering, and might involve treating things like the incumbent Realm as analogous to an AsyncContext variable, etc. Several use cases (both the built-in cases that Yoav is looking at, and the JS-level AsyncContext cases that the rest of us here are thinking about) should be considered in terms of what "processing model" would work best. - Implementation: Andreu Botella and Chengzhong Wu to collaborate on an implementation in V8, potentially informed by pending feedback from shu about what performance concerns the V8 team might have. This process should start with a design doc to be reviewed in v8-dev, like all major V8 projects; the design doc should include the V8 API changes, which Yoav can review. Once this has a complete CL draft, they will work with Yoav Weiss to integrate it with TaskAttribution. It's currently unclear yet whether this will work with the existing TaskAttribution implementation or if significant changes are required; it might be at this point that we all realize that it'd be a lot easier to do this as a Web API, and change course. Maybe people could work on implementing this feature in a second browser as well, and/or test262+web-platform-test tests. 2023-03-28 [23:55:57.0361] https://docs.google.com/spreadsheets/d/1WMFWa0quTuZgNgwJAn1nPcQMNpdTm1PIMWacYgGbUd8/edit?usp=sharing&resourcekey=0-B9OI2cBK2Bik5SAwc-yerg [23:57:02.0845] Please feel free to fill out the call doodle :). The deadline is April 7. After the time and cadence are finalized, I will send out a calendar invite. [00:13:40.0076] * https://github.com/tc39/Reflector/issues/463 [00:14:27.0956] * [Doodle](https://docs.google.com/spreadsheets/d/1WMFWa0quTuZgNgwJAn1nPcQMNpdTm1PIMWacYgGbUd8/edit?usp=sharing&resourcekey=0-B9OI2cBK2Bik5SAwc-yerg) 2023-03-29 [07:59:26.0189] This is really good: https://github.com/tc39/proposal-async-context/issues/45 [16:09:51.0601] I didn't realize how closely our API matched [Python's](https://peps.python.org/pep-0567/) [16:10:06.0553] Even my suggested constructor params, method names, etc, all somehow the same [16:10:48.0700] Their `set()` and `reset()` API is worse, though. [16:59:33.0853] I like the idea of calling the instances “context variables” instead of “async contexts”. This seems more intuitive [16:59:34.0710] Also I like the name “context” for the AsyncResource/wrap thing [16:59:35.0348] The cross-linguistic comparison is interesting because I think much of the committee thinks we are doing a crazy C# thing when actually everyplace that has async await finds the need for this [16:59:36.0227] Maybe that should make it into our next presentation 2023-03-30 [17:05:36.0483] (Does the community feel like Python is falling off the complexity cliff? They seem to have All The Features) [17:08:59.0843] Thanks for the reference; I hadn’t seen this [18:50:29.0160] Wanna weigh in at https://github.com/tc39/proposal-async-context/issues/44? [18:52:31.0722] Ah haha yes that sounds more productive than rambling here… On my todo list