jerry/buzz
1
0
Fork 0
mirror of https://github.com/block/buzz.git synced 2026-11-03 16:01:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

perf(mobile): stop re-parsing message markdown on every rebuild (#8928)

Browse Source 
## Context

Each message in a channel shows its text as Markdown. The `GptMarkdown`
widget parses that text. Parsing is slow for long messages.

`GptMarkdownTheme` gives Markdown colours and styles to the widgets
below it. When it gets a new theme data object, it tells all
`GptMarkdown` widgets below it to parse their text again. It compares
the old and new objects by identity, because `GptMarkdownThemeData` has
no value equality. Thus a new object with the same values also causes a
parse.

## Problem

Two widgets made a new theme data object on each build:
- `MessageContent` made one for each message bubble.
- `AppMarkdownTheme` made one for the full app.

Thus each rebuild parsed all messages on the screen again. Many events
cause a rebuild: a batch of profiles, a read marker, a keyboard frame.
In a large channel such as #4things (1,235 members), these events occur
almost all the time.

## Change

- `MessageContent` keeps its bubble theme until one of its inputs
changes. The inputs are: the app Markdown theme, the link colour, own or
other message, the ink colour and `onSurface`.
- `AppMarkdownTheme` makes new theme data only when `ThemeData` changes
or when an outer Markdown theme changes.

A change of colour, of brightness, or of own or other message still
makes new theme data. Then the Markdown parses again with the new
colours. This is correct.

### Related issue

None found.

## Testing

- New regression tests fail on main. They pass on this branch.
- `message_content_stability_test.dart`: When a bubble rebuilds with the
same inputs, it keeps the same theme object and the same parsed
`TextSpan`. The test does this for own messages and for other messages.
When own or other changes, the bubble gets a new theme.
- `app_markdown_theme_test.dart`: The app keeps one theme object when it
rebuilds. It makes a new one when light or dark mode changes.
- `just mobile-install mobile-check mobile-test` passes.
- Device test: iPhone 12 mini, Profile build, Tom's Block community,
open #4things. The two runs had different live traffic and different
cached content. Thus they show the cause and the fix. They do not give a
controlled speed measurement.
- Before: In the first 2.4 s, parses from rebuilds (`performRebuild`)
used approximately 90 ms. One frame used 116 ms. Approximately 48 ms of
that frame parsed visible messages again.
- After: In 30 s, rebuilds caused no parses. Only the first build of
each message (`_firstBuild`) parsed.
- An independent agent review found no defects. It found one small item:
`ChatColors` has no value equality. Thus a rebuild of `MaterialApp`
still makes new app theme data one time. This costs only a little
performance. We did not change it.

Not tested: A human test did not occur. The mobile testing exception
waives it, and an agent did the device test. No test occurred on an
Android device.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Signed-off-by: Tom Brow <tomb@squareup.com>
Co-authored-by: Tom Brow <tomb@squareup.com>
Co-authored-by: studio <dc2fc481343672df31a689840c09299972e704dac592e0da6f18486646a7ee3e@buzz.block.builderlab.xyz>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Tom Brow
2026-11-03 08:00:33 -08:00
committed by GitHub
parent eee75a1c7b
commit 369e1ef322
4 changed files with 140 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files

74
mobile/lib/features/channels/message_content.dart
View File

@@ -209,6 +209,32 @@ class MessageContent extends HookConsumerWidget {
// Own-bubble links share the bubble ink; markdown marks them by underline.
final linkColor = ownMessage ? ownTextColor : context.colors.tertiary;
final bubbleInk = ownMessage ? ownTextColor : context.colors.onSurface;
final onSurface = context.colors.onSurface;
// GptMarkdownTheme notifies its dependents whenever the data object
// changes (the data has no value equality), and each notification makes
// every GptMarkdown below re-parse its text. Reuse one bubble theme while
// its inputs are unchanged so an ordinary rebuild does not re-parse.
final bubbleMarkdownTheme = useMemoized(
() => markdownTheme.copyWith(
linkColor: linkColor,
// A neutral chip would put white ink on a light fill; tint the accent
// instead.
inlineCode: ownMessage
? markdownTheme.inlineCode.copyWith(
color: ownTextColor,
backgroundColor: chatCodeTint(ownTextColor, 0.18),
borderColor: ownTextColor.withValues(alpha: 0.3),
)
: null,
h1: markdownTheme.h1?.copyWith(color: bubbleInk),
h2: markdownTheme.h2?.copyWith(color: bubbleInk),
h3: markdownTheme.h3?.copyWith(color: bubbleInk),
h4: markdownTheme.h4?.copyWith(color: bubbleInk),
h5: markdownTheme.h5?.copyWith(color: bubbleInk),
h6: markdownTheme.h6?.copyWith(color: bubbleInk),
),
[markdownTheme, linkColor, ownMessage, ownTextColor, onSurface],
);
final baseTextStyle =
(baseStyle ??
context.textTheme.bodyMedium?.copyWith(
@@ -474,53 +500,7 @@ class MessageContent extends HookConsumerWidget {
),
),
child: GptMarkdownTheme(
gptThemeData: markdownTheme.copyWith(
linkColor: linkColor,
// A neutral chip would put white ink on a light
// fill; tint the accent instead.
inlineCode: ownMessage
? markdownTheme.inlineCode.copyWith(
color: ownTextColor,
backgroundColor: chatCodeTint(
ownTextColor,
0.18,
),
borderColor: ownTextColor.withValues(
alpha: 0.3,
),
)
: null,
h1: markdownTheme.h1?.copyWith(
color: ownMessage
? ownTextColor
: context.colors.onSurface,
),
h2: markdownTheme.h2?.copyWith(
color: ownMessage
? ownTextColor
: context.colors.onSurface,
),
h3: markdownTheme.h3?.copyWith(
color: ownMessage
? ownTextColor
: context.colors.onSurface,
),
h4: markdownTheme.h4?.copyWith(
color: ownMessage
? ownTextColor
: context.colors.onSurface,
),
h5: markdownTheme.h5?.copyWith(
color: ownMessage
? ownTextColor
: context.colors.onSurface,
),
h6: markdownTheme.h6?.copyWith(
color: ownMessage
? ownTextColor
: context.colors.onSurface,
),
),
gptThemeData: bubbleMarkdownTheme,
child: renderMarkdown(segment.text),
),
),

22
mobile/lib/shared/theme/app_markdown_theme.dart
View File

@@ -1,4 +1,5 @@
import 'package:flutter/material.dart';
import 'package:flutter_hooks/flutter_hooks.dart';
import 'package:gpt_markdown/gpt_markdown.dart';
import 'app_theme.dart';
@@ -15,16 +16,23 @@ import 'code_style.dart';
/// ambient headings carry the localized text geometry, which exists only once
/// `Theme.of` has run and a `ThemeData` factory cannot see. Overriding the one
/// field on the ambient theme is what leaves non-code Markdown untouched.
class AppMarkdownTheme extends StatelessWidget {
class AppMarkdownTheme extends HookWidget {
const AppMarkdownTheme({super.key, required this.child});
final Widget child;
@override
Widget build(BuildContext context) {
final scheme = Theme.of(context).colorScheme;
return GptMarkdownTheme(
gptThemeData: GptMarkdownTheme.of(context).copyWith(
final theme = Theme.of(context);
final outer = context
.dependOnInheritedWidgetOfExactType<GptMarkdownTheme>()
?.gptThemeData;
// GptMarkdownTheme notifies by data identity, and every notification makes
// each GptMarkdown in the app re-parse its text. Derive new data only when
// what it is derived from changes.
final data = useMemoized(() {
final scheme = theme.colorScheme;
return GptMarkdownTheme.of(context).copyWith(
// Inline code is otherwise drawn in gpt_markdown's own bundled face, at
// its own size, tinted from `onSurface`. Stating the app's code style
// gives every Markdown surface the face, size and chip colours a fenced
@@ -39,8 +47,8 @@ class AppMarkdownTheme extends StatelessWidget {
// scale rather than the `Radii.card` a block uses.
borderRadius: const Radius.circular(Radii.xs),
).resolve(scheme),
),
child: child,
);
);
}, [theme, outer]);
return GptMarkdownTheme(gptThemeData: data, child: child);
}
}

54
mobile/test/features/channels/message_content_stability_test.dart
View File

@@ -22,6 +22,8 @@ class _Fixture {
bool scaleEmoji = false;
bool mentionHandler = true;
bool mediaHandlers = false;
bool bubble = false;
bool ownMessage = false;
int? tappedRevision;
String? tappedId;
@@ -40,6 +42,8 @@ class _Fixture {
agentMentionPubkeys: Set.of(agents),
tags: tags,
scaleEmojiOnly: scaleEmoji,
bubble: bubble,
ownMessage: ownMessage,
onMediaReply: mediaHandlers
? () {
tappedRevision = generation;
@@ -79,7 +83,57 @@ List<MarkdownComponent> _components(WidgetTester tester) => tester
.widget<GptMarkdown>(find.byType(GptMarkdown).first)
.inlineComponents!;
GptMarkdownThemeData _bubbleTheme(WidgetTester tester) => tester
.widget<GptMarkdownTheme>(
find
.ancestor(
of: find.byType(GptMarkdown).first,
matching: find.byType(GptMarkdownTheme),
)
.first,
)
.gptThemeData;
InlineSpan _renderedText(WidgetTester tester) => tester
.widget<RichText>(
find
.descendant(
of: find.byType(GptMarkdown).first,
matching: find.byType(RichText),
)
.first,
)
.text;
void main() {
for (final own in [false, true]) {
testWidgets(
'a bubble rebuild with unchanged inputs keeps its parsed markdown '
'(own: $own)',
(tester) async {
final fixture = _Fixture()
..bubble = true
..ownMessage = own
..content = '# Title\n\nHello `code` and #sample';
addTearDown(fixture.revision.dispose);
await tester.pumpWidget(fixture.build());
final theme = _bubbleTheme(tester);
final text = _renderedText(tester);
for (var i = 0; i < 3; i++) {
await fixture.refresh(tester);
// A new theme object would notify every GptMarkdown below it, and
// each one would parse its text again.
expect(_bubbleTheme(tester), same(theme));
expect(_renderedText(tester), same(text));
}
fixture.ownMessage = !own;
await fixture.refresh(tester);
expect(_bubbleTheme(tester), isNot(same(theme)));
expect(_bubbleTheme(tester).linkColor, isNot(theme.linkColor));
},
);
}
testWidgets('explicit channel links retain the current callback', (
tester,
) async {

44
mobile/test/shared/theme/app_markdown_theme_test.dart
View File

@@ -33,6 +33,50 @@ Future<GptMarkdownThemeData> _markdownTheme(
}
void main() {
testWidgets('keeps one theme object until the app theme changes', (
tester,
) async {
final theme = ValueNotifier(AppTheme.light());
final revision = ValueNotifier(0);
addTearDown(theme.dispose);
addTearDown(revision.dispose);
final reads = <GptMarkdownThemeData>[];
await tester.pumpWidget(
ValueListenableBuilder<ThemeData>(
valueListenable: theme,
builder: (_, data, _) => MaterialApp(
theme: data,
home: ValueListenableBuilder<int>(
valueListenable: revision,
// A fresh subtree each time, as an ordinary ancestor rebuild gives.
builder: (_, generation, _) => AppMarkdownTheme(
child: Builder(
key: ValueKey(generation),
builder: (context) {
reads.add(GptMarkdownTheme.of(context));
return const SizedBox.shrink();
},
),
),
),
),
),
);
await tester.pumpAndSettle();
final first = reads.last;
for (var i = 0; i < 3; i++) {
revision.value++;
await tester.pump();
// GptMarkdownTheme notifies by identity: a new object would make every
// GptMarkdown in the app parse its text again.
expect(reads.last, same(first));
}
theme.value = AppTheme.dark();
await tester.pumpAndSettle();
expect(reads.last, isNot(same(first)));
expect(reads.last.inlineCode.color, AppTheme.dark().colorScheme.onSurface);
});
testWidgets('states inline code and leaves the rest of Markdown alone', (
tester,
) async {