Skip to content
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

remove cx #17

Open
obust opened this issue Aug 19, 2024 · 1 comment
Open

remove cx #17

obust opened this issue Aug 19, 2024 · 1 comment

Comments

@obust
Copy link

obust commented Aug 19, 2024

leptos managed to remove the cx Scope in 0.5.0

In order to provide signals that implement Copy are are 'static and are therefore easy to use with closures and event listeners in 100% safe Rust, Leptos allocates memory for signals, memos, and effects in an arena. This raises the question: When is it safe to deallocate/dispose of these signals?

From 0.0 to 0.4, Leptos allocated signals in a dedicated Scope, which was ubiquitous in APIs. This had several drawbacks

Ergonomics: It was annoying additional boilerplate to pass around.
Trait implementations: Needing an additional Scope argument on many functions prevented us from implementing many traits that could not take an additional argument on signals, like From, Serialize/Deserialize.
Correctness: Two characteristics made this system somewhat broken

The Scope was stored in a variable that was passed around, meaning that the “wrong” scope could be passed into functions (most frequently Resource::read()). If, for example, a derived signal or memo read from a resource in the component body, and was called under a Suspense lower in the tree, the Scope used would be from the parent component, not the Suspense. This was just wrong, but involved wrapping the function in another closure to pass in the correct Scope.
It was relatively easy to create situations, that could leak memory unless child Scopes were manually created and disposed, or in which on_cleanup was never called. (See https://github.com/leptos-rs/leptos/issues/802 and https://github.com/leptos-rs/leptos/pull/918 for more background.)

The solution to this problem was to do what I should have been doing a year ago, and merge the memory allocation function of Scope into the reactive graph itself, which already handles reactive unsubscriptions and cleanup. JavaScript doesn’t deal with memory management, but SolidJS handles its onCleanup through a concept of reactive ownership; disposing of memory for our signals is really just a case of cleanup on an effect or memo rerunning.

Essentially, rather than being owned by a Scope every signal, effect, or memo is now owned by its parent effect or memo. (If it’s in an untrack, there’s no reactive observer but the reactive owner remains.) Every time an effect or memo reruns, it disposes of everything “beneath” it in the tree. This makes sense: for a signal to be owned by an effect/memo, it must have been created during the previous run, and will be recreated as needed during the next run, so this is the perfect time to dispose of it.

@viridia
Copy link
Owner

viridia commented Aug 19, 2024

The current ownership model for bevy_reactor is inspired by Solid: signals, effects, and memos are entities which are owned by the TrackingScope, an ECS component which is attached to the entity for the parent effect or memo.

Unfortunately, Cx has other things in it besides a reference to the tracking scope: it has the owner entity id, and a reference to the Bevy World. The world reference is used for things like accessing resources, and is the most difficult to deal with from a borrowing/mutability standpoint. Those things are still needed, so there needs to be some mechanism for passing them in to the reactive function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants