-
Notifications
You must be signed in to change notification settings - Fork 13k
fix: high cpu usage with large amount of channels #37395
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
Conversation
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: 5e22651 The changes in this PR will be included in the next version bump. This PR includes changesets to release 41 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughA changelog entry documents a patch fix for "@rocket.chat/meteor" addressing client slowdown for users with many channels. The useUnread hook is refactored to use per-subscription state snapshots and selective emission, replacing reduce-based aggregation with explicit loops for performance optimization. Changes
Sequence Diagram(s)sequenceDiagram
participant subscription as Subscriptions
participant hook as useUnread Hook
participant state as Per-Subscription State
loop For each subscription
subscription->>hook: subscription data change
hook->>state: check unread/alert/unreadAlert fields
alt Field changed?
state->>hook: emit unread-changed-by-subscription
else No change
state->>hook: skip emission
end
end
hook->>hook: explicit loop: accumulate unreadCount
hook->>hook: compute global badgeIndicator
hook->>hook: cap unreadCount to 999+
hook->>hook: set unread state (count or badge)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Areas for attention:
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #37395 +/- ##
===========================================
- Coverage 68.10% 68.09% -0.02%
===========================================
Files 3364 3364
Lines 115755 115760 +5
Branches 20910 20909 -1
===========================================
- Hits 78838 78828 -10
- Misses 34220 34234 +14
- Partials 2697 2698 +1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.changeset/quiet-cars-smile.md(1 hunks)apps/meteor/client/views/root/hooks/loggedIn/useUnread.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37357
File: apps/meteor/ee/server/startup/federation.ts:39-74
Timestamp: 2025-11-05T20:53:57.761Z
Learning: In Rocket.Chat (apps/meteor/app/settings/server/CachedSettings.ts), the settings.watchMultiple() method immediately invokes its callback with current values if all requested settings exist in the store, then continues watching for subsequent changes. It does not wait for a setting to change before the first invocation.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (6)
.changeset/quiet-cars-smile.md (1)
1-5: LGTM!The changelog entry clearly documents the patch fix for client slowdown with large channel counts.
apps/meteor/client/views/root/hooks/loggedIn/useUnread.ts (5)
3-3: LGTM!The addition of
useRefand theUnreadDatatype alias properly support the new per-subscription snapshot mechanism.Also applies to: 11-12
23-27: Excellent documentation and optimization strategy.The comment clearly explains the performance problem (O(N) scaling with subscription count) and the solution (selective event emission). Using a Map for O(1) lookups is the right choice.
29-32: LGTM!Variables are properly typed and initialized. The
nextSnapshotpattern enables atomic state updates.
53-61: LGTM!The snapshot swap enables efficient per-subscription change detection, and the unread state prioritization logic (count → badge → clear) is correct. The 999+ cap is appropriate for UI display.
64-64: LGTM!The dependency array is correct. All listed dependencies are necessary for the effect, and removing
unread(which is the output, not an input) prevents unnecessary re-renders.
|
/patch |
|
/backport 7.11.2 |
|
/backport 7.10.5 |
|
Pull request #37520 added to Project: "Patch 7.12.2" |
|
Pull request #37521 added to Project: "Patch 7.11.2" |
|
Pull request #37522 added to Project: "Patch 7.10.5" |
Root Cause
The
useUnreadhook.Explanation
useEffectdependency causes it to run multiple times.Proposed changes (including videos or screenshots)
This PR addresses the performance issues of the
useUnreadhook.Explanation
reducewith a regular loop to cut closure overhead.useEffectdependency (unread) which caused multiple runs was removed.Issue(s)
Closes #36643
Closes #37331
Closes SUP-862
Steps to test or reproduce
Test Scenario
Profiling
To create a bunch of channels, I ran a playwright script similar to this (rate limit must be disabled):
I had also changed the createPublicChannel helper to add another user.
Further comments
I'm omitting the test for now since we want to backport this, but I may add them in another PR:
https://gist.github.com/cardoso/7f4cfc461c9bbd7a519cadbd0632490f
A few of these will fail without these optimizations and all will pass with the optimizations.
Summary by CodeRabbit