⚡ Bolt: Lexer keyword lookup optimization - #41
Conversation
Replaced map-based keyword lookup with an auto-generated length-and-character-based switch statement to eliminate string allocations and hashing overhead during lexical analysis. Co-authored-by: srimon12 <33979603+srimon12@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. |
📝 WalkthroughWalkthroughThis PR adds a Go generator ( ChangesFast Keyword Lookup
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Developer
participant generate.go
participant lexer.go
participant keyword_lookup.go
Developer->>generate.go: go run generate.go
generate.go->>lexer.go: read keywords map literal
generate.go->>generate.go: group keywords by length and first char
generate.go->>keyword_lookup.go: write lookupKeywordFast source
generate.go->>keyword_lookup.go: gofmt -w
lexer.go->>keyword_lookup.go: call lookupKeywordFast(word)
keyword_lookup.go-->>lexer.go: return TokenKind, matched
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
internal/lexer/generate.go (1)
20-43: 📐 Maintainability & Code Quality | 🔵 TrivialFragile text-based parsing of the keywords map.
Extracting the map via
bytes.Index/line-splitting/SplitN(":")is brittle: it assumes one entry per line, no trailing comments, and an exact literal signature for the map declaration. Any future inline comment ("ID": TokenKindId, // ambiguous) or reformatted map would either silently mis-parse the value (leaving stray text appended tokindStr, likely surfacing only as a compile error in the generated file) or corrupt the extracted keyword set without a clear diagnostic. Since this is the single source of truth feeding the runtime lookup, consider parsing withgo/parser/go/astto walk the actualast.CompositeLitforkeywords, which is robust to formatting/comments and gives you typed key/value pairs directly.🤖 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 `@internal/lexer/generate.go` around lines 20 - 43, The keyword extraction in generate.go is doing fragile text parsing of the keywords map, so it can break or misread entries when formatting, comments, or the declaration shape changes. Update the logic in the generator to locate and read the keywords composite literal via go/parser and go/ast instead of bytes.Index, Split, and SplitN, and derive the key/value pairs directly from the ast.CompositeLit for keywords. Keep the existing byLength grouping, but populate it from typed AST nodes so the generator remains stable across formatting and inline comments.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@internal/lexer/generate.go`:
- Around line 20-21: The marker lookup in generate.go can panic if "var keywords
= map[string]TokenKind{" is not present, because bytes.Index may return -1
before slicing content[startIdx:]. Update the keyword extraction logic around
the startIdx/endIdx calculation to validate that both markers are found before
using them, and return a clear error from the generator instead of slicing
invalid bounds.
- Around line 14-125: Add a drift/parity check between the source keywords map
and the generated fast lookup so manual edits can’t silently desync
tokenization. Use the existing generate flow in generate.go and the lexer-side
symbols keywords and lookupKeywordFast to create either a test that iterates
every keyword entry and verifies the generated lookup returns the same
TokenKind, or a CI check that runs go generate ./... and fails on a dirty diff.
Ensure the check is wired where it will run automatically with the lexer tests.
---
Nitpick comments:
In `@internal/lexer/generate.go`:
- Around line 20-43: The keyword extraction in generate.go is doing fragile text
parsing of the keywords map, so it can break or misread entries when formatting,
comments, or the declaration shape changes. Update the logic in the generator to
locate and read the keywords composite literal via go/parser and go/ast instead
of bytes.Index, Split, and SplitN, and derive the key/value pairs directly from
the ast.CompositeLit for keywords. Keep the existing byLength grouping, but
populate it from typed AST nodes so the generator remains stable across
formatting and inline comments.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ad709c91-fde0-4ee8-9dee-c612f61c1eb1
📒 Files selected for processing (4)
.jules/bolt.mdinternal/lexer/generate.gointernal/lexer/keyword_lookup.gointernal/lexer/lexer.go
| func main() { | ||
| content, err := os.ReadFile("lexer.go") | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
|
|
||
| startIdx := bytes.Index(content, []byte("var keywords = map[string]TokenKind{")) | ||
| endIdx := bytes.Index(content[startIdx:], []byte("}")) + startIdx | ||
|
|
||
| mapLines := bytes.Split(content[startIdx:endIdx], []byte("\n")) | ||
|
|
||
| byLength := make(map[int]map[string]string) | ||
| for _, line := range mapLines[1:] { | ||
| lineStr := strings.TrimSpace(string(line)) | ||
| if lineStr == "" || strings.HasPrefix(lineStr, "//") { | ||
| continue | ||
| } | ||
| parts := strings.SplitN(lineStr, ":", 2) | ||
| if len(parts) != 2 { | ||
| continue | ||
| } | ||
| kwStr := strings.Trim(strings.TrimSpace(parts[0]), `"`) | ||
| kindStr := strings.TrimSuffix(strings.TrimSpace(parts[1]), ",") | ||
|
|
||
| l := len(kwStr) | ||
| if byLength[l] == nil { | ||
| byLength[l] = make(map[string]string) | ||
| } | ||
| byLength[l][kwStr] = kindStr | ||
| } | ||
|
|
||
| var buf bytes.Buffer | ||
| buf.WriteString(`// Code generated by go generate; DO NOT EDIT. | ||
| package lexer | ||
|
|
||
| func lookupKeywordFast(s string) (TokenKind, bool) { | ||
| if len(s) == 0 { | ||
| return 0, false | ||
| } | ||
| switch len(s) { | ||
| `) | ||
|
|
||
| var lengths []int | ||
| for l := range byLength { | ||
| lengths = append(lengths, l) | ||
| } | ||
| sort.Ints(lengths) | ||
|
|
||
| for _, l := range lengths { | ||
| buf.WriteString(fmt.Sprintf("\tcase %d:\n", l)) | ||
| buf.WriteString("\t\tswitch s[0] | 0x20 {\n") | ||
|
|
||
| byFirstChar := make(map[byte]map[string]string) | ||
| for kw, kind := range byLength[l] { | ||
| fc := kw[0] | 0x20 | ||
| if byFirstChar[fc] == nil { | ||
| byFirstChar[fc] = make(map[string]string) | ||
| } | ||
| byFirstChar[fc][kw] = kind | ||
| } | ||
|
|
||
| var chars []int | ||
| for c := range byFirstChar { | ||
| chars = append(chars, int(c)) | ||
| } | ||
| sort.Ints(chars) | ||
|
|
||
| for _, cInt := range chars { | ||
| c := byte(cInt) | ||
| buf.WriteString(fmt.Sprintf("\t\tcase '%c':\n", c)) | ||
|
|
||
| var sortedKws []string | ||
| for kw := range byFirstChar[c] { | ||
| sortedKws = append(sortedKws, kw) | ||
| } | ||
| sort.Strings(sortedKws) | ||
|
|
||
| for _, kw := range sortedKws { | ||
| kind := byFirstChar[c][kw] | ||
| buf.WriteString("\t\t\tif ") | ||
| for i := 1; i < len(kw); i++ { | ||
| if i > 1 { | ||
| buf.WriteString(" && ") | ||
| } | ||
| lc := kw[i] | 0x20 | ||
| if lc >= 'a' && lc <= 'z' { | ||
| buf.WriteString(fmt.Sprintf("(s[%d] | 0x20) == '%c'", i, lc)) | ||
| } else { | ||
| buf.WriteString(fmt.Sprintf("s[%d] == '%c'", i, kw[i])) | ||
| } | ||
| } | ||
| if len(kw) == 1 { | ||
| buf.WriteString("true") | ||
| } | ||
| buf.WriteString(" {\n") | ||
| buf.WriteString(fmt.Sprintf("\t\t\t\treturn %s, true\n", kind)) | ||
| buf.WriteString("\t\t\t}\n") | ||
| } | ||
| } | ||
| buf.WriteString("\t\t}\n") | ||
| } | ||
|
|
||
| buf.WriteString("\t}\n") | ||
| buf.WriteString("\treturn 0, false\n}\n") | ||
|
|
||
| err = os.WriteFile("keyword_lookup.go", buf.Bytes(), 0644) | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
|
|
||
| exec.Command("gofmt", "-w", "keyword_lookup.go").Run() | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for existing parity test or CI drift-check for the generated keyword lookup
rg -n 'lookupKeywordFast|keywords\b' --type=go internal/lexer
rg -n 'go generate' -g '*.yml' -g '*.yaml' -g 'Makefile' .
fd . .github/workflows 2>/dev/null | xargs -I{} grep -l 'go generate' {} 2>/dev/nullRepository: srimon12/qql-go
Length of output: 587
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== relevant files =='
git ls-files internal/lexer internal/parser .github/workflows Makefile | sed -n '1,200p'
echo
echo '== lexer files outline =='
ast-grep outline internal/lexer/lexer.go --view expanded
echo
ast-grep outline internal/lexer/generate.go --view expanded
echo
ast-grep outline internal/lexer/keyword_lookup.go --view expanded
echo
echo '== search for tests / regeneration / drift checks =='
rg -n 'lookupKeywordFast|keywords\b|go generate|generated by go generate|keyword_lookup.go' internal .github/workflows Makefile -g '*.go' -g '*.yml' -g '*.yaml' -g 'Makefile'
echo
echo '== workflow files mentioning generate =='
fd -a . .github/workflows 2>/dev/null | while read -r f; do
echo "--- $f"
rg -n 'go generate|git diff|keyword_lookup|lexer' "$f" || true
done
echo
echo '== focused lexer snippets =='
sed -n '1,120p' internal/lexer/lexer.go
echo
sed -n '1,120p' internal/lexer/keyword_lookup.go
echo
sed -n '1,220p' internal/lexer/generate.goRepository: srimon12/qql-go
Length of output: 2992
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== lexer test file outline =='
ast-grep outline internal/lexer/lexer_test.go --view expanded
echo
echo '== lexer tests mentioning keywords / lookup =='
rg -n 'lookupKeywordFast|keywords|go generate|generated by go generate' internal/lexer/lexer_test.go internal/lexer/*.go
echo
echo '== CI workflow snippets =='
sed -n '1,220p' .github/workflows/ci.yml
echo
sed -n '1,220p' .github/workflows/release.ymlRepository: srimon12/qql-go
Length of output: 8879
Add a drift check for keyword generation The lexer tests cover current cases, but there’s still no explicit parity check between keywords and lookupKeywordFast. A manual edit to keywords can desync the committed generated file and silently change tokenization. Add a test that compares every entry against the generated lookup, or fail CI when go generate ./... produces a diff.
🤖 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 `@internal/lexer/generate.go` around lines 14 - 125, Add a drift/parity check
between the source keywords map and the generated fast lookup so manual edits
can’t silently desync tokenization. Use the existing generate flow in
generate.go and the lexer-side symbols keywords and lookupKeywordFast to create
either a test that iterates every keyword entry and verifies the generated
lookup returns the same TokenKind, or a CI check that runs go generate ./... and
fails on a dirty diff. Ensure the check is wired where it will run automatically
with the lexer tests.
| startIdx := bytes.Index(content, []byte("var keywords = map[string]TokenKind{")) | ||
| endIdx := bytes.Index(content[startIdx:], []byte("}")) + startIdx |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Guard against marker-not-found before slicing.
If "var keywords = map[string]TokenKind{" isn't found (e.g., after reformatting lexer.go), bytes.Index returns -1, and content[startIdx:] panics with an unhelpful "slice bounds out of range" instead of a clear error.
🛡️ Proposed fix
startIdx := bytes.Index(content, []byte("var keywords = map[string]TokenKind{"))
+ if startIdx == -1 {
+ panic("could not find keywords map declaration in lexer.go")
+ }
endIdx := bytes.Index(content[startIdx:], []byte("}")) + startIdx📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| startIdx := bytes.Index(content, []byte("var keywords = map[string]TokenKind{")) | |
| endIdx := bytes.Index(content[startIdx:], []byte("}")) + startIdx | |
| startIdx := bytes.Index(content, []byte("var keywords = map[string]TokenKind{")) | |
| if startIdx == -1 { | |
| panic("could not find keywords map declaration in lexer.go") | |
| } | |
| endIdx := bytes.Index(content[startIdx:], []byte("}")) + startIdx |
🤖 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 `@internal/lexer/generate.go` around lines 20 - 21, The marker lookup in
generate.go can panic if "var keywords = map[string]TokenKind{" is not present,
because bytes.Index may return -1 before slicing content[startIdx:]. Update the
keyword extraction logic around the startIdx/endIdx calculation to validate that
both markers are found before using them, and return a clear error from the
generator instead of slicing invalid bounds.
1 participant
💡 What: Replaced the
map[string]TokenKindlookup in the Lexer with an auto-generated case-insensitive switch statement (lookupKeywordFast). Added ago generatescript so maintainers can still edit the map as the source of truth.🎯 Why: The previous lookup mechanism caused runtime string allocations and hashing overhead. Replacing this with a structured switch statement matching string lengths and characters (via bitwise operators to coerce lowercase) ensures the lookups do not allocate.
📊 Impact:
Lexer Simple benchmark: ~491ns to ~393ns (20% faster)
Parser Simple benchmark: ~851ns to ~753ns (11% faster)
🔬 Measurement:
Run
go test -bench . -benchmem ./internal/parser/and compareBenchmarkLex_SimpleandBenchmarkParse_Simpleresults.PR created automatically by Jules for task 246312254983283493 started by @srimon12
Summary by CodeRabbit