perf(image): move image pre-rendering off the main thread#24
Merged
Conversation
It turns out that the reason image loading was causing scroll hangs and general UI misery is that we were doing *all* the expensive image work on the main thread. Fetching happened in the background (good), but as soon as the raw image arrived, pre_render() would run synchronously — Lanczos3 resizing, PNG encoding, Sixel quantization, base64 encoding — right there in the event loop. Blocking everything. This is not great. The old code also called rebuild() on every single image completion, which re-parses the entire markdown document just to adjust a few placeholder rows. For a document with ten images, that's ten full re-parses for absolutely no reason. The fix has two parts: 1. Replace the synchronous pre_render() with background threads. queue_all_pre_renders() spawns a thread per image that does the heavy resize/encode work off the main thread. Results come back via a dedicated channel (poll_pre_rendered()), separate from the fetch channel. Images are stored as Arc<DynamicImage> so we hand off a refcount bump instead of cloning potentially 16MB of pixel data. 2. Extract finalize_layout() from rebuild(). When a raw image fetch completes, the event loop now calls finalize_layout() instead of rebuild() — it adjusts placeholder rows and rebuilds indices without re-parsing markdown. The main thread never touches image pixels. Stale pre-render results (from terminal resizes or file switches) are discarded via content_width checks and channel replacement, same pattern we already use for fetch cancellation.
The pre-render pipeline was happily spawning a new thread for every single image with *no* concurrency limit. The fetch side already had MAX_CONCURRENT_FETCHES = 10, but apparently nobody thought the pre-render side deserved the same courtesy. A document with 30 images would spawn 30 Lanczos3 resize threads simultaneously. That's not parallelism, that's a denial of service attack on your own CPU. While at it, poll_pre_rendered() was setting `any = true` even when the pre-render result was None (i.e., the background thread panicked). This triggered a pointless re-render cycle for an image that will never be ready. Move the flag inside the Some(data) arm where it belongs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
rebuild()with lightweightfinalize_layout()when image fetches complete — adjusts placeholder rows and rebuilds indices without re-parsing the entire markdown documentArc<DynamicImage>to avoid cloning large pixel buffers when handing off to pre-render threadsDetails
The old code fetched images in the background but ran
pre_render()synchronously on the main thread. For large images this meant hundreds of milliseconds of Lanczos3 resizing and protocol-specific encoding blocking the event loop. It also calledrebuild()(full markdown re-parse) on every single image completion.Now there are two background channels:
Results from stale terminal widths or cancelled file switches are automatically discarded. The viewer shows a "Loading" placeholder until both fetch and pre-render complete.
Test plan
cargo build— compiles cleanlycargo test— all 120 tests passcargo clippy— no warnings