Skip to content

Address performance problems for StructuredText code blocks#39

Closed
danielrothmann wants to merge 3 commits into
gonzalezreal:mainfrom
42futures:bugfix/streaming_crash
Closed

Address performance problems for StructuredText code blocks#39
danielrothmann wants to merge 3 commits into
gonzalezreal:mainfrom
42futures:bugfix/streaming_crash

Conversation

@danielrothmann

Copy link
Copy Markdown

This PR improves the reliability of StructuredText when rendering large code blocks and makes Textual more suited to streaming.

Context

It has previously been observed that the library may cause a crash for large markdown documents as discussed in #23

If using StructuredText for streaming content (frequent layout updates), this problem can be excessive.

The causes

I found a few things that influence this behaviour:

1) Stack overflow in TextBuilder concatenation

TextBuilder constructs the final Text view by reducing an array of per-run Text values with string interpolation.

self = textValues.reduce(Text(verbatim: "")) { partialResult, text in
    Text("\(partialResult)\(text)")
 }

This creates an unbalanced tree where each step nests the previous result on the left. For syntax-highlighted code blocks, it creates a very deep Text tree. When SwiftUI resolves it, Text.resolve() recurses through every level and with hundreds of code block tokens, causes a stack overflow.

The change here balances that tree, so that each side has roughly equal depth. This makes a stack overflow from excessive recursion depth very unlikely.

2) Borrowed AttributedSubstring captured across async boundary

HighlighedTextFragment passes its content directly to a task closure for tokenisation. AttributedSubstring borrows storage from its parent AttributedString, so if the content updates rapidly (like when streaming), the parent can be replaced while the async task may still reference the old substring storage.

3) Redundant tokenisation when there is no highlighting

When content changes frequently, HighlightedTextFragment re-tokenises on every update. In the streaming scenario, this leads to performance degradation. If syntax highlighting is disabled, the tokenisation pass can be skipped, which allows downstream applications to disable syntax while streaming and only pay the tokenisation cost once, when done.

jerrickhakim added a commit to jerrickhakim/textual that referenced this pull request Mar 28, 2026
Cherry-picked from gonzalezreal#39:
- Replace linear Text reduce with balanced binary tree concatenation
  to prevent stack overflow on long code blocks (O(log N) vs O(N) depth)
- Convert AttributedSubstring to owned String before async boundary
  to prevent dangling references during streaming
- Skip expensive tokenizer when theme has no token properties

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
AldenClark added a commit to AldenClark/textual that referenced this pull request Apr 21, 2026
@gonzalezreal

gonzalezreal commented Jun 13, 2026

Copy link
Copy Markdown
Owner

Sorry for the late review. I’ve been swamped with work.

@danielrothmann Thanks for putting this together!

I found one issue before merging: if an app uses .plain while streaming and then switches back to .default for the same code, syntax highlighting will not come back.

Could you update the task key to include the language hint and whether the theme needs tokens, and also make highlighting react to theme changes?

The GitHub workflows aged out because this PR is from a fork, so we can rerun them after the update.

@gonzalezreal

Copy link
Copy Markdown
Owner

I ended up merging #65 for the TextBuilder stack overflow fix, since it was smaller and included a regression test. This PR also includes useful highlighter improvements, especially skipping tokenization when highlighting is disabled.
I’m going to close this one as partially superseded, but I’d be happy to review a smaller follow-up PR with those remaining changes.

@danielrothmann

Copy link
Copy Markdown
Author

No problem @gonzalezreal and sounds good to get the stack overflow fix in. I'll try and have a look later and come back with an updated PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants