Skip to content

perf: fix ChatListSortRooms redundant rebuilds and lifecycle leaks#2932

Open
9clg6 wants to merge 1 commit intomainfrom
perf/chat-list-sort-fix
Open

perf: fix ChatListSortRooms redundant rebuilds and lifecycle leaks#2932
9clg6 wants to merge 1 commit intomainfrom
perf/chat-list-sort-fix

Conversation

@9clg6
Copy link
Collaborator

@9clg6 9clg6 commented Mar 17, 2026

Summary

Fix the chat list sort widget that was re-running an expensive async sort on every frame rebuild, and fix pre-existing lifecycle bugs causing memory leaks.

Problems

  1. Redundant sort on every build(): FutureBuilder(future: sortRooms()) was called directly in build(), creating a new Future on every rebuild. Since a parent StreamBuilder triggers rebuilds ~1x/second (rate-limited sync), this launched a full async sort of all rooms every second.

  2. Memory leak (pre-existing): No dispose() method — StreamSubscription instances (one per room, listening to room.onUpdate) were never cancelled when the widget was destroyed.

  3. Crash risk (pre-existing): sortRooms() accesses context and widget after multiple await calls without checking mounted. If the widget is disposed mid-sort, this throws "Looking up a deactivated widget's ancestor".

  4. Concurrent stale futures (pre-existing): Each didUpdateWidget call launched a new sort without invalidating the previous one. Multiple sort futures could run in parallel, mutating shared state and writing to the parent's ValueNotifier.

Solution

  • Cache the sort Future in _sortFuture, computed only in initState() and didUpdateWidget() — not in build()
  • Add dispose() to cancel all room subscriptions
  • Add mounted guards after every await in _sortRooms()
  • Add a _sortGeneration counter: each new sort increments it, and in-flight sorts bail out early if their generation is stale

Results

Metrics collected via Chrome DevTools performance trace with CPU 4x throttling (simulating a low-end device). The raw flame chart JSON exceeded 350,000 lines — analysis was automated using the Chrome DevTools MCP on Claude Code to run the app, navigate through conversations, record performance traces, and extract metrics programmatically.

Metric Before After Delta
Long tasks (>50ms) count 33 23 -30%
Long tasks total time 4,145ms 3,521ms -16.9%
Jank (frames >16.7ms) 90.3% 84.8% -5.5 pts
JS Heap 79 MB 68 MB -14%

Files changed

File Change
lib/pages/chat_list/chat_list_sort_rooms.dart Fix rebuild + add dispose/mounted/generation

Summary by CodeRabbit

  • Bug Fixes

    • Improved chat room list sorting to handle edge cases when widget state changes, ensuring sorting results remain up-to-date and responsive.
  • Chores

    • Removed unused import.

@coderabbitai
Copy link

coderabbitai bot commented Mar 17, 2026

Warning

Rate limit exceeded

@9clg6 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 14 minutes and 18 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7218792d-f551-4afa-b495-6f0a5c8b2e9e

📥 Commits

Reviewing files that changed from the base of the PR and between 046d017 and 7cb83ee.

📒 Files selected for processing (1)
  • lib/pages/chat_list/chat_list_sort_rooms.dart

Walkthrough

This pull request introduces a generation-invalidated sorting pipeline for room list ordering in the chat interface. The sorting logic is refactored to use futures with stale result detection, allowing the system to discard in-flight sort operations when invalidated. Early exit conditions are added to handle widget unmounting and newer sort generation detection. A disposal method cancels subscriptions and increments the generation counter. Additionally, an unused import is removed from a utility file.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The PR title 'perf: fix ChatListSortRooms redundant rebuilds and lifecycle leaks' clearly and specifically describes the main changes: addressing performance issues from redundant rebuilds and fixing lifecycle/memory leak bugs in the ChatListSortRooms widget.
Description check ✅ Passed The PR description is comprehensive and well-structured. It covers the root causes (redundant rebuilds, memory leaks, crash risk, concurrent futures), the solution (caching, dispose, mounted guards, generation counter), and includes detailed performance metrics demonstrating the impact.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch perf/chat-list-sort-fix
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Contributor

This PR has been deployed to https://linagora.github.io/twake-on-matrix/2932

@9clg6 9clg6 force-pushed the perf/chat-list-sort-fix branch from 1158615 to ba164a4 Compare March 17, 2026 17:20
Copy link
Collaborator

@tddang-linagora tddang-linagora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Add before after screenshots of the improvement (Either Flutter devtools or Chrome devtools)

@9clg6 9clg6 changed the base branch from refacto/performance to main March 20, 2026 10:59
@9clg6 9clg6 force-pushed the perf/chat-list-sort-fix branch from ba164a4 to 046d017 Compare March 20, 2026 11:02
@9clg6 9clg6 force-pushed the perf/chat-list-sort-fix branch from bee06f0 to 4ea663d Compare March 20, 2026 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants