Optimize per-keystroke hot path: precompiled regexes, memoization, render caching#115
Open
simeji wants to merge 10 commits into
Open
Optimize per-keystroke hot path: precompiled regexes, memoization, render caching#115simeji wants to merge 10 commits into
simeji wants to merge 10 commits into
Conversation
Add benchmarks covering the per-keystroke hot path: query validation, keyword parsing, JSON filtering (legacy and JMESPath), pretty-printing, suggestion candidates, terminal cell conversion, and key-line search. Fixtures are generated deterministically in code (no testdata files).
validate(), GetKeywords(), isJMESPathQuery(), getFilteredDataLegacy(), getItem(), and Suggestion.Get() compiled their regexes on every call — up to ~10 compilations per keystroke. Compile them once at package init, following the existing reWildcardFieldTyping/reWildcardIndexed pattern. The two literal-only alternations in isJMESPathQuery become plain strings.Contains checks (exactly equivalent).
Suggestion prefix patterns are built from user input, so they cannot be precompiled. Memoize successful compiles in a sync.Map keyed by pattern (bounded by distinct keystrokes per session); failed compiles keep the exact regexp.Compile error behavior. The keyword is intentionally NOT QuoteMeta'd where it wasn't before, preserving keyword-as-regex matching. drawCandidates matched the selected candidate with a per-frame regex of literal parts only ([[:space:]] can only match the plain spaces the rows are built with), so a strings.Index search is exactly equivalent.
The engine calls GetPretty once per frame even when the query is unchanged (scrolling, candidate cycling, cursor moves), re-running the filter and re-marshaling the entire result each time. origin/originData are write-once, so results are pure functions of (query, confirm): a single-entry memo makes repeat frames free. evalJMESPath gets a small bounded cache as well — one keystroke can evaluate the same expression several times via evalBaseExpr and the suggestion paths, each round-tripping through json.Marshal + simplejson.NewJson. Cached results are safe to share: all callers only read the returned simplejson values and reassign (never mutate) the slices.
getContents pretty-printed the filtered JSON and immediately discarded the string when keymode is active. Call GetFilteredData directly in that branch; the marshal error was already ignored at this call site.
getCurrentKeys built two slices and used a strings.Builder per dotted key; build one preallocated slice, sort raw keys, escape in place (ordering stays defined on the unescaped names). rowsToCells rejoined all rows into one string every frame to feed the jsoncolor formatter even though the engine already had the exact pretty string — pass it through as TerminalDrawAttributes.ContentsRaw. The monochrome fallback now preallocates its cell slices. highlightCandidateKey allocated rune slices just to count runes; use utf8.RuneCountInString and pre-size the row builder.
Scrolling and candidate cycling redraw identical content, but every frame re-ran the jsoncolor formatter over the whole document. Cache the converted cells keyed by the raw content string and reuse them when the content is unchanged. Only the outer slice is copied per frame: Draw replaces rows with highlighted copies (highlightCandidateKey is copy-on-write, pinned by TestHighlightCandidateKeyOriginalUnmodified), and the row slices themselves are never mutated.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f5783352a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The eval cache could retain up to 64 full result trees across query changes, multiplying memory usage on large inputs. Clear it whenever the (query, confirm) key misses: all evalJMESPath calls happen inside that computation, so the cache still deduplicates repeated evaluations within one filtering pass while releasing results once the query moves on. Addresses Codex review feedback.
moveCursorWordBackwark/moveCursorWordForward were empty stubs with no callers; drawFuncHelp was superseded by the inline rendering in Draw.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Behavior-preserving performance refactoring of the interactive hot path. Every keystroke used to recompile ~10+ regexes, re-evaluate the JMESPath query, re-marshal the entire filtered document, and re-run the jsoncolor formatter over the whole output — even when nothing changed (scrolling, candidate cycling, cursor moves). This PR removes that redundant work without changing any observable behavior.
Changes
Each commit is independently green (
go build,go vet,go test):go vetfix (unreachablebreak).validate(),GetKeywords(),isJMESPathQuery(),getFilteredDataLegacy(),getItem(),Suggestion.Get()compiled their patterns on every call; they are now package-level vars (following the existingreWildcardIndexedpattern). Two literal-only alternations became plainstrings.Contains.compileCachedhelper instead. The keyword is intentionally not QuoteMeta'd where it wasn't before — keyword-as-regex matching and invalid-pattern fallbacks are preserved exactly.drawCandidatesnow usesstrings.Index(its rows are built with plain spaces, so the[[:space:]]regex was equivalent to a literal search).origin/originDataare write-once, soGetFilteredData/GetPrettyresults are pure functions of(query, confirm)— a single-entry memo makes repeat frames free.evalJMESPathgets a small bounded cache (one keystroke evaluates the same expression several times viaevalBaseExprand the suggestion paths).getCurrentKeys(sort order on raw keys unchanged), pass the pretty string through torowsToCellsinstead of re-joining rows, preallocate monochrome cell slices,utf8.RuneCountInStringinstead of[]runeconversions inhighlightCandidateKey.highlightCandidateKeyis copy-on-write (pinned byTestHighlightCandidateKeyOriginalUnmodified), so row slices are never mutated.Benchmarks (Apple M1, benchstat, count=10)
.(repeat frame).users[*].nameThe headline numbers are cache hits, which is precisely the scroll/candidate-cycle frame profile; cold-path numbers (Validate, GetKeywords, Suggestion*) improve independently of caching.
Behavior verification
GetPretty/Getfor 30+ queries (legacy paths, wildcards, pipes, partial function typing,&field, dotted keys, error paths, repeated/interleaved calls to exercise the caches) against master and this branch: 288/288 output lines byte-identical, except. | keys(@)where master itself is non-deterministic across runs (Go map iteration order); this branch pins one order per session, which is within the unspecified-order semantics.Notes for reviewers
compileCachedusessync.Maponly so tests stay race-free.[]runequery storage,validate()rule order, suggestion keyword-as-regex behavior, candidate ordering, output formatting, and thehighlightCandidateKeycopy-on-write contract.@codex review