robrix icon indicating copy to clipboard operation
robrix copied to clipboard

Fix @mention search performance and refactor search functionality

Open ZhangHanDong opened this issue 3 months ago • 4 comments

Fixed issue #506

This is a remastered version of the PR #550 .

  • Add a lightweight cpu_worker thread that stays alive for the app lifetime and runs CPU-bound jobs (currently the room-member search); App::handle_startup initializes it and widgets enqueue work via cpu_worker::spawn_cpu_job.
  • Rework MentionableTextInput into an explicit state machine with search IDs, cancellation tokens, loading/empty UI states, and streaming updates that arrive over the background worker channel.
  • Replace the old MatrixRequest::SearchRoomMembers path: sliding_sync now fetches joined-member lists directly, emits RoomMembersListFetched, and RoomScreen caches those members plus precomputed sort data for downstream widgets.
  • Refactor room/member_search.rs to support batched streaming, cancellation, richer match heuristics, optional precomputed sort reuse, and add unit tests for the new helpers.

There are three related issues that still need to be fixed.

ZhangHanDong avatar Oct 22 '25 03:10 ZhangHanDong

@alanpoon & @tyreseluo, can you please give this a review before I dig in? Thanks.

kevinaboos avatar Oct 22 '25 20:10 kevinaboos

@alanpoon & @tyreseluo, can you please give this a review before I dig in? Thanks.

yep, i will.

tyreseluo avatar Oct 23 '25 04:10 tyreseluo

Review suggestions: • Select a large room (thousands or tens of thousands of members), @ a user, and search. • Continue testing in rooms with more than 50 members. • Continue testing in rooms with fewer than 50 members. • Close large/small rooms, reopen them, and continue testing. • Logout, then login again, and rerun the above four steps.

ZhangHanDong avatar Oct 28 '25 04:10 ZhangHanDong

@kevinaboos

Current PR progress:

  1. Fix the issue where remote sync of members does not work after logout-login - Previously, after logging out and then logging back in, remote synchronization of room members failed to trigger properly - The relevant state reset logic has been corrected
  2. Remove the behavior of automatically deleting the local members cache after closing a tab - This behavior is considered unnecessary and harms the user experience - In the future, a setting can be added to manually clear local cached data
  3. Optimize the member list loading experience - Automatically check local cached members; if a cache exists, render the user list immediately without waiting for remote data - Each room performs remote synchronization only once per application session - During synchronization, show a loading animation at the bottom of the user list; it will disappear automatically after sync completes
  4. Fix the issue where the cursor disappears after clicking to select a mention user with the mouse - Previously, after clicking to select a user in the mention popup, the cursor would blink and then disappear, making it impossible to continue typing - Resolved by retrying the focus restoration mechanism in draw_walk
  5. Fix duplicate room tabs after app restart or logout-login - Previously, restoring rooms could create duplicate tabs - Avoided by looking up existing tabs based on room_id

Remote sync room_members trigger conditions:

Remote member synchronization (SyncRoomMemberList) is triggered in the following cases:

  • First time opening a room after the application starts
  • Login again after Logout
  • Switching to a new room that has never been loaded

Note: Within the same application session, closing a room tab and reopening it will not trigger remote synchronization again.


The current code still needs another review; functional testing can proceed first.

ZhangHanDong avatar Nov 26 '25 05:11 ZhangHanDong

Show Video

Summary

This PR comprehensively refactors the @mention member search functionality to address performance issues in large rooms (#506).

Key Architectural Changes

  1. CPU Worker Module - A lightweight background thread handles CPU-bound member search operations, keeping the UI responsive during searches in large rooms.

  2. State Machine Design - MentionableTextInput now uses an explicit state machine (MentionSearchState) with four states: Idle, WaitingForMembers, Searching, and JustCancelled. This replaces multiple boolean flags and reduces race conditions.

  3. Streaming Search Results - Results are sent via MPSC channel in batches of 10, enabling progressive UI updates. Each search has a unique ID and cancellation token support.

  4. Precomputed Sort Data - PrecomputedMemberSort caches sort keys and indices, making empty searches O(1) and avoiding repeated computation.

  5. Local-First Rendering - Member lists render immediately from local cache while remote sync continues in background.

Bug Fixes

  • @room in DMs: Block @room mentions in direct message rooms
  • Duplicate Tabs: Fix duplicate room tabs on restore by using find_open_room_live_id() lookup
  • Focus Loss: Restore text input focus after mention selection via pending_draw_focus_restore
  • Logout/Login: Clear Matrix SDK database on logout to prevent stale data

img_v3_02sc_3c9cb061-35a3-495f-8b60-0a6635cb29ag Duplicate Tabs

Security Fix

  • Add validate_path_within_app_data() to prevent arbitrary directory deletion when clearing app state.

The database path from session files is now validated to be strictly within the app data directory before deletion.

Documentation

  • Added comprehensive module documentation with ASCII diagrams for state machine and background thread flow
  • Documented duplicate tab prevention mechanism
  • Documented database cleanup rationale and security measures

Test Plan

  • [ ] Verify @mention search is responsive in large rooms (1000+ members)
  • [ ] Verify @room option does not appear in direct messages
  • [ ] Verify no duplicate tabs when clicking already-open rooms
  • [ ] Verify focus remains in text input after selecting a mention
  • [ ] Verify logout and re-login shows fresh data without stale cache
  • [ ] Verify ESC cancels mention search and prevents immediate re-trigger

ZhangHanDong avatar Dec 11 '25 18:12 ZhangHanDong