perf: memoize satellite-connector binary-path probes (#448)#459
Merged
Conversation
Addresses #448's binary-probe half (the per-call subprocess re-probing). memory_mesh, memtrace, and vaultmem each re-resolved their CLI binary on every call by running a `--version` subprocess against several candidate paths — and the resolvers are called per query / per repo / per project. vaultmem's fallback runs `npx -y vault-mem-mcp --version`, which can hit the npm registry (10s timeout), and was invoked on the availability check AND once per project per render (≥ 1 + N probes). Memoize each resolver's result per process — including the not-installed result, which is the common case in most environments, so a missing binary is probed once instead of on every call. (_vaultmem_binary splits into a cached wrapper + _vaultmem_binary_uncached.) conftest clears the three caches between tests so a test mocking the binary present/absent isn't served a stale result. Deferred (the larger half of #448): reusing a persistent MCP subprocess across queries instead of spawn + handshake + teardown per call (template: MimirConnector's persistent Popen singleton). That needs the real external binaries to integration-test and is left as follow-up. Tests (new test_connector_binary_cache.py): memtrace probe runs once then caches (positive), a not-installed result is cached (no re-probe), and memorymesh + vaultmem both memoize their None result. 3 passed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
Addresses #448's binary-probe half (the per-call subprocess re-probing).
Problem
memory_mesh,memtrace, andvaultmemeach re-resolved their CLI binary on every call by running a--versionsubprocess against several candidate paths — and the resolvers are called per query / per repo / per project. vaultmem's fallback runsnpx -y vault-mem-mcp --version, which can hit the npm registry (10s timeout), and was invoked on the availability check and once per project per render (≥ 1 + N probes).Fix
Memoize each resolver's result per process — including the not-installed result (the common case in most environments), so a missing binary is probed once instead of on every call.
_vaultmem_binarysplits into a cached wrapper +_vaultmem_binary_uncached.conftestclears the three caches between tests so a test mocking the binary present/absent isn't served a stale result.Deferred (the larger half of #448)
Reusing a persistent MCP subprocess across queries instead of spawn + handshake + teardown per call (template:
MimirConnector's persistentPopensingleton). That needs the real external binaries to integration-test — left as follow-up.Tests
New
test_connector_binary_cache.py: memtrace probe runs once then caches (positive); a not-installed result is cached (no re-probe); memorymesh + vaultmem both memoize theirNoneresult. 3 passed.🤖 Generated with Claude Code