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

Investigate using SlotMap #5292

Closed
kanashimia opened this issue May 23, 2024 · 3 comments
Closed

Investigate using SlotMap #5292

kanashimia opened this issue May 23, 2024 · 3 comments
Labels
needs info Further information from the reporter is requested

Comments

@kanashimia
Copy link

kanashimia commented May 23, 2024

Currently slint uses Rc<RefCell<_>> in lots of places, it causes unnecessary memory fragmentation, and is very error prone, as it essentially moves borrow checking errors to runtime.
In Rust it is typical to use arenas instead of smart pointers for self recursive cases.
SlotMap is basically a generational arena with free list, so it doesn't suffer from memory leaks.
I hope is that it will make code simpler and will help with having less runtime borrow errors.

Example: https://github.com/orlp/slotmap/blob/master/examples/doubly_linked_list.rs

Leptos uses slotmaps for their reactive code, although there are still many Rc<RefCell<_>> there.

The reactive system is implemented as a single data structure that exists at runtime. In exchange for giving ownership over a value to the reactive system (by creating a signal), you receive a Copy + 'static identifier for its location in the reactive system. This enables most of the ergonomics of storing and sharing state, the use of callback closures without lifetime issues, etc. This is implemented by storing signals in a slotmap arena. The signal, memo, and scope types that are exposed to users simply carry around an index into that slotmap.

Their code: https://github.com/leptos-rs/leptos/tree/main/leptos_reactive

@ogoffart
Copy link
Member

Which particular use of Rc<RefCell>> would you consider problematic?
I'm not seeing how this would help in our case.

@ogoffart ogoffart added the needs info Further information from the reporter is requested label May 28, 2024
@kanashimia
Copy link
Author

kanashimia commented May 31, 2024

Sorry my mention of Rc<RefCell<_>> was dubious, will remove it.
In particular I think it may be helpful for internal/core/model.rs and core/internal/item_tree.rs, to simplify those abstractions.
It also may fix issues like #3215 more apparent and easier to fix, but I'm not sure about it.
You can close this issue if you don't think it can help, I mostly just wanted to mention the possibility.

@kanashimia kanashimia changed the title Investigate using SlotMap instead of Rc<RefCell<_>> Investigate using SlotMap May 31, 2024
@ogoffart
Copy link
Member

ogoffart commented Jun 4, 2024

I don't think this would help for #3215
The model really need to be a Rc because it is shared with user's code.
But this issue isn't really actionable as is so i'm going to close it.
Thanks anyway for the suggestion. And feel free to open other issue if you have more concrete suggestions.

@ogoffart ogoffart closed this as not planned Won't fix, can't repro, duplicate, stale Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs info Further information from the reporter is requested
Projects
None yet
Development

No branches or pull requests

2 participants