Skip to content

⚡ Bolt: Optimize remove_numeric_separators to avoid allocation #6

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.

Sign up for GitHub

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

Open
SayanthRock wants to merge 1 commit into
base: main
Choose a base branch
from
+21 −9
Open

⚡ Bolt: Optimize remove_numeric_separators to avoid allocation #6

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .jules/bolt.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
## 2024-05-24 - Optimize remove_numeric_separators
**Learning:** Iterating over `chars()` and collecting them into a `Vec<char>` for simple ASCII manipulation (like removing underscores between digits) causes unnecessary overhead from UTF-8 decoding and intermediate allocations.
**Action:** Use `.as_bytes()` to work directly with the underlying UTF-8 representation when dealing with ASCII characters, and avoid allocations entirely if the character isn't present by checking `.contains('_')` first. Reconstruct the string using `unsafe { String::from_utf8_unchecked(output) }` since removing ASCII characters from a valid UTF-8 string is inherently safe and preserves validity.
27 changes: 18 additions & 9 deletions compiler/rockql-sql/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,22 +160,31 @@ fn normalize_expression(expression: &str, _dialect: Dialect) -> String {
}

fn remove_numeric_separators(value: &str) -> String {
let characters = value.chars().collect::<Vec<_>>();
let mut output = String::with_capacity(value.len());
// ⚡ Bolt: Fast path to avoid allocation and iteration when there are no separators
if !value.contains('_') {
return value.to_owned();
Comment on lines +163 to +165

@coderabbitai coderabbitai Bot Aug 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Clarify the fast-path optimization boundaries.

The no-separator branch still scans value and allocates the returned String. It avoids UTF-8 character decoding and intermediate output storage, not all iteration or allocation.

  • compiler/rockql-sql/src/lib.rs#L163-L165: update the code comment to describe avoided intermediate work.
  • .jules/bolt.md#L1-L3: replace “avoid allocations entirely” with “avoid intermediate allocations.”
📍 Affects 2 files
  • compiler/rockql-sql/src/lib.rs#L163-L165 (this comment)
  • .jules/bolt.md#L1-L3
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@compiler/rockql-sql/src/lib.rs` around lines 163 - 165, The fast path comment
overstates the optimization by implying it avoids all iteration and allocation.
Update the comment above the no-separator branch in
compiler/rockql-sql/src/lib.rs:163-165 to state that it avoids UTF-8 character
decoding and intermediate output storage while retaining the contains scan and
returned String allocation; update .jules/bolt.md:1-3 to replace “avoid
allocations entirely” with “avoid intermediate allocations.”

}

for (index, character) in characters.iter().enumerate() {
let is_numeric_separator = *character == '_'
// ⚡ Bolt: Iterate over bytes instead of chars.
// '_' and ASCII digits are exactly 1 byte in UTF-8, so we can avoid decoding overhead.
let bytes = value.as_bytes();
let mut output = Vec::with_capacity(value.len());

for (index, &byte) in bytes.iter().enumerate() {
let is_numeric_separator = byte == b'_'
&& index > 0
&& index + 1 < characters.len()
&& characters[index - 1].is_ascii_digit()
&& characters[index + 1].is_ascii_digit();
&& index + 1 < bytes.len()
&& bytes[index - 1].is_ascii_digit()
&& bytes[index + 1].is_ascii_digit();

if !is_numeric_separator {
output.push(*character);
output.push(byte);
}
}

output
// SAFETY: We only remove ASCII '_' characters, which are exactly 1 byte.
// Removing an ASCII character from a valid UTF-8 string preserves its validity.
unsafe { String::from_utf8_unchecked(output) }
}

fn normalize_keywords(value: &str) -> String {
Expand Down