| 06:51 | <Mathieu Hofman> | The proposal very nearly avoids the need to support shared->local edges. The incremental cost of having to overhaul GC is significantly larger than the incremental benefits for the few use cases. |
| 15:55 | <iain> | I think everybody agrees that in the long run it would be good to have shared structs that can be used as weakmap keys. My argument is that in terms of the actual implementation effort, supporting shared-to-local edges (which is required to implement weakmaps that don't leak) is a major implementation challenge (like, 1/3 to 1/2 of the overall proposal?). The new capabilities that it unlocks are relatively small (compared to the overall proposal). So the question is whether it is better to ship a complete version of shared structs in N years, or a more restricted version in N/2 years followed by a complete version N/2 years later. |
| 15:59 | <iain> | I also expect that getting implementation feedback from more than one engine before locking in the final proposal will lead to a better end result. |
| 16:22 | <littledan> | Didn't V8 also initially consider banning shared to local edges? Do we know more about what led V8 to change their minds? |
| 16:28 | <iain> | Sometimes you want to associate some necessarily local data (eg an event handler, a DOM node) with a shared struct. |
| 16:29 | <littledan> | are you saying, we could support those edges, just not for WeakMaps? |
| 16:33 | <iain> | V8 isn't proposing unrestricted shared to local edges. But putting shared structs in weak maps ends up in a similar situation with respect to cycles through the shared space. |
| 16:35 | <iain> | Eg if you have two shared structs and two workers, and each worker has a weakmap with one shared struct as a key and the other as the corresponding value, then how do you collect that? |
| 16:38 | <iain> | The "no shared to local edges" principle, if robustly enforced, means that you can do local collections without having to coordinate with anybody else. Once you break it, then you can have cycles that (as far as anybody can tell) require you to stop the world occasionally to avoid leaking memory. |
| 16:40 | <littledan> | oh I see. Yeah, if we want to restrict those edges, then restricting weakmap keys is part of it. Seems reasonable |
| 18:16 | <ljharb> | having an object that you can't store in a weakmap seems like a nonstarter tho |
| 18:20 | <iain> | Shared structs are going to be weird in all sorts of ways. Why is this specific restriction a non-starter (compared to, say, not being able to store references to local objects)? |
| 18:24 | <iain> | I reiterate that from an implementation perspective this is genuinely a major additional effort to implement well, on the rough order of magnitude as the rest of the proposal put together. |
| 18:25 | <iain> | (It can be implemented badly with less effort, but there are significant web-compat concerns there.) |
| 18:28 | <Mathieu Hofman> | I there a middle ground where collecting non cyclic shared->local edges created by weakmaps would not cause significantly more implementation work? I think it's totally acceptable for some non common cases to leak at first |
| 18:29 | <Mathieu Hofman> | The concern with not being able to blindly put a typeof foo === 'object' && foo !== null in a WeakMap is a compat concern when an app gradually adopts shared structs |
| 18:32 | <iain> | I think that roughly works out to be "WeakMap as a way to implement main-thread TBS" in the V8 design doc here. I would have to double-check with our GC experts, but I think that's less effort. My concern there is webcompat: if one browser implements that, and another browser implements full cycle collection, then you're going to have websites that leak in one browser but not the other. |
| 18:33 | <iain> | So in practice there's no difference between that middle ground and the full requirement. |
| 18:34 | <iain> | I acknowledge that this makes incremental adoption harder. |
| 18:35 | <iain> | I am not convinced that it makes incremental adoption so much harder that it outweighs the benefit of being able to ship something sooner. |
| 18:36 | <iain> | To be clear, I'm also not convinced of the converse! I just want to make sure that we're all clear on the decision we're making. |
| 19:04 | <ljharb> | Shared structs are going to be weird in all sorts of ways. Why is this specific restriction a non-starter (compared to, say, not being able to store references to local objects)? a new thing accepting a subset of values is fine. an existing thing suddenly changing its current invariants (all objects can be weakly held) is not. this would apply to weakmap, weakset, weakref, and finalizationregistry - that's a lot of things to have changed invariants. if browsers were OK with a |
| 19:08 | <snek> | let isWeakable = (o) => try { new WeakMap([o, 1]); return true; } catch { return false; } |
| 19:12 | <ljharb> | indeed, that'd be a polyfill for it :-) but unless it's built in, the ecosystem won't use it, and there'll be tons of checks like Object(o) === o or o && typeof o === 'object' that naively check for a subset of weakable values, that will break if a non-weakable object suddenly gets passed into something |
| 19:56 | <rbuckton> | I'm still very concerned that shipping shared structs "fast" by dropping agent/realm-local prototypes and not supporting WeakMap is going to significantly hamper adoption for the teams actually requesting this feature. If I don't have the ability to define behavior on the prototype, then I would need to create a membrane over shared data to be able to incrementally adopt. If I can't put the shared structs in a WeakMap to ensure I only ever produce a single proxy for a given shared struct, then I have to put them in a Map. If I put them in a Map, they will leak. If I don't want them to leak, I need to force all of my API consumers to perform manual memory management, which would be a brand new requirement for all API consumers, so we probably just end up leaking memory. That is not a great outcome. |
| 19:58 | <rbuckton> | I would be happy with a scenario where A) we get agent-local prototypes, but B) we can't have shared structs as WeakMap keys, since for (A) those prototypes would probably never be collected anyways since they will be retained by the shared struct constructors and the module graph. |
| 21:19 | <rbuckton> | Actually, having prototypes but not WeakMap support is still a problem. Since prototypes would give us behavior, we wouldn't need a membrane for Nodes, but we still need one for NodeArray. A TypeScript NodeArray is essentially an Array with extra pos/end properties attached. Unfortunately, the SharedArray type introduced in the shared structs dev trial doesn't allow for additional (non-indexed) properties. The only way to handle that transparently/incrementally would still be via a Proxy, which means we still have a potential memory leak if we don't have WeakMap. |
| 21:24 | <rbuckton> | If we have prototypes and WeakMaps, something like
where the proxy emulates |
| 21:26 | <rbuckton> | I experimented with just creating a shared struct with properties like { length; "0"; "1"; }, but the dev trial had major perf issues with struct fields whose names were integer indexes. |
| 21:40 | <rbuckton> | If a
|
| 21:41 | <rbuckton> | If we can get that to work, then my need for WeakMap support drops significantly so long as we have prototypes. |
| 21:56 | <rbuckton> | Assuming shared structs act like primitives, such that their protoypes are looked up, then those prototypes would need to be retained indefinitely even if the shared struct never leaves the agent since it won't, itself, retain a reference to the prototype. |
| 22:03 | <rbuckton> | Which means whatever correlation scheme we use doesn't necessarily need to maintain a shared to local edge anyways. If shared structs are top-level only, their prototypes would be retained by their constructors, which would be retained by the module, which is in turn retained for the lifetime of the program. |