tc39 / proposal-async-context

Async Context for JavaScript
https://tc39.es/proposal-async-context/
Creative Commons Zero v1.0 Universal
587 stars 14 forks source link

Terminology: AsyncContext mapping/store/snapshot #73

Closed andreubotella closed 1 month ago

andreubotella commented 8 months ago

In the spec, the key-value map data structure mapping AsyncContext.Variable objects to their values is called the "Async Context Mapping Record". However, the various fields in which it's stored have different names: [[AsyncContextMapping]], [[AsyncContextSnapshot]], [[AsyncSnapshotMapping]]... We (the various folks working on this proposal) have also sometimes referred to this data structure as the "AsyncContext store". I think we should decide if we keep the name "AsyncContext mapping", and change the various field names to use it.

(This issue is not about renaming the AsyncContext.Snapshot API.)

jridgewell commented 8 months ago

The different [[…]] names were intentional, so that types can't be used interchangeably. Eg, snapshot.run.call(asyncVar, …) should fail because they have different fields. We could standardize on calling all of these …Store, as long as the is unique between types.

littledan commented 6 months ago

Let's stick to "snapshot" for all three of these, since they represent the same thing. Or, we could change the JS-level API, and then use that name for all of them.

Qard commented 6 months ago

WinterCG calls that internal thing AsyncContextFrame, for reference.

littledan commented 6 months ago

WinterCG calls that internal thing AsyncContextFrame, for reference.

Note that the use of that term, "Async Context Frame", is entirely contained in that one document. It shouldn't be taken any more seriously as a "precedent" than the current spec text.

Qard commented 6 months ago

Yep, just sharing an example of a way it has been done before in case that is relevant to the naming, but also possibly relevant to the structure in that AsyncContextFrame combines multiple parts of what AsyncContext does into one place.

The frame approach is especially an advantage because it means the most common case of propagating is made into only a pointer copy while the far less common case of an individual store.run(...) does a small bit of extra work to produce a new frame rather than only updating that store instance. This provides multi-tenancy without O(m x n) cost where m is store count and n is async transition count. It's still O(m x n) but over run counts which should generally never exceed async transition counts and should typically be far less.

Anyway, that's a bit of an aside to the terminology point, but I think what should be said is the terminology is part reflection of the implementation and part reflection of the intent.

littledan commented 6 months ago

What is the counterargument to using "snapshot" everywhere?

Qard commented 6 months ago

It does the same thing as Snapshot, but without needing separate Mapping, Storage, and Revert types. It works by treating the "frame" as immutable, making a clone whenever it would be changed, which happens far less frequently than when a snapshot needs to happen as you need a snapshot every time it propagates. The frame design means propagation is only a copy of the reference to what the current context frame is. A "snapshot" then becomes just a copy of the pointer to the current frame, which is very fast.

The API as it is presently seems a lot more complicated than it actually needs to be. It seems to be optimizing for making writes fast when reads are typically thousands of times more frequent.

jridgewell commented 6 months ago

Based on today's meeting, we'll switch to consistently using "mapping" term internally.

legendecas commented 6 months ago

Using the name [[*Snapshot]] would be misleading as the value of the fields are not AsyncContext.Snapshot. Discussed in the meeting on April 16 that the term "mapping" should be used universally.

Sorry for the duplicated update: my GitHub page lied to me that there was no updates until I force refreshed the page :(