-
Notifications
You must be signed in to change notification settings - Fork 0
β‘ Bolt: Optimize search fast-path #411
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-05-18 - [Terminal Search Fast-path Optimization] | ||
| **Learning:** Hoisting the first-character case-conversion out of the search hot loop `for_each_char_match_start` significantly reduces overhead. | ||
| **Action:** In search loops, perform the expensive `.to_ascii_lowercase()`/`.to_ascii_uppercase()` once before the loop, and use it inside the fast path. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -75,11 +75,21 @@ fn for_each_char_match_start( | |
| return; | ||
| } | ||
| let first_needle = needle[0]; | ||
| let first_lower = first_needle.to_ascii_lowercase(); | ||
| let first_upper = first_needle.to_ascii_uppercase(); | ||
|
Comment on lines
+78
to
+79
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.
For users searching large scrollback buffers, this fast path intentionally changes observable search latency, but 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. We can only short-circuit if BOTH the haystack character | ||
| // AND the needle character are ASCII. Non-ASCII characters might case-fold | ||
| // to ASCII (e.g., Kelvin sign U+212A folds to 'k'). | ||
| if h != first_lower && h != first_upper && h.is_ascii() && first_needle.is_ascii() { | ||
| 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.
.julesdirectoryThis creates a new root content category solely for a three-line note about one terminal-search implementation detail. Repository guidance limits new top-level directories to durable categories and directs feature-specific material to remain near its owner, so this generated note should be removed or moved beside the terminal-search code if it provides lasting value.
AGENTS.md reference: AGENTS.md:L131-L131
Useful? React with πΒ / π.