Conversation
- Move MediaCache to a global thread-local storage. - Update MediaCacheEntry to support multiple subscribers for pending requests. - Remove per-room MediaCache from TimelineUiState. - Update RoomScreen, LinkPreview, and RoomImageViewer to use the global cache. - Reduce memory usage and network requests for shared media. Co-authored-by: kevinaboos <1139460+kevinaboos@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
- Remove stale `media_cache` initialization in `TimelineUiState`. - Update `populate_matrix_image_modal` call to pass `update_sender`. - Update `MediaCacheEntry::Requested` match pattern to handle tuple variant. - Import `MediaCacheEntry` where needed or ensure accessibility. Co-authored-by: kevinaboos <1139460+kevinaboos@users.noreply.github.com>
- Fix usage of `media_cache` which was removed from `TimelineUiState`. - Initialize `update_sender` in `TimelineUiState` constructor. - Fix logic in `handle_image_click` to correctly pass `update_sender` to `populate_matrix_image_modal`. - Fix `MediaCacheEntry` pattern matching. Co-authored-by: kevinaboos <1139460+kevinaboos@users.noreply.github.com>
- Reverted incorrect change in `populate_image_message_content` that was trying to call `populate_matrix_image_modal` with an undefined variable. - Restored `fetch_and_show_media_source` call which is the correct logic for the timeline view. - Ensured `handle_image_click` correctly opens the modal using the available `update_sender`. Co-authored-by: kevinaboos <1139460+kevinaboos@users.noreply.github.com>
|
Doesn't this also make it way slower to query the cache for a given specific timeline, as the cache will be HUGE now with millions of entries? You're assuming that images are shared across multiple rooms, which is actually quite unlikely and rare in practice. |
While the cache will contain more entries, Regarding memory usage: effectively, the previous implementation also kept all media in memory indefinitely, because Moving to a global cache:
|
⚡ Bolt: Global Media Cache
💡 What:
Moved
MediaCachefrom being an instance perRoomScreen(inTimelineUiState) to a global thread-localGLOBAL_MEDIA_CACHE. Updated the cache logic to support multiple subscribers (senders) for a single media request, ensuring that all timelines waiting for a specific media item are notified when it loads.🎯 Why:
Previously, if the same image (e.g., a custom sticker or a shared meme) was displayed in multiple rooms, it would be fetched and cached separately for each room. This wasted memory and bandwidth. Additionally, removing
MediaCachefromTimelineUiStatereduces the size of the state struct.📊 Impact:
🔬 Measurement:
Code analysis confirms that
MediaCacheis now a singleton (per thread) andtry_get_media_or_fetchhandles multiple subscribers. Verification was done via manual code review of the refactoring due to environment limitations on running the full build/test suite.PR created automatically by Jules for task 7471736247335357365 started by @kevinaboos