⚡ Bolt: [performance improvement] Optimize terminal scrollback search fast-path - #413
⚡ Bolt: [performance improvement] Optimize terminal scrollback search fast-path#413Lucenx9 wants to merge 1 commit into
Conversation
This commit optimizes the `for_each_char_match_start` hot loop in the terminal search implementation. By hoisting the first character's ASCII case-folding outside the loop and checking `h.is_ascii()` before taking the equality fast-path, we avoid the overhead of heavy Unicode case-folding functions for the vast majority of non-matches during full scrollback scans. Co-authored-by: Lucenx9 <185146821+Lucenx9@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8fde20276b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -0,0 +1,3 @@ | |||
| ## 2024-03-12 - [Optimizing Terminal Scrollback Search] | |||
There was a problem hiding this comment.
.jules root bucket
This file is automation scratch material rather than a durable product, tooling, or documentation category, so committing it introduces an ad hoc top-level directory with no runtime or build owner. Remove it instead of restoring repository clutter that CHANGELOG.md:2342 records as previously cleaned up.
AGENTS.md reference: AGENTS.md:L131-L131
Useful? React with 👍 / 👎.
| let first_is_ascii = first_needle.is_ascii(); | ||
| let first_lower = first_needle.to_ascii_lowercase(); | ||
| let first_upper = first_needle.to_ascii_uppercase(); |
There was a problem hiding this comment.
This optimization is explicitly intended to make terminal scrollback search noticeably more responsive, but the commit does not add an entry under CHANGELOG.md's Unreleased section. Add a user-facing Fixed or Changed entry so this release-visible performance improvement is not omitted from the release notes.
AGENTS.md reference: AGENTS.md:L183-L183
Useful? React with 👍 / 👎.
💡 What
Optimized the
for_each_char_match_starthot loop incrates/forktty-ui-gtk/src/gtk_app/terminal_search.rs. It now pre-calculates the ASCII uppercase/lowercase variants of the first character of the needle and does a fast inline ASCII match for the first character reject, avoiding expensive calls tochars_eq_ignore_casefor the vast majority of characters. It properly falls back to full Unicode comparison for non-ASCII characters to preserve correctness (e.g., Kelvin sign).🎯 Why
Terminal scrollback can be huge (100k+ lines). When searching, the algorithm rejects millions of characters on the first letter. The previous implementation called
chars_eq_ignore_casefor every single character check. While it had an ASCII fast path inside, the overhead of the function call and branch setup was still a bottleneck for the millions of non-matching characters.📊 Impact
Significant reduction in CPU time spent scanning massive scrollback buffers for non-matching characters, leading to a much more responsive search experience in the UI.
🔬 Measurement
Running
cargo test -p forktty-ui-gtk bench_search_on_huge_scrollback -- --nocapture --ignoredwithFORKTTY_BENCH_SCROLLBACK=100000shows improved search times. (A custom python test script verified the logic correctness).PR created automatically by Jules for task 1658216959720328621 started by @Lucenx9