⚡ Bolt: [Performance] Auto-generated fast keyword lookup for Lexer - #40
⚡ Bolt: [Performance] Auto-generated fast keyword lookup for Lexer#40srimon12 wants to merge 1 commit into
Conversation
Replaced dynamic map lookup in the lexer with a fast, auto-generated switch statement to improve parsing performance by eliminating string allocations and runtime map lookups. 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 code generator (gen_keywords.go) that produces an optimized, switch-based keyword lookup function (keywords_fast.go), wires generation via a go:generate directive, replaces the lexer's map-based keyword lookup with the fast lookup in readIdentifier, and adds benchmarks and a generated note file. ChangesLexer keyword lookup optimization
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Lexer as Lexer.readIdentifier
participant FastLookup as lookupKeywordFast
participant Switch as len(s) switch cases
Lexer->>FastLookup: call with candidate segment
FastLookup->>Switch: switch on len(s)
Switch-->>FastLookup: matching case block (or none)
alt keyword matched
FastLookup-->>Lexer: return TokenKind, true
else no match
FastLookup-->>Lexer: return 0, false
Lexer-->>Lexer: fallback to TokenKindIdentifier
end
🚥 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/lexer/lexer.go (1)
7-106: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the unused
keywordstable —internal/lexer/lexer.gono longer reads this map, and the generator already keeps its own private keyword list ininternal/lexer/gen_keywords.go. Keeping both copies just adds dead code and drift risk.🤖 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/lexer.go` around lines 7 - 106, Remove the dead keywords table from the lexer package: `internal/lexer/lexer.go` no longer uses the `keywords` map, and the generator-owned list in `internal/lexer/gen_keywords.go` is the single source of truth. Delete the unused `keywords` declaration and any now-redundant related references so the lexer stays consistent with the generated keyword handling.
🤖 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/gen_keywords.go`:
- Around line 13-113: The keyword map in gen_keywords.go is duplicating the
canonical set from lexer.go, so keep only one source of truth and generate both
from shared data. Refactor the keyword definition into a shared structure or
generator input used by both the lexer keyword lookup and the generated keywords
output, and update the generation flow so keywords are not manually maintained
in two places.
---
Outside diff comments:
In `@internal/lexer/lexer.go`:
- Around line 7-106: Remove the dead keywords table from the lexer package:
`internal/lexer/lexer.go` no longer uses the `keywords` map, and the
generator-owned list in `internal/lexer/gen_keywords.go` is the single source of
truth. Delete the unused `keywords` declaration and any now-redundant related
references so the lexer stays consistent with the generated keyword handling.
🪄 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: 4e2bee98-e5fc-4f4a-86ba-00f0c4ceefc1
📒 Files selected for processing (6)
.jules/bolt.mdinternal/lexer/gen_keywords.gointernal/lexer/generate.gointernal/lexer/keywords_fast.gointernal/lexer/lexer.gointernal/lexer/lexer_bench_test.go
| // keywords must be kept in sync with lexer.go | ||
| var keywords = map[string]string{ | ||
| "GEO_BBOX": "TokenKindGeoBbox", | ||
| "GEO_RADIUS": "TokenKindGeoRadius", | ||
| "VALUES_COUNT": "TokenKindValuesCount", | ||
| "HAS_VECTOR": "TokenKindHasVector", | ||
| "BOOST": "TokenKindBoost", | ||
| "DEFAULTS": "TokenKindDefaults", | ||
| "CASE": "TokenKindCase", | ||
| "WHEN": "TokenKindWhen", | ||
| "THEN": "TokenKindThen", | ||
| "ELSE": "TokenKindElse", | ||
| "END": "TokenKindEnd", | ||
| "INSERT": "TokenKindInsert", | ||
| "INTO": "TokenKindInto", | ||
| "COLLECTION": "TokenKindCollection", | ||
| "VALUES": "TokenKindValues", | ||
| "USING": "TokenKindUsing", | ||
| "MODEL": "TokenKindModel", | ||
| "HYBRID": "TokenKindHybrid", | ||
| "DENSE": "TokenKindDense", | ||
| "SPARSE": "TokenKindSparse", | ||
| "RERANK": "TokenKindRerank", | ||
| "EXACT": "TokenKindExact", | ||
| "WITH": "TokenKindWith", | ||
| "AS": "TokenKindAs", | ||
| "ACORN": "TokenKindAcorn", | ||
| "QUANTIZE": "TokenKindQuantize", | ||
| "SCALAR": "TokenKindScalar", | ||
| "BINARY": "TokenKindBinary", | ||
| "PRODUCT": "TokenKindProduct", | ||
| "TURBO": "TokenKindTurbo", | ||
| "BITS": "TokenKindBits", | ||
| "QUANTILE": "TokenKindQuantile", | ||
| "ALWAYS": "TokenKindAlways", | ||
| "RAM": "TokenKindRam", | ||
| "HNSW": "TokenKindHnsw", | ||
| "VECTORS": "TokenKindVectors", | ||
| "OPTIMIZERS": "TokenKindOptimizers", | ||
| "PARAMS": "TokenKindParams", | ||
| "DISABLED": "TokenKindDisabled", | ||
| "CREATE": "TokenKindCreate", | ||
| "ALTER": "TokenKindAlter", | ||
| "DROP": "TokenKindDrop", | ||
| "SHOW": "TokenKindShow", | ||
| "COLLECTIONS": "TokenKindCollections", | ||
| "SELECT": "TokenKindSelect", | ||
| "SCROLL": "TokenKindScroll", | ||
| "AFTER": "TokenKindAfter", | ||
| "RECOMMEND": "TokenKindRecommend", | ||
| "QUERY": "TokenKindQuery", | ||
| "NEAREST": "TokenKindNearest", | ||
| "CONTEXT": "TokenKindContext", | ||
| "DISCOVER": "TokenKindDiscover", | ||
| "PAIRS": "TokenKindPairs", | ||
| "TARGET": "TokenKindTarget", | ||
| "ORDER": "TokenKindOrder", | ||
| "ASC": "TokenKindAsc", | ||
| "DESC": "TokenKindDesc", | ||
| "LIMIT": "TokenKindLimit", | ||
| "GROUP": "TokenKindGroup", | ||
| "BY": "TokenKindBy", | ||
| "GROUP_SIZE": "TokenKindGroupSize", | ||
| "STRATEGY": "TokenKindStrategy", | ||
| "DELETE": "TokenKindDelete", | ||
| "UPDATE": "TokenKindUpdate", | ||
| "SET": "TokenKindSet", | ||
| "VECTOR": "TokenKindVector", | ||
| "PAYLOAD": "TokenKindPayload", | ||
| "FROM": "TokenKindFrom", | ||
| "WHERE": "TokenKindWhere", | ||
| "ID": "TokenKindId", | ||
| "INDEX": "TokenKindIndex", | ||
| "ON": "TokenKindOn", | ||
| "FOR": "TokenKindFor", | ||
| "TYPE": "TokenKindType", | ||
| "AND": "TokenKindAnd", | ||
| "OR": "TokenKindOr", | ||
| "NOT": "TokenKindNot", | ||
| "IN": "TokenKindIn", | ||
| "BETWEEN": "TokenKindBetween", | ||
| "IS": "TokenKindIs", | ||
| "NULL": "TokenKindNull", | ||
| "EMPTY": "TokenKindEmpty", | ||
| "MATCH": "TokenKindMatch", | ||
| "ANY": "TokenKindAny", | ||
| "PHRASE": "TokenKindPhrase", | ||
| "OFFSET": "TokenKindOffset", | ||
| "SCORE": "TokenKindScore", | ||
| "THRESHOLD": "TokenKindThreshold", | ||
| "LOOKUP": "TokenKindLookup", | ||
| "COSINE": "TokenKindCosine", | ||
| "DOT": "TokenKindDot", | ||
| "EUCLID": "TokenKindEuclid", | ||
| "MANHATTAN": "TokenKindManhattan", | ||
| "PREFETCH": "TokenKindPrefetch", | ||
| "FUSION": "TokenKindFusion", | ||
| "SAMPLE": "TokenKindSample", | ||
| "RELEVANCE": "TokenKindRelevance", | ||
| "FEEDBACK": "TokenKindFeedback", | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether lexer.go still declares its own `keywords` map and whether it's used anywhere,
# and confirm the two keyword lists are actually kept in sync.
rg -n 'var keywords' internal/lexer/*.go
rg -n '\bkeywords\[' internal/lexer/*.go
rg -n '\blookupKeyword\b' internal/lexer/*.goRepository: srimon12/qql-go
Length of output: 343
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant files and inspect the keyword declarations plus generator context.
ast-grep outline internal/lexer/lexer.go --view expanded || true
ast-grep outline internal/lexer/gen_keywords.go --view expanded || true
printf '\n--- lexer.go (top) ---\n'
sed -n '1,180p' internal/lexer/lexer.go
printf '\n--- gen_keywords.go (top/middle) ---\n'
sed -n '1,220p' internal/lexer/gen_keywords.goRepository: srimon12/qql-go
Length of output: 11540
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether the generator or lexer references any shared source of truth for keywords.
rg -n 'shared|source of truth|generate|go:generate|keywords' internal/lexer README.md .github -g '!**/vendor/**' || trueRepository: srimon12/qql-go
Length of output: 1249
Single source of truth for keywords
internal/lexer/gen_keywords.go duplicates the keyword set already defined in internal/lexer/lexer.go, so the two will drift unless both are updated together. Move the canonical list into shared data and generate both outputs from it.
🤖 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/gen_keywords.go` around lines 13 - 113, The keyword map in
gen_keywords.go is duplicating the canonical set from lexer.go, so keep only one
source of truth and generate both from shared data. Refactor the keyword
definition into a shared structure or generator input used by both the lexer
keyword lookup and the generated keywords output, and update the generation flow
so keywords are not manually maintained in two places.
1 participant
💡 What:
Replaced the dynamic map-based keyword lookup in the lexer (
lookupKeyword) with an auto-generated, highly optimized switch statement (lookupKeywordFast). It groups keywords by length and uses unrolled, case-insensitive byte comparisons. Addedgo:generateintegration to easily rebuild the optimized switch.🎯 Why:
Lexical analysis is in the hot path of query processing. The previous
lookupKeywordmethod performed allocations for case conversion and utilized dynamic map lookups. This optimization removes those heap allocations and speeds up the tokenization phase.📊 Impact:
The lexer's
Tokenizemethod execution time improved by roughly 13% (from ~1224 ns/op to ~1067 ns/op). The keyword lookup itself got a ~5x speedup, going from ~125 ns/op down to ~25 ns/op. Zero heap allocations are maintained.🔬 Measurement:
go test -bench . ./internal/lexer/... -benchmemBenchmarkLookupKeywordFastclocking in at ~25 ns/op vsBenchmarkLookupKeywordat ~125 ns/op.go test ./...PR created automatically by Jules for task 17796768965943723457 started by @srimon12
Summary by CodeRabbit