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

Don't send TimelineUpdates to rooms that are not currently open. Handle and consolidate pending TimelineUpdates on the background async task. #292

Open
kevinaboos opened this issue Dec 18, 2024 · 0 comments
Assignees

Comments

@kevinaboos
Copy link
Member

Currently, whenever we discover a new room in the sync loop, we eagerly spawn a timeline_subscriber_handler for that room, which listens for timeline changes and then sends TimelineUpdates back to the RoomScreen widget (on the main UI thread) for that room.

This is technically fine, but it might become very inefficient for user accounts that have thousands of rooms. If that user only opens 5-10 rooms, then we wouldn't want to continuously watch for updates for all of those rooms and enqueue TimelineUpdates to those rooms that will never be opened. This will cause the queues of TimelineUpdates for those unopened rooms to grow endlessly until that room is actually opened by the user, upon which point there may be thousands of pending TimelineUpdates that it must process first before being able to display anything.

There are two solutions for this; we can do just one or both as they are complimentary to one another.

  1. Continue processing updates in the timeline_subscriber_handler, but don't actually send TimelineUpdates to the main UI thread if that room isn't currently open.
    • Instead, while the room has not yet been opened, we can pop off pending updates from the queue and process the diffs locally (in the async background task) in order to merge many pending updates into just one. We can then push that one merged update onto the queue, which will make it a lot faster for the first time that the room gets opened.
    • We can use the existence of the TimelineUpdate receiver in that room's RoomInfo struct (specifically the timeline_singleton_endpoints) to determine if the room is currently open. This would require us to restore the update_receiver and request_sender objects in the TimelineUiState struct back into the room's RoomInfo struct in the ALL_ROOM_INFO map when hiding a timeline (in hide_timeline()).
  2. Don't eagerly spawn timeline_subscriber_handler tasks for every new room that gets discovered.
    • Instead, only spawn new timeline subscriber loops when the new room is actually opened by the user.
    • We could also close/stop the timeline subscriber loop when the user closes the room, but this would require a bit more of a code restructuring. Plus, I'm not sure how desirable this is, since a user will still likely want to receive updates about a room they just closed (unless they left it).
@kevinaboos kevinaboos self-assigned this Dec 18, 2024
@github-project-automation github-project-automation bot moved this to Ready in Robrix Dec 18, 2024
@kevinaboos kevinaboos changed the title Don't send TimelineUpdates to rooms that are not currently open. Handle and consolidate the updates on the background async task. Don't send TimelineUpdates to rooms that are not currently open. Handle and consolidate pending TimelineUpdates on the background async task. Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Ready
Development

No branches or pull requests

1 participant