fix(timeline): diff-based UI updates to address Android freeze#131
Draft
TigerInYourDream wants to merge 3 commits into
Draft
fix(timeline): diff-based UI updates to address Android freeze#131TigerInYourDream wants to merge 3 commits into
TigerInYourDream wants to merge 3 commits into
Conversation
…work Switch TimelineUpdate from NewItems(full snapshot) to ItemsChanged(diff batch) so the background subscriber stops cloning the entire timeline on every incremental SDK diff. Introduce TimelineDiff<T> covering all 11 SDK VectorDiff variants and apply diffs to TimelineUiState.items in place on the UI thread. Cap timeline updates per Event::Signal at MAX_TIMELINE_UPDATES_PER_SIGNAL and re-trigger SignalToUI when the receiver still has pending work, so the Makepad event loop is not blocked by an unbounded try_recv drain. Narrow bot discovery to changed_indices via a timeline_update_scan_range helper instead of scanning the full timeline. TimelineUpdate::FirstUpdate remains the only full-snapshot path. Addresses Android timeline freeze caused by UI-thread amplification of small SDK diffs into full-snapshot work. See specs/task-android-timeline-freeze.spec.md for the task contract.
clippy::field_reassign_with_default suggested initializing AppState in a single struct literal with ..Default::default() instead of mutating the default value field-by-field. Apply this in the two test sites that hit the lint; the result is functionally identical and removes the remaining warnings reported by `cargo clippy --lib --tests`.
Capture the root cause as Robrix UI-thread amplification of small SDK diffs (RoomScreen::process_timeline_updates draining the receiver in an unbounded try_recv loop, sliding_sync::timeline_subscriber_handler cloning the full timeline on every incremental update) and record the decisions implemented in this branch: - bound timeline updates per Event::Signal via MAX_TIMELINE_UPDATES_PER_SIGNAL and resume via SignalToUI - replace TimelineUpdate::NewItems with ItemsChanged carrying a Robrix-owned TimelineDiff covering the SDK VectorDiff operations - limit bot discovery to changed_indices via timeline_update_scan_range - keep TimelineUpdate::FirstUpdate as the only full-snapshot path Includes BDD acceptance scenarios for the four focused unit tests and the manual Android verification path. Inherits specs/project.spec.md.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
TimelineUpdate::ItemsChanged { item_diffs, .. }instead ofNewItems { new_items: timeline_items.clone() }.TimelineUiState.itemsin place on the UI thread via a newTimelineDiff<T>covering all 11 SDKVectorDiffvariants.TimelineUpdate::FirstUpdateremains the only full-snapshot path.Event::SignalatMAX_TIMELINE_UPDATES_PER_SIGNAL = 8and re-triggerSignalToUIfor the remainder, so the main thread does not infinitely drain the receiver.changed_indicesvia the newtimeline_update_scan_rangehelper instead of scanning the full timeline.clippy::field_reassign_with_defaultwarnings in test fixtures socargo clippy --lib --testsis now warning-free.Why
On Android the timeline page can stall or hit ANR after the app has been running for a while. Root cause is Robrix UI-thread amplification, not Makepad: every incremental SDK timeline diff was being cloned into a full timeline snapshot and
RoomScreen::process_timeline_updateswas draining the channel without any per-signal upper bound. Both pressures land on the main thread.Test plan
Automated (all green):
cargo checkcargo clippy --lib --tests— 0 warningscargo test timeline_update_batch_yields_after_budgetcargo test timeline_update_scan_range_limits_incremental_workcargo test timeline_items_apply_append_and_set_diffscargo test timeline_items_apply_remove_truncate_clear_reset_diffsManual (pending — this is why the PR is draft):
process_timeline_updatesfor multiple seconds, touch input still responsive during the burstSpec
specs/task-android-timeline-freeze.spec.md— full task contract with BDD acceptance scenarios.Out of scope
iOS IME,
PortalListinternals, room-list startup pagination, media/avatar cache redesign. See spec.