-
Notifications
You must be signed in to change notification settings - Fork 63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Consumers should be held weakly to allow all nodes to be live #252
Comments
In my opinion |
Actually, |
That said, there is a similar design which I think is quite promising: use a strong reference from producer to consumer but use |
IIRC, Angular used it for both producers and consumers. |
I would love to learn more about this, because on paper it seems like holding the strong references would prevent it from being garbage collected, and hence |
It's a little more challenging with the TC39 design since the reactive node and the user-facing signal object are the same entity. In Angular signals, a computed signal is a "getter function" which closes over the reactive node object and interacts with it to produce a value. Only the user's code ever references the getter function, the reactive graph pointers are always from node to node. We could use Sometimes an orphan node object might still have consumers which previously read the computed and are still alive. If this is the case, then we can add the dependencies of the computed node as dependencies of the computed node's consumers directly. This would look like |
I agree, cf #227 However, if (unlike #227) all nodes are held live (with a WeakRef), it means |
(This issue summarizes a discussion from the community Zoom call, to create a public record for any design decisions made in this space.)
The current design only holds reverse dependencies for the purpose of stale signal propagation on nodes that are being watched/live. Otherwise the "stale" state of a computed is determined by transitively checking the entire graph, which is O(n) on the number of nodes that are transitively depended on. (There is an optimization where this can be short-circuited if no nodes in any graph in the entire system have been marked stale since the last read, but that isn't helpful in most real applications).
This is a performance foot-gun, where a Signal can have drastically different performance characteristics depending on if some other code is making it live or not.
The proposed alternative design that avoids the issue would be to use
WeakRef
(in JS parlance) to always store the consumers (reverse dependencies) of a node. This allows for constant time checking if any node is stale, by eagerly propagating the dirty flag along all the weak edges.There seems to be some suspicion of using
WeakRef
from certain individuals. I'm not totally certain what the concern is, but I believe it is a vague sense of "WeakRef
looks nice on benchmarks, but those don't take into account the extra pressure put on the garbage collector by their use". I would love for any individuals with more information in that regard to contribute here to the discussion.If that concern is true, then we also need to determine if the browser-native equivalent of
WeakRef
also causes the same issue.The text was updated successfully, but these errors were encountered: