Repository navigation
perf: take general-purpose parsing and query costs off the interactive hot paths - #93
Merged
Merged
Conversation
Cover the four hot paths about to be optimized: per-keystroke history prefix search, built-in completion spec parsing, config load (clean and recovery), and per-line command parsing. Committed before the changes so before/after comparisons run the same benchmark code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012axhLiAUtKxRS6TaMux5UM
The ghost-text suggestion function runs SearchByPrefix on every keystroke, and the backing SQL (GLOB + GROUP BY + MAX(timestamp)) has to aggregate every historical occurrence of every matching command. Under the default unlimited-retention config that cost grows with history size: ~0.9ms per keystroke at 10k rows and ~15ms at 100k rows in benchmarks. Load the distinct successful commands once in the background at store open, keep them in a slice sorted by command text, and answer prefix lookups with a binary search plus a scan of the matching range. Recency is a session-local sequence number, so no timestamp parsing is needed and commands run this session always outrank loaded ones. Store.Add updates the index in place; until the initial load lands, SearchByPrefix keeps answering from SQL, and a load failure only ever costs speed. In-memory databases now cap the connection pool at one: a plain :memory: DSN gives each pooled connection its own empty database, and the background load is the first thing to make the pool open a second one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012axhLiAUtKxRS6TaMux5UM
builtinPluginSpecs re-parsed the docker and hash-builtins TOML through the full decoder on every handler-table rebuild: shell startup and every completions reload. The specs are compile-time constants and immutable once compiled, so parse them under sync.OnceValue and hand rebuilds a cloned slice sharing the same spec instances. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012axhLiAUtKxRS6TaMux5UM
Every Load parsed config.toml twice in full: once into the Config struct and once into a raw map just to pull out the [agent.<name>] tables, and the recovery path parsed it a third time even though it already held a raw map. Decode only the [agent] key on the happy path so the decoder skips every other section, and hand the recovery path's existing raw map straight to loadNamedAgents. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012axhLiAUtKxRS6TaMux5UM
Parse and the shell dispatch site allocated map[string]any trace payloads on every dispatched line just for Emit to drop them, since tracing is off in normal sessions. Guard the payload construction behind trace.Enabled, the pattern the completion plugin code already uses. Emitted events are unchanged when tracing is enabled. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012axhLiAUtKxRS6TaMux5UM
gocyclo: the per-branch trace guards pushed Parse past the complexity limit; move emission into traceDetect/traceResult helpers that carry the enabled check themselves, keeping Parse's control flow to the actual parsing. Event payloads per command type are unchanged, and Parse stays allocation-free when tracing is off. gofmt: realign inline comments in prefix_index_test.go. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012axhLiAUtKxRS6TaMux5UM
Store.Close returned while the background prefix-index load could still hold a live SQLite connection: db.Close only reaps a busy connection once its query finishes, so the load's WAL activity kept touching the database files after Close. Under go test this raced t.TempDir cleanup into 'directory not empty' failures (reproducible locally within ~300 runs of TestStore_CreateAndAdd). Run the load under a context that Close cancels, and have Close wait for the goroutine before closing the pool. Also rename a local that the new context import shadowed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012axhLiAUtKxRS6TaMux5UM
Ghost-text suggestions now come from a per-session in-memory index, so commands run in another concurrently running shell only appear after a restart. Plan is an optional refresh through the upcoming hook system (after-execution hook re-running the index load); install already merges a reload with live session entries, so only the trigger is missing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012axhLiAUtKxRS6TaMux5UM
tfcace
added a commit
that referenced
this pull request
Aug 1, 2026
…e hot paths (#93) Applies the idea behind uv's custom lockfile parser (astral-sh/uv#20648): stop paying a general-purpose engine on a hot path with a known input shape. - history: SearchByPrefix ran a GLOB + GROUP BY + MAX(timestamp) query on every keystroke, aggregating every historical occurrence of every match. Distinct successful commands now load once in the background at store open into a text-sorted in-memory slice; lookup is a binary search plus range scan, Store.Add updates the index in place, SQL stays the fallback until the load lands. In-memory SQLite DSNs cap the pool at one conn, since each pooled :memory: conn gets its own empty database. - completion: built-in docker/hash-builtins TOML specs are compile-time constants; parse once per process under sync.OnceValue instead of on every handler-table rebuild. - config: Load parsed config.toml twice in full, three times on recovery. Happy path now decodes only the agent key; recovery reuses its raw map. - parser: map[string]any trace payloads were built per line for Emit to discard when tracing is off; now behind trace.Enabled guards. Emitted events are unchanged when tracing is on. Benchmarks: linux/amd64, Intel Xeon 2.80GHz, go1.25.0, -benchmem. Committed first in 8687faa so before/after run identical benchmark code. go test -run '^$' -bench . -benchmem \ ./internal/history/ ./internal/completion/ \ ./internal/config/ ./internal/parser/ history.SearchByPrefix — per keystroke ----------------------------------------------------- case before after speedup 10k rows, dense prefix 845 µs 6.2 µs 136x 10k rows, narrow prefix 453 µs 3.8 µs 120x 10k rows, miss 32.9 µs 86 ns 383x 100k rows, dense prefix 15.2 ms 57 µs 268x 100k rows, narrow prefix 7.9 ms 40 µs 196x 100k rows, miss 32.7 µs 107 ns 307x completion — startup, `completions reload` ---------------------------------------------------------------- benchmark before after speedup allocs builtinPluginSpecs 138 µs 63 ns ~2200x 552 -> 1 NewPluginCompleter 137 µs 1.6 µs 86x 561 -> 10 config.Load — startup ---------------------------------------------------------------- case before after speedup allocs clean 44.0 µs 39.2 µs 1.12x -46% recovery 154 µs 131 µs 1.18x -21% parser.Parse — per dispatched line ---------------------------------------------------------------- case before after speedup allocs regular command 699 ns 59 ns 11.8x 7 -> 0 ?? prefix 854 ns 18 ns 46.7x 9 -> 0 pipe | ?? 1295 ns 31 ns 42.0x 10 -> 0 inline ?? 1074 ns 75 ns 14.4x 11 -> 0 The history before-numbers scale with table size (0.85 ms -> 15.2 ms from 10k to 100k rows, i.e. visible typing lag); the after-numbers scale only with the number of distinct matching commands. config.Load is modest by design — it runs once at startup, so the win is not parsing the same file two-to-three times. New tests cover the prefix index: live-vs-loaded recency merge, top-k ordering, literal prefix matching, failed-command exclusion, SQL fallback before load, and serving with the database closed. go test ./... passes; touched packages pass with -race. golangci-lint v2.8.0: 0 issues. e2e is excluded — it doesn't compile on the base commit either (ScenarioMockTransport predates agent.Transport's AvailableModels).
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
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.
Applies the idea behind uv's custom lockfile parser (astral-sh/uv#20648) — stop paying a general-purpose engine on a hot path with a known input shape — to hash's interactive paths. Four independent changes, one commit each, with benchmarks committed first (
8687faa) so before/after run identical benchmark code.Changes
perf(history)—SearchByPrefixruns on every keystroke for ghost-text suggestions, backed by aGLOB+GROUP BY+MAX(timestamp)query that aggregates every historical occurrence of every matching command; with the default unlimited retention that cost grows with history size. Distinct successful commands now load once in the background at store open into a text-sorted in-memory slice; a lookup is a binary search plus a range scan,Store.Addupdates the index in place, and SQL remains the fallback until the load lands (a load failure only costs speed). Side fix: in-memory SQLite DSNs cap the pool at one connection, since each pooled:memory:connection gets its own empty database and the background load is the first concurrent pool user.perf(completion)— the built-in docker/hash-builtins TOML specs were re-parsed on every handler-table rebuild (startup and eachcompletions reload); they're compile-time constants, now parsed once per process undersync.OnceValue.perf(config)—Loadparsedconfig.tomltwice in full (struct + whole-file raw map just for[agent.<name>]tables) and the recovery path a third time. The happy path now decodes only theagentkey; recovery reuses the raw map it already holds.perf(parser)—Parseand the shell dispatch site builtmap[string]anytrace payloads on every line forEmitto discard when tracing is off. Payload construction now sits behindtrace.Enabledguards; emitted events are unchanged when tracing is on.Benchmarks
Linux amd64, Intel Xeon 2.80GHz, go1.25.0,
-benchmem. Reproduce with:History:
SearchByPrefix(per keystroke)The before numbers scale with table size (0.85 ms → 15.2 ms from 10k to 100k rows — visible typing lag); the after numbers scale only with the number of distinct matching commands.
Completion: built-in plugin specs (startup,
completions reload)builtinPluginSpecsNewPluginCompleterConfig:
Load(startup)The modest one by design: it runs once at startup, so this is mostly about not parsing the same file two-to-three times.
Parser:
Parse(per dispatched line)??prefix| ????Testing
go test ./...passes; touched packages also pass with-race(three consecutive runs forinternal/history).golangci-lint run(v2.8.0, CI config): 0 issues.e2epackage doesn't compile on the base commit either (ScenarioMockTransportpredatesagent.Transport'sAvailableModelsmethod), so it's excluded from verification here.🤖 Generated with Claude Code
https://claude.ai/code/session_012axhLiAUtKxRS6TaMux5UM