From 52aa37a929afb2b86a6ef2a74db5f613d96d1128 Mon Sep 17 00:00:00 2001 From: Wes Date: Mon, 19 Oct 2026 15:09:52 -0700 Subject: [PATCH] perf(mobile): stop an open chat rebuilding on other channels' events (#8456) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary While a chat is open, a message in any other channel rebuilt every built row of that chat. There were two causes: - **The chat watched the whole channel list.** `channelsProvider` republishes the list on every live event in any channel. The chat page, its message list and the thread page watched all of it. They now watch only their own channel's entry. The `#channel` name map comes from a new `channelIdsByNameProvider`, which notifies only when a channel name or id changes. - **The root `App` rebuilt often, and each rebuild rebuilt every pushed route.** It watched the agent observer and the user-status cache only to keep them alive, and it watched the whole Me unread set. It now listens to the first two and selects only whether Me has unread. Rows still remount when a message arrives in the open channel. That comes from `scrollable_positioned_list` keying rows by index, and it gets its own PR. ## Before and after Setup: - iPhone 18 Pro simulator, debug build. - Local relay with about 120 ms of added round-trip time. - Seeded community: 120 channels, about 5.5k messages, a busy chat of about 1.4k messages. - The same scripted run on `main` at `5730c83f7` and on this branch, counted with local probes that are not part of this PR. - Counts are exact. Debug milliseconds are only useful for comparing the two runs. 10 messages arrive in another channel while the busy chat is open: | | `main` | this PR | |---|---|---| | Chat page rebuilds | 42 | 0 | | Message row rebuilds | 1,218 | 0 | | Frames with build time over 16.7 ms | 42 of 129 | 0 of 10 | | Worst frame build time (debug) | 78 ms | 13 ms | 5 messages arrive in the open chat: 1,300 row rebuilds on `main`, 693 on this PR. Most of the rest are the remounts from the index-keyed list. ## Test A widget test publishes a new channel list where only another channel changed, and fails if any `MessageContent` rebuilds. Reverting the production change makes it fail. ## Validation - `just mobile-check` - Full mobile suite: 3,532 passed. - The simulator run above. **Human test:** Wes ran #8456, #8460 and #8462 together on the simulator with his real account and confirmed they work. buzz-review-completed 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Wes Co-authored-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz> Co-authored-by: Claude Opus 5.5 --- mobile/lib/app.dart | 10 +- .../channels/channel_detail_page.dart | 21 ++- .../channel_detail_page/message_list.dart | 18 +- .../channel_ids_by_name_provider.dart | 27 +++ .../features/channels/message_content.dart | 8 +- .../features/channels/thread_detail_page.dart | 20 +-- .../channels/channel_detail_page_test.dart | 156 ++++++++++++++++++ 7 files changed, 220 insertions(+), 40 deletions(-) create mode 100644 mobile/lib/features/channels/channel_ids_by_name_provider.dart diff --git a/mobile/lib/app.dart b/mobile/lib/app.dart index d45f5c5aff..497d6937cc 100644 --- a/mobile/lib/app.dart +++ b/mobile/lib/app.dart @@ -331,16 +331,20 @@ class App extends HookConsumerWidget { if (ageSignalState != AgeSignalState.restricted && authState.value?.status == AuthStatus.authenticated) { ref.watch(relaySessionProvider); - ref.watch(observerRelayProvider); + // Listen, not watch: these publish often, and an App rebuild rebuilds + // every route, including an open chat. + ref.listen(observerRelayProvider, (_, _) {}); ref.watch(appLifecycleProvider); - ref.watch(userStatusCacheProvider); + ref.listen(userStatusCacheProvider, (_, _) {}); if (ref.watch(activeCommunityProvider).value?.pushNotificationsEnabled == true && ref.watch(currentRelayPushDescriptorProvider).value != null) { ref.watch(pushSubscriptionSyncProvider); } hasUnreadInbox = ref.watch(_unreadInboxItemCountProvider) > 0; - hasUnreadSolo = ref.watch(meUnreadChannelIdsProvider).isNotEmpty; + hasUnreadSolo = ref.watch( + meUnreadChannelIdsProvider.select((ids) => ids.isNotEmpty), + ); } // Start listening for buzz:// links immediately (even pre-auth) so a diff --git a/mobile/lib/features/channels/channel_detail_page.dart b/mobile/lib/features/channels/channel_detail_page.dart index 6d3a2487d7..be320dc0c5 100644 --- a/mobile/lib/features/channels/channel_detail_page.dart +++ b/mobile/lib/features/channels/channel_detail_page.dart @@ -58,6 +58,7 @@ import 'channel_messages_provider.dart'; import 'channel_typing_provider.dart'; import 'channel_typing_indicator.dart'; import 'inline_agent_typing.dart'; +import 'channel_ids_by_name_provider.dart'; import 'channels_provider.dart'; import 'unread_badge/observed_unread_event.dart'; import 'compose_bar.dart'; @@ -307,7 +308,14 @@ class _ResolvedChannelDetailPage extends HookConsumerWidget { ); final mePlacement = personalChat ? ref.watch(meGroupsProvider) : null; final detailsAsync = ref.watch(channelDetailsProvider(channel.id)); - final channelsAsync = ref.watch(channelsProvider); + // Only this channel's entry: the list republishes on every live event. + final listedChannel = ref.watch( + channelsProvider.select( + (channels) => channels.value + ?.where((candidate) => candidate.id == channel.id) + .firstOrNull, + ), + ); final huddleLifecycle = ref.watch(huddleLifecycleProvider(channel.id)).value ?? const []; final sessionStatus = ref.watch(relaySessionProvider).status; @@ -365,16 +373,7 @@ class _ResolvedChannelDetailPage extends HookConsumerWidget { e.pubkey.toLowerCase() != currentPubkey.toLowerCase(), ) .toList(); - final baseChannel = - channelsAsync - .whenData( - (channels) => channels.firstWhere( - (candidate) => candidate.id == channel.id, - orElse: () => channel, - ), - ) - .value ?? - channel; + final baseChannel = listedChannel ?? channel; final resolvedChannel = detailsAsync.whenData(baseChannel.mergeDetails).value ?? baseChannel; final participantCount = resolvedChannel.participantPubkeys diff --git a/mobile/lib/features/channels/channel_detail_page/message_list.dart b/mobile/lib/features/channels/channel_detail_page/message_list.dart index f3cfb1d10e..35e903da52 100644 --- a/mobile/lib/features/channels/channel_detail_page/message_list.dart +++ b/mobile/lib/features/channels/channel_detail_page/message_list.dart @@ -91,6 +91,10 @@ class _MessageList extends HookConsumerWidget { final oldestUnreadMessageId = useState(null); final unreadBoundaryLoadFailed = useState(false); final unreadBoundaryFetchCount = useRef(0); + // Reaching the oldest page publishes no messages, so nothing else + // rebuilds this list to re-run the unread effect below. A count, not a + // flag: a reconnect can reset the notifier and reach the oldest page again. + final unreadBoundaryReachedOldestCount = useState(0); final hasUnreadDeepLink = initialMessageId != null || initialThreadRootId != null; final dayTimestampByReversedIndex = {}; @@ -172,7 +176,10 @@ class _MessageList extends HookConsumerWidget { unawaited( Future(() async { final loaded = await notifier.fetchOlder(); - if (!cancelled && !loaded && !notifier.reachedOldest) { + if (cancelled || loaded) return; + if (notifier.reachedOldest) { + unreadBoundaryReachedOldestCount.value += 1; + } else { unreadBoundaryLoadFailed.value = true; } }), @@ -810,14 +817,7 @@ class _MessageList extends HookConsumerWidget { } if (entries.isEmpty) return const _EmptyMessages(); - // Build channel names map once for all message bubbles. - final channelsAsync = ref.watch(channelsProvider); - final channelNamesMap = {}; - channelsAsync.whenData((channels) { - for (final ch in channels) { - channelNamesMap[ch.name.toLowerCase()] = ch.id; - } - }); + final channelNamesMap = ref.watch(channelIdsByNameProvider); return Stack( children: [ diff --git a/mobile/lib/features/channels/channel_ids_by_name_provider.dart b/mobile/lib/features/channels/channel_ids_by_name_provider.dart new file mode 100644 index 0000000000..6370a762be --- /dev/null +++ b/mobile/lib/features/channels/channel_ids_by_name_provider.dart @@ -0,0 +1,27 @@ +import 'package:flutter/foundation.dart'; +import 'package:hooks_riverpod/hooks_riverpod.dart'; + +import 'channels_provider.dart'; + +/// Lowercased channel name to channel id, for `#channel` links. +/// +/// `channelsProvider` publishes a new list on every live event, so watching it +/// rebuilds every message row. This notifies only when a name or id changes. +final channelIdsByNameProvider = + NotifierProvider>( + ChannelIdsByName.new, + ); + +class ChannelIdsByName extends Notifier> { + @override + Map build() => Map.unmodifiable({ + for (final channel in ref.watch(channelsProvider).value ?? const []) + channel.name.toLowerCase(): channel.id, + }); + + @override + bool updateShouldNotify( + Map previous, + Map next, + ) => !mapEquals(previous, next); +} diff --git a/mobile/lib/features/channels/message_content.dart b/mobile/lib/features/channels/message_content.dart index daf0b42566..a813a104e0 100644 --- a/mobile/lib/features/channels/message_content.dart +++ b/mobile/lib/features/channels/message_content.dart @@ -30,7 +30,7 @@ import '../../shared/custom_emoji/custom_emoji_provider.dart'; import '../../shared/custom_emoji/custom_emoji_render.dart'; import '../../shared/emoji/emoji_data_provider.dart'; import '../../shared/emoji/emoji_only.dart'; -import 'channels_provider.dart'; +import 'channel_ids_by_name_provider.dart'; import 'local_send_bubble_surface.dart'; import 'media_viewer_page.dart'; import 'message_content/link_normalizer.dart'; @@ -224,11 +224,7 @@ class MessageContent extends HookConsumerWidget { }; final resolvedChannelNames = channelNames.isNotEmpty ? channelNames - : { - for (final channel - in ref.watch(channelsProvider).asData?.value ?? const []) - channel.name.toLowerCase(): channel.id, - }; + : ref.watch(channelIdsByNameProvider); final channelHandler = useRef(onChannelTap)..value = onChannelTap; final resolvedChannelTap = useMemoized( () => (String channelId) { diff --git a/mobile/lib/features/channels/thread_detail_page.dart b/mobile/lib/features/channels/thread_detail_page.dart index 3bb517008f..d0c6de7e7b 100644 --- a/mobile/lib/features/channels/thread_detail_page.dart +++ b/mobile/lib/features/channels/thread_detail_page.dart @@ -36,6 +36,7 @@ import 'channel_messages_provider.dart'; import 'channel_typing_provider.dart'; import 'channel_typing_indicator.dart'; import 'thread_replies_provider.dart'; +import 'channel_ids_by_name_provider.dart'; import 'channels_provider.dart'; import 'compose_bar.dart'; import 'composer_dock_size_reporter.dart'; @@ -899,17 +900,14 @@ class ThreadDetailPage extends HookConsumerWidget { return null; }, [settledImeBottomInset.value]); - // Channel names for message content rendering. - final channelsAsync = ref.watch(channelsProvider); - final channel = channelsAsync.value - ?.where((candidate) => candidate.id == channelId) - .firstOrNull; - final channelNamesMap = {}; - channelsAsync.whenData((channels) { - for (final ch in channels) { - channelNamesMap[ch.name.toLowerCase()] = ch.id; - } - }); + final channel = ref.watch( + channelsProvider.select( + (channels) => channels.value + ?.where((candidate) => candidate.id == channelId) + .firstOrNull, + ), + ); + final channelNamesMap = ref.watch(channelIdsByNameProvider); return FrostedScaffold( foreground: threadViewportVisible diff --git a/mobile/test/features/channels/channel_detail_page_test.dart b/mobile/test/features/channels/channel_detail_page_test.dart index 50bacdb440..1f9ca1e029 100644 --- a/mobile/test/features/channels/channel_detail_page_test.dart +++ b/mobile/test/features/channels/channel_detail_page_test.dart @@ -3639,6 +3639,55 @@ void main() { ); }); + testWidgets('activity in another channel does not rebuild the open chat', ( + tester, + ) async { + final otherChannel = Channel( + id: _otherChannelId, + name: 'random', + channelType: 'stream', + visibility: 'open', + description: '', + createdBy: 'abc123', + createdAt: DateTime(2025), + memberCount: 5, + isMember: true, + ); + final channelsNotifier = _FakeChannelsNotifier([ + _testChannel, + otherChannel, + ]); + await tester.pumpWidget( + _buildTestable( + messages: [ + for (var i = 0; i < 5; i++) + _textMsg( + id: 'msg$i', + pubkey: 'alice', + content: 'Message $i, see #random', + createdAt: 1000 + i, + ), + ], + channelsNotifier: channelsNotifier, + ), + ); + await tester.pumpAndSettle(); + expect(find.byType(MessageContent), findsWidgets); + + final rebuilt = []; + debugOnRebuildDirtyWidget = (element, _) => rebuilt.add(element.widget); + addTearDown(() => debugOnRebuildDirtyWidget = null); + + // A live event elsewhere republishes the whole list. + channelsNotifier.setChannels([ + _testChannel, + otherChannel.copyWith(lastMessageAt: DateTime.utc(2026)), + ]); + await tester.pumpAndSettle(); + + expect(rebuilt.whereType(), isEmpty); + }); + testWidgets('clears the composer inset when membership is revoked', ( tester, ) async { @@ -4978,6 +5027,80 @@ void main() { expect(findRichText('Reachable unread'), findsOneWidget); }); + testWidgets( + 'settles the unread jump each time history silently reaches its oldest page', + (tester) async { + final readMessage = _textMsg( + id: 'read', + pubkey: 'alice', + content: 'Already read', + createdAt: 1010, + ); + final messagesNotifier = _SilentOldestMessagesNotifier([readMessage]); + final channelsNotifier = _FakeChannelsNotifier( + [_testChannel], + observedUnread: { + _channelId: [ + for (final (id, createdAt) in [ + ('never-loaded', 1021), + ('reachable-unread', 1022), + ]) + makeObservedUnreadEvent( + id: id, + createdAt: createdAt, + rootId: null, + highPriority: false, + channelType: 'stream', + isThreadedReply: false, + ), + ], + }, + ); + final readState = _SynchronousReadStateNotifier( + const ReadStateState( + isReady: true, + pubkey: 'self', + contexts: {_channelId: 1020}, + version: 0, + ), + ); + final unreadButton = find.byKey( + const ValueKey('channel-jump-to-oldest-unread'), + ); + + await tester.pumpWidget( + _buildTestable( + messages: const [], + messagesNotifier: messagesNotifier, + channelsNotifier: channelsNotifier, + readStateNotifier: readState, + users: const { + 'alice': UserProfile(pubkey: 'alice', displayName: 'Alice'), + }, + ), + ); + await _settleThenFinishFetch(tester); + // The first fetch reached the oldest page with no unread row loaded. + expect(messagesNotifier.fetchOlderCalls, 1); + expect(unreadButton, findsNothing); + + // A reconnect forgets the oldest page and reloads the window, now + // with an unread row; the next fetch reaches the oldest page again. + messagesNotifier.reconnect([ + readMessage, + _textMsg( + id: 'reachable-unread', + pubkey: 'alice', + content: 'Reachable unread', + createdAt: 1022, + ), + ]); + await _settleThenFinishFetch(tester); + expect(messagesNotifier.fetchOlderCalls, 2); + expect(unreadButton, findsOneWidget); + }, + ); + testWidgets('pages past a loaded forced unread for an older target', ( tester, ) async { @@ -16773,6 +16896,39 @@ class _FakeMessagesNotifier extends ChannelMessagesNotifier { } } +/// Lets the rows settle while an older fetch is in flight, then lets it finish, +/// so nothing but the fetch's own completion can rebuild the list afterwards. +Future _settleThenFinishFetch(WidgetTester tester) async { + await tester.pumpAndSettle(); + await tester.pump(_SilentOldestMessagesNotifier.fetchDelay); + await tester.pumpAndSettle(); +} + +/// Each older fetch takes a while, then finds the oldest page without +/// publishing anything, like a window whose cursor has run out. +class _SilentOldestMessagesNotifier extends _FakeMessagesNotifier { + _SilentOldestMessagesNotifier(super.messages); + + static const fetchDelay = Duration(seconds: 5); + bool _reachedOldest = false; + + @override + bool get reachedOldest => _reachedOldest; + + @override + Future fetchOlder() async { + fetchOlderCalls += 1; + await Future.delayed(fetchDelay); + _reachedOldest = true; + return false; + } + + void reconnect(List messages) { + _reachedOldest = false; + setMessages(messages); + } +} + /// Counts loads; each settles after mount, like the real network load. class _RetryCountingMessagesNotifier extends ChannelMessagesNotifier { _RetryCountingMessagesNotifier(this.nextResult) : super(_channelId);