mirror of
https://github.com/block/buzz.git
synced 2026-10-27 12:40:30 +00:00
perf(mobile): stop an open chat rebuilding on other channels' events (#8456)
## 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 <wesbillman@users.noreply.github.com> Co-authored-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -91,6 +91,10 @@ class _MessageList extends HookConsumerWidget {
|
||||
final oldestUnreadMessageId = useState<String?>(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 = <int, int>{};
|
||||
@@ -172,7 +176,10 @@ class _MessageList extends HookConsumerWidget {
|
||||
unawaited(
|
||||
Future<void>(() 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 = <String, String>{};
|
||||
channelsAsync.whenData((channels) {
|
||||
for (final ch in channels) {
|
||||
channelNamesMap[ch.name.toLowerCase()] = ch.id;
|
||||
}
|
||||
});
|
||||
final channelNamesMap = ref.watch(channelIdsByNameProvider);
|
||||
|
||||
return Stack(
|
||||
children: [
|
||||
|
||||
@@ -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, Map<String, String>>(
|
||||
ChannelIdsByName.new,
|
||||
);
|
||||
|
||||
class ChannelIdsByName extends Notifier<Map<String, String>> {
|
||||
@override
|
||||
Map<String, String> build() => Map.unmodifiable({
|
||||
for (final channel in ref.watch(channelsProvider).value ?? const [])
|
||||
channel.name.toLowerCase(): channel.id,
|
||||
});
|
||||
|
||||
@override
|
||||
bool updateShouldNotify(
|
||||
Map<String, String> previous,
|
||||
Map<String, String> next,
|
||||
) => !mapEquals(previous, next);
|
||||
}
|
||||
@@ -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
|
||||
: <String, String>{
|
||||
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) {
|
||||
|
||||
@@ -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 = <String, String>{};
|
||||
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
|
||||
|
||||
@@ -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 = <Widget>[];
|
||||
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<MessageContent>(), 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<void> _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<bool> fetchOlder() async {
|
||||
fetchOlderCalls += 1;
|
||||
await Future<void>.delayed(fetchDelay);
|
||||
_reachedOldest = true;
|
||||
return false;
|
||||
}
|
||||
|
||||
void reconnect(List<NostrEvent> 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);
|
||||
|
||||
Reference in New Issue
Block a user