search

project-robius / robrix

A multi-platform Matrix chat client written in pure Rust using the Makepad UI toolkit and the Robius app dev framework
MIT License
88 stars 16 forks source link

Restructure `RoomsList` to store the list of `all_rooms` as a HashMap keyed by room ID #185

Open kevinaboos opened 1 week ago

kevinaboos commented 1 week ago

The full set/list of all rooms should not be ordered by default, which is what's currently happening implicitly because RoomsList::all_rooms is a Vec.

This is also especially relevant since we have some pending PRs about ordering/sorting/filtering the rooms list, so basically we're already encountering the need to separate the concern of storing the list from the concern of displaying the ordered/filtered list.

That would remove the need for the newly-added additional field RoomScreen::rooms_list_owned_room_id_map too.

_Originally posted by @kevinaboos in https://github.com/project-robius/robrix/pull/181#discussion_r1796231692_

Demolemon11 commented 6 days ago

I am looking at this : )

Demolemon11 commented 6 days ago

Our intention is to change Vec<RoomPreviewEntry> to HashMap<OwnedRoomID, RoomPreviewEntry>, right?

Demolemon11 commented 6 days ago

Btw, I think if we want to use HashMap instead of normal tuple, we should avoid the standard library HashMap, its performance is very poor (not as good as Java), let me see what high-performance HashMap is available on crates.io : )

Demolemon11 commented 6 days ago

I am a little pulzz about this, does the OwnedRoomID under RoomPerviewEntry need to be removed?

kevinaboos commented 6 days ago

This has already been self-assigned to me, but sure, you can take a look at it.

Our intention is to change Vec<RoomPreviewEntry> to HashMap<OwnedRoomID, RoomPreviewEntry>, right?

yep. I wouldn't be concerned about performance, even the largest matrix accounts have like ~2000 rooms. Also, don't get swept up in premature optimization. There are other hashmaps, e.g., hashbrown, but std is fine for now.

(its performance is very poor (not as good as Java)

I sincerely doubt that, but it's not important right now.

I am a little pulzz about this, does the OwnedRoomID under RoomPerviewEntry need to be removed?

nope, why? we'd still want easy access to a room's ID from its RoomPreviewEntry. Again, this issue is specifically about changing the choice of data structure to store the list of rooms, so let's restrict the scope to that.

Demolemon11 commented 6 days ago

I see. Thanks very much: )

Demolemon11 commented 5 days ago

Then, There is no Vec, and the index is not avilibale in HashMap. So, Should we change rooms_list_map from HashMap<u64, usize> to HashMap<OwnedRoomID, u64> ? (u64 is Widget ID) :)

Demolemon11 commented 5 days ago

One last question, why my pr cannot checks automatically?, thanks :)