From 57e7f73aa856061f169936b24f37adde093cbf38 Mon Sep 17 00:00:00 2001 From: Tommy Chow Date: Fri, 3 Jul 2026 14:45:20 -0400 Subject: [PATCH] perf(chat): cut per-flush rebuild and decode costs in message list every 200ms flush shifted all sliver indices (reverse list), and the per-row ObjectKeys without findChildIndexCallback made the framework deactivate and re-inflate every built row instead of moving it. add the callback so elements move, and cache ChatMessage widget instances per message (Expando) so unchanged rows skip build() entirely via the identical-widget short-circuit. IRCMessage.renderRevision invalidates rows the CLEARCHAT/CLEARMSG handlers mutate in place. also decode emotes/badges at display size (memCacheHeight = logical size x devicePixelRatio) instead of the CDN's 3x-4x intrinsic size: several-fold less image memory and raster cost, animation preserved (ResizeImage applies the target size to the whole codec). Co-Authored-By: Claude Fable 5 --- lib/models/irc.dart | 26 +++ lib/screens/channel/chat/chat.dart | 157 +++++++++++++++---- lib/widgets/frosty_cached_network_image.dart | 14 ++ test/models/irc_test.dart | 38 +++++ 4 files changed, 207 insertions(+), 28 deletions(-) diff --git a/lib/models/irc.dart b/lib/models/irc.dart index e7f7ce78..7009ca23 100644 --- a/lib/models/irc.dart +++ b/lib/models/irc.dart @@ -42,6 +42,12 @@ class IRCMessage { final VoidCallback? actionCallback; final String? actionLabel; + /// Bumped whenever the message is mutated in place after creation (the + /// moderation marking in [clearChat]/[clearMessage]), so render output + /// cached against this message's identity knows to invalidate. Purely a + /// render-cache signal — not part of the IRC model. + int renderRevision = 0; + IRCMessage({ required this.raw, required this.command, @@ -85,6 +91,7 @@ class IRCMessage { if (message.user == bannedUser) { // Mark the message for removal. messages[i].command = Command.clearChat; + messages[i].renderRevision++; // If timed-out, indicate the duration. if (banDuration != null) { @@ -98,6 +105,7 @@ class IRCMessage { if (bufferedMessage.user == bannedUser) { // Mark the message for removal. bufferedMessages[i].command = Command.clearChat; + bufferedMessages[i].renderRevision++; // If timed-out, indicate the duration. if (banDuration != null) { @@ -119,6 +127,7 @@ class IRCMessage { for (var i = 0; i < messages.length; i++) { if (messages[i].tags['id'] == targetId) { messages[i].command = Command.clearMessage; + messages[i].renderRevision++; break; } } @@ -127,6 +136,7 @@ class IRCMessage { for (var i = 0; i < bufferedMessages.length; i++) { if (bufferedMessages[i].tags['id'] == targetId) { bufferedMessages[i].command = Command.clearMessage; + bufferedMessages[i].renderRevision++; break; } } @@ -591,6 +601,10 @@ class IRCMessage { imageUrl: emote.url, height: emote.height != null ? emote.height! * emoteScale : emoteSize, width: emote.width != null ? emote.width! * emoteScale : emoteSize, + memCacheHeight: _memCacheDimension( + context, + emote.height != null ? emote.height! * emoteScale : emoteSize, + ), useFade: false, ), ), @@ -748,6 +762,7 @@ class IRCMessage { double? size, Color? backgroundColor, bool? isSvg, + int? memCacheHeight, }) { if (backgroundColor != null) { return ColoredBox( @@ -756,6 +771,7 @@ class IRCMessage { imageUrl: badge.url, height: size, width: size, + memCacheHeight: memCacheHeight, useFade: false, ), ); @@ -766,6 +782,7 @@ class IRCMessage { imageUrl: badge.url, height: size, width: size, + memCacheHeight: memCacheHeight, useFade: false, ); } @@ -801,6 +818,7 @@ class IRCMessage { size: size, backgroundColor: backgroundColor, isSvg: isSvg, + memCacheHeight: _memCacheDimension(context, size), ), ), ); @@ -825,6 +843,9 @@ class IRCMessage { imageUrl: emote.url, height: height, width: width, + // Decode at the rendered size instead of the CDN's 3x-4x intrinsic + // size. Height-only preserves the aspect ratio. + memCacheHeight: _memCacheDimension(context, height), useFade: false, placeholder: (context, url) => const SizedBox(), ), @@ -832,6 +853,11 @@ class IRCMessage { ); } + /// Converts a layout dimension to a physical-pixel decode cap for the + /// in-memory image cache. + static int _memCacheDimension(BuildContext context, double logicalSize) => + (logicalSize * MediaQuery.devicePixelRatioOf(context)).round(); + static TextSpan _createTextSpan({ required String text, required bool launchExternal, diff --git a/lib/screens/channel/chat/chat.dart b/lib/screens/channel/chat/chat.dart index 80ceda50..ffe2e0ee 100644 --- a/lib/screens/channel/chat/chat.dart +++ b/lib/screens/channel/chat/chat.dart @@ -1,6 +1,8 @@ import 'package:flutter/material.dart'; import 'package:flutter/rendering.dart'; import 'package:flutter_mobx/flutter_mobx.dart'; +import 'package:frosty/models/irc.dart'; +import 'package:frosty/models/user.dart'; import 'package:frosty/screens/channel/chat/emote_menu/emote_menu_panel.dart'; import 'package:frosty/screens/channel/chat/emote_menu/recent_emotes_panel.dart'; import 'package:frosty/screens/channel/chat/stores/chat_store.dart'; @@ -16,6 +18,27 @@ import 'package:frosty/widgets/frosty_scrollbar.dart'; /// ahead of fast scroll-back without holding the entire history. const _chatCacheExtent = ScrollCacheExtent.pixels(500.0); +/// Cached [ChatMessage] instances keyed by message identity. Returning the +/// identical widget instance across list rebuilds lets the framework skip the +/// row's build entirely (`Element.updateChild` short-circuits on identical +/// widgets), so a batch flush only builds the genuinely new messages instead +/// of regenerating every visible row's spans. Entries are GC'd together with +/// their messages; [IRCMessage.renderRevision] invalidates rows the moderation +/// handlers mutate in place. MobX reactivity is unaffected — each row's +/// Observer subscription lives on its element, not its widget instance. +final _messageWidgetCache = Expando<_CachedMessageWidget>(); + +/// Merged-mode cache, kept separate because merged rows carry per-active-tab +/// parameters that must invalidate the cached widget when they change. +final _mergedMessageWidgetCache = Expando<_CachedMessageWidget>(); + +class _CachedMessageWidget { + const _CachedMessageWidget(this.widget, this.revision); + + final ChatMessage widget; + final int revision; +} + class Chat extends StatelessWidget { final ChatStore chatStore; final EdgeInsetsGeometry? listPadding; @@ -176,6 +199,10 @@ class Chat extends StatelessWidget { ScrollController scrollController, double bottomPadding, ) { + final messages = chatStore.renderMessages; + // Built lazily on the first keyed-child lookup of a rebuild. + Map? sliverIndexByMessage; + return ListView.builder( reverse: true, padding: (listPadding ?? EdgeInsets.zero).add( @@ -184,23 +211,51 @@ class Chat extends StatelessWidget { addAutomaticKeepAlives: false, scrollCacheExtent: _chatCacheExtent, controller: scrollController, - itemCount: chatStore.renderMessages.length, + itemCount: messages.length, + // Every appended batch shifts all sliver indices (the list is reversed, + // so new messages land at index 0). Mapping a row's key back to its new + // index here lets the framework *move* the existing element — without + // it, the key mismatch at each shifted index would deactivate and + // re-inflate every built row on every flush. + findChildIndexCallback: (key) { + final lookup = sliverIndexByMessage ??= { + for (var i = 0; i < messages.length; i++) + messages[messages.length - 1 - i]: i, + }; + return lookup[(key as ObjectKey).value]; + }, itemBuilder: (context, index) { - final ircMessage = chatStore - .renderMessages[chatStore.renderMessages.length - 1 - index]; - // Key by message identity so the list reconciles by message rather - // than by slot. This keeps each message's element (and its gesture - // recognizers) bound to the same message when new messages arrive - // mid-interaction, fixing long-press landing on the wrong message. - return ChatMessage( - key: ObjectKey(ircMessage), - ircMessage: ircMessage, - chatStore: chatStore, - ); + final ircMessage = messages[messages.length - 1 - index]; + return _cachedChatMessage(ircMessage); }, ); } + /// Returns the (cached) widget for [ircMessage], building it on first + /// render. Keyed by message identity so the list reconciles by message + /// rather than by slot — this keeps each message's element (and its gesture + /// recognizers) bound to the same message when new messages arrive + /// mid-interaction, fixing long-press landing on the wrong message. + Widget _cachedChatMessage(IRCMessage ircMessage) { + final cached = _messageWidgetCache[ircMessage]; + if (cached != null && + cached.revision == ircMessage.renderRevision && + identical(cached.widget.chatStore, chatStore)) { + return cached.widget; + } + + final widget = ChatMessage( + key: ObjectKey(ircMessage), + ircMessage: ircMessage, + chatStore: chatStore, + ); + _messageWidgetCache[ircMessage] = _CachedMessageWidget( + widget, + ircMessage.renderRevision, + ); + return widget; + } + Widget _buildMergedList( ScrollController scrollController, double bottomPadding, @@ -208,6 +263,8 @@ class Chat extends StatelessWidget { final mergedMessages = chatTabsStore!.mergedMessages; final channelIdToUserTwitch = chatTabsStore!.mergedChannelIdToUserTwitch; final currentChannelId = chatTabsStore!.activeTab.channelId; + // Built lazily on the first keyed-child lookup of a rebuild. + Map? sliverIndexByMessage; return ListView.builder( reverse: true, @@ -218,29 +275,73 @@ class Chat extends StatelessWidget { scrollCacheExtent: _chatCacheExtent, controller: scrollController, itemCount: mergedMessages.length, + // See _buildNormalList — moves existing elements across index shifts. + findChildIndexCallback: (key) { + final lookup = sliverIndexByMessage ??= { + for (var i = 0; i < mergedMessages.length; i++) + mergedMessages[mergedMessages.length - 1 - i].ircMessage: i, + }; + return lookup[(key as ObjectKey).value]; + }, itemBuilder: (context, index) { final merged = mergedMessages[mergedMessages.length - 1 - index]; - return ChatMessage( - key: ObjectKey(merged.ircMessage), - ircMessage: merged.ircMessage, - chatStore: merged.chatStore, - inputChatStore: chatStore, - onActivateSourceTab: () { - final tabs = chatTabsStore!.tabs; - final idx = tabs.indexWhere( - (t) => t.channelId == merged.chatStore.channelId, - ); - if (idx != -1 && idx != chatTabsStore!.activeTabIndex) { - chatTabsStore!.setActiveTab(idx, silent: true); - } - }, - overrideChannelIdToUserTwitch: channelIdToUserTwitch, - overrideCurrentChannelId: currentChannelId, + return _cachedMergedChatMessage( + merged, + channelIdToUserTwitch, + currentChannelId, ); }, ); } + /// Merged-mode variant of [_cachedChatMessage]. The extra identity checks + /// invalidate the cached widget when the active tab (send target) or the + /// combined channel-profile map changes. + Widget _cachedMergedChatMessage( + MergedMessage merged, + Map channelIdToUserTwitch, + String currentChannelId, + ) { + final ircMessage = merged.ircMessage; + final cached = _mergedMessageWidgetCache[ircMessage]; + final cachedWidget = cached?.widget; + if (cached != null && + cachedWidget != null && + cached.revision == ircMessage.renderRevision && + identical(cachedWidget.chatStore, merged.chatStore) && + identical(cachedWidget.inputChatStore, chatStore) && + identical( + cachedWidget.overrideChannelIdToUserTwitch, + channelIdToUserTwitch, + ) && + cachedWidget.overrideCurrentChannelId == currentChannelId) { + return cachedWidget; + } + + final widget = ChatMessage( + key: ObjectKey(ircMessage), + ircMessage: ircMessage, + chatStore: merged.chatStore, + inputChatStore: chatStore, + onActivateSourceTab: () { + final tabs = chatTabsStore!.tabs; + final idx = tabs.indexWhere( + (t) => t.channelId == merged.chatStore.channelId, + ); + if (idx != -1 && idx != chatTabsStore!.activeTabIndex) { + chatTabsStore!.setActiveTab(idx, silent: true); + } + }, + overrideChannelIdToUserTwitch: channelIdToUserTwitch, + overrideCurrentChannelId: currentChannelId, + ); + _mergedMessageWidgetCache[ircMessage] = _CachedMessageWidget( + widget, + ircMessage.renderRevision, + ); + return widget; + } + Widget _buildResumeScrollButton() { return Builder( builder: (context) { diff --git a/lib/widgets/frosty_cached_network_image.dart b/lib/widgets/frosty_cached_network_image.dart index 0f1f9934..4a2def96 100644 --- a/lib/widgets/frosty_cached_network_image.dart +++ b/lib/widgets/frosty_cached_network_image.dart @@ -8,6 +8,16 @@ class FrostyCachedNetworkImage extends StatelessWidget { final String? cacheKey; final double? width; final double? height; + + /// Decode size caps (physical pixels) for the in-memory image. Without + /// these, images decode and stay resident at their intrinsic size — emote + /// CDNs serve 3x-4x assets (~112-128 px) that render in ~28 lp boxes, so + /// capping the decode cuts memory and raster cost several-fold. Applies to + /// animated codecs too (all frames decode at the target size; animation is + /// preserved). Providing only one axis keeps the aspect ratio. + final int? memCacheWidth; + final int? memCacheHeight; + final Color? color; final BlendMode? colorBlendMode; final Widget Function(BuildContext, String)? placeholder; @@ -21,6 +31,8 @@ class FrostyCachedNetworkImage extends StatelessWidget { this.cacheKey, this.width, this.height, + this.memCacheWidth, + this.memCacheHeight, this.color, this.colorBlendMode, this.placeholder, @@ -36,6 +48,8 @@ class FrostyCachedNetworkImage extends StatelessWidget { cacheKey: cacheKey, width: width, height: height, + memCacheWidth: memCacheWidth, + memCacheHeight: memCacheHeight, color: color, colorBlendMode: colorBlendMode, placeholder: placeholder, diff --git a/test/models/irc_test.dart b/test/models/irc_test.dart index 56b1cf99..a3851935 100644 --- a/test/models/irc_test.dart +++ b/test/models/irc_test.dart @@ -339,6 +339,35 @@ void main() { expect(affectedMessage.tags['ban-duration'], '300'); }); + test('bumps renderRevision only on affected messages', () { + final clearChatMsg = IRCMessage( + raw: '', + command: Command.clearChat, + tags: {'ban-duration': '600'}, + message: 'testuser', + ); + bufferedMessages = [ + IRCMessage.fromString(basicPrivmsg), // from testuser + ]; + + IRCMessage.clearChat( + messages: messages, + bufferedMessages: bufferedMessages, + ircMessage: clearChatMsg, + ); + + // Mutated-in-place messages must signal render caches to invalidate. + expect( + messages.where((m) => m.user == 'testuser').first.renderRevision, + 1, + ); + expect(bufferedMessages.first.renderRevision, 1); + expect( + messages.where((m) => m.user == 'moduser').first.renderRevision, + 0, + ); + }); + test('clears entire chat when no target user', () { final clearAllMsg = IRCMessage( raw: '', @@ -408,6 +437,15 @@ void main() { messages.firstWhere((m) => m.tags['id'] == 'msg-2').command, Command.clearMessage, ); + // Mutated-in-place message must signal render caches to invalidate. + expect( + messages.firstWhere((m) => m.tags['id'] == 'msg-2').renderRevision, + 1, + ); + expect( + messages.firstWhere((m) => m.tags['id'] == 'msg-1').renderRevision, + 0, + ); // Other messages unchanged expect( messages.firstWhere((m) => m.tags['id'] == 'msg-1').command,