⚡ Bolt: Optimize terminal scrollback search with ASCII fast-path - #412
⚡ Bolt: Optimize terminal scrollback search with ASCII fast-path#412Lucenx9 wants to merge 1 commit into
Conversation
Hoist target character's ASCII lower/upper conversions outside the search loop and use them for a fast short-circuit check against ASCII haystack characters. 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: 0315c343fe
ℹ️ 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".
| import sys | ||
|
|
||
| with open("crates/forktty-ui-gtk/src/gtk_app/terminal_search.rs", "r") as f: |
There was a problem hiding this comment.
This is an unused implementation artifact: a repository-wide search finds no caller, and running it only performs an unchecked textual rewrite of terminal_search.rs that is already present in this commit. Keeping it at the repository root creates a misleading mutation entry point unrelated to the build or test workflow; remove it rather than shipping it.
AGENTS.md reference: AGENTS.md:L122-L124
Useful? React with 👍 / 👎.
| // ⚡ Bolt: Fast path for case-insensitive search. | ||
| // Hoist target character's ASCII lower/upper conversions outside the loop. | ||
| // If the haystack character is ASCII, we can directly compare it against bounds. | ||
| let first_needle = needle[0]; | ||
| let first_lower = first_needle.to_ascii_lowercase(); | ||
| let first_upper = first_needle.to_ascii_uppercase(); |
There was a problem hiding this comment.
This commit presents a measurable user-visible speedup for searches over large scrollback buffers, but CHANGELOG.md still has an empty Unreleased section, so the improvement will be absent from the next release notes. Add an Unreleased entry describing the optimized terminal search behavior.
AGENTS.md reference: AGENTS.md:L183-L183
Useful? React with 👍 / 👎.
💡 What:
Hoist target character's ASCII lower/upper conversions outside the search loop and use them for a fast short-circuit check against ASCII haystack characters. The fast-path now explicitly ensures the needle is ASCII before applying the shortcut to safely handle characters (like the Kelvin sign
\u{212A}) that case-fold to ASCII.🎯 Why:
The previous case-insensitive string matching fallback (
chars_eq_ignore_case) builds string iterators or calls the heavyto_lowercasefunction. For millions of characters in a scrollback buffer, the vast majority are ASCII non-matches that can be rejected cheaply without falling back to full case-insensitive Unicode matching.📊 Impact:
Significantly reduces redundant allocations and method calls for every non-matching character, resulting in a measurable speedup for huge scrollback queries.
🔬 Measurement:
Running
FORKTTY_BENCH_SCROLLBACK=100000 cargo test bench_search_on_huge_scrollbackconfirms that the changes are functionally correct, and micro-benchmarks confirm that the fast path executes at a fraction of the cost of the old check.PR created automatically by Jules for task 2127360482330049597 started by @Lucenx9