-
Notifications
You must be signed in to change notification settings - Fork 0
β‘ Bolt: [performance improvement] Optimize terminal scrollback search fast-path #413
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| ## 2024-03-12 - [Optimizing Terminal Scrollback Search] | ||
| **Learning:** [In Rust, optimizing text search hot loops (like terminal scrollback searches) can be achieved by extracting the first-character comparison out of the iterator chain and utilizing an ASCII fast-path. This avoids the overhead of setting up slice iterators, zipping, closures, and invoking heavy `chars_eq_ignore_case` functions for the vast majority of non-matches.] | ||
| **Action:** [When implementing case-insensitive character matching hot loops in Rust, hoist the target character's ASCII lower/uppercase conversions outside the loop, compare the iterated haystack characters against these bounds (`h != first_lower && h != first_upper`), but ONLY short-circuit reject if the haystack character is also ASCII (`h.is_ascii()`), since non-ASCII characters can map to ASCII when case-folded. Use full Unicode case-insensitive matching for the fallback.] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -75,11 +75,24 @@ fn for_each_char_match_start( | |
| return; | ||
| } | ||
| let first_needle = needle[0]; | ||
| let first_is_ascii = first_needle.is_ascii(); | ||
| let first_lower = first_needle.to_ascii_lowercase(); | ||
| let first_upper = first_needle.to_ascii_uppercase(); | ||
|
Comment on lines
+78
to
+80
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This optimization is explicitly intended to make terminal scrollback search noticeably more responsive, but the commit does not add an entry under AGENTS.md reference: AGENTS.md:L183-L183 Useful? React with πΒ / π. |
||
|
|
||
| let mut index = 0; | ||
| while index + needle.len() <= haystack.len() { | ||
| let h = haystack[index]; | ||
| // Fast-path: short-circuit the full substring check if the first character | ||
| // doesn't match, avoiding iterator overhead in the common case. | ||
| if !chars_eq_ignore_case(haystack[index], first_needle) { | ||
| // doesn't match, avoiding function call overhead in the common case. | ||
| // We only short-circuit if both characters are ASCII. Non-ASCII characters | ||
| // (like the Kelvin sign) can fold to ASCII, so they must fall back to | ||
| // full Unicode comparison. | ||
| if first_is_ascii && h.is_ascii() && h != first_lower && h != first_upper { | ||
| index += 1; | ||
| continue; | ||
| } | ||
|
|
||
| if !chars_eq_ignore_case(h, first_needle) { | ||
| index += 1; | ||
| continue; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.julesroot bucketThis 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:2342records as previously cleaned up.AGENTS.md reference: AGENTS.md:L131-L131
Useful? React with πΒ / π.