Skip to content

Cache the PyPI version check and resolve version from dist-info - #654

Open
j-sperling wants to merge 4 commits into
zilliztech:mainfrom
j-sperling:perf/session-start-latency
Open

Cache the PyPI version check and resolve version from dist-info#654
j-sperling wants to merge 4 commits into
zilliztech:mainfrom
j-sperling:perf/session-start-latency

Conversation

@j-sperling

Copy link
Copy Markdown
Contributor

Summary

  • Resolve the installed version from its dist-info directory name instead of a separate memsearch --version call, which spawned a second Python interpreter purely to render the status line. Falls back to the CLI when no dist-info is discoverable (uvx, editable installs).
  • Cache the PyPI latest-version lookup for 24h, so the update hint still appears but a slow or unreachable PyPI no longer blocks every session start.
  • Both plugins/claude-code and plugins/codex carried the same two patterns, so both are updated; the two helpers are added to each hooks/common.sh.

Why

SessionStart hooks block the session from starting, so their cost is paid on every launch. Measured across 50 recent sessions on one machine, this hook was the largest single contributor of the several that run:

hook median p95 max
memsearch session-start.sh 1213ms 3315ms 5965ms
next largest hook in the same sessions 234ms 726ms 1280ms

Breaking the median down against a warm macOS install:

step cost note
config list --resolved --json-output 0.32s needed — this loads the config
memsearch --version 0.33s second interpreter start, display string only
curl pypi.org/pypi/memsearch/json 0.12s warm, 2.0s on timeout answer changes at most daily

0.32 + 0.33 + 0.12 ≈ 0.77s, which accounts for the observed 1.21s median once bash startup and stop_watch are included. The tail is the curl hitting its 2s ceiling plus cold interpreter starts.

After the change, version resolution measured 0.33s → 0.01s returning the same value (verified against the CLI), and PyPI is contacted once per day rather than once per session. That puts the expected median near 0.77s and removes the 2s timeout from the tail on cached runs. The remaining config list start is inherent — the hook genuinely needs the resolved config.

This only moves blocking work. The background index spawned further down was already non-blocking and is untouched, as is the update hint's content and format.

Test plan

  • pytest tests/test_claude_hooks.py — 16 passed
  • Full suite pytest — 290 passed, 7 skipped
  • New test_claude_session_start_reads_version_from_dist_info — status version comes from dist-info and --version is never invoked
  • New test_claude_session_start_falls_back_to_cli_version_without_dist_info — layouts without a discoverable dist-info still report a version via the CLI
  • New test_claude_session_start_caches_pypi_lookup — curl runs once across two starts, the cache file is written, and the update hint survives the cache round trip
  • Regression check: reverting the version hunk alone fails test_claude_session_start_reads_version_from_dist_info
  • bash -n clean on all four modified shell files

The SessionStart hook spent two Python interpreter starts and a network
round trip building its status line. `config list --resolved` already
loads the resolved config; querying `--version` separately paid for a
second interpreter start (~0.3s warm, several seconds cold) purely to
render a display string, and the PyPI lookup blocked every session start
on a request whose answer changes at most daily.

Resolve the installed version from its dist-info directory name instead,
falling back to the CLI where no dist-info is discoverable (uvx, editable
installs), and cache the PyPI response for 24h. Both plugins carried the
same two patterns, so both are updated.

Measured on a warm macOS install: version resolution drops 0.33s -> 0.01s
and the PyPI request goes from every start to once a day. Median hook
time falls ~1.21s -> ~0.77s, and the tail loses the 2s curl timeout.
The first pass only cached successful lookups, so a machine that cannot
reach PyPI re-paid the full curl timeout on every session start — the
exact cost the cache was meant to remove.

Treat a cache file younger than a day as authoritative even when empty:
an empty file records a failed lookup. An unreachable index now costs one
timeout per day instead of one per session, and no update hint is claimed
while the latest version is unknown.
@zc277584121

Copy link
Copy Markdown
Collaborator

Thanks — the overall direction looks good, and the focused hook tests pass locally. I found one portability issue that blocks the main version-resolution optimization on a common macOS install layout.

_installed_version_from_dist_info currently relies on:

readlink -f "$bin"

The default BSD readlink on macOS does not support -f. With a typical uv tool install layout:

~/.local/bin/memsearch
  -> ~/.local/share/uv/tools/memsearch/bin/memsearch

that command fails and real falls back to the symlink path. The helper then searches under ~/.local/lib/python*/site-packages instead of the uv tool environment, finds no dist-info, and SessionStart falls back to memsearch --version. Correctness is preserved, but the second Python startup this PR is intended to remove still happens.

I reproduced this with a test that creates a symlinked memsearch entry point and shadows readlink with a macOS-like implementation that rejects -f:

expected: 9.9.9
actual:   empty

Could you switch this to a portable symlink-resolution approach and add a regression test covering a symlinked entry point when readlink -f is unavailable? Ideally the behavior should be covered for both the Claude Code and Codex hook copies.

BSD readlink on macOS rejects -f, so _installed_version_from_dist_info
fell back to the symlink path on uv tool installs, never found the
dist-info tree, and re-paid the second CLI start this branch removes.
Walk the symlink chain with plain readlink instead and canonicalize the
directory with pwd -P. The same helper replaces the readlink/python3
chain in the session-start upgrade-command detection of both hook copies.
@j-sperling

Copy link
Copy Markdown
Contributor Author

Good catch — fixed in a6813d0. _installed_version_from_dist_info now walks the symlink chain with plain readlink (relative targets resolved against the link's directory, hop cap against cycles) and canonicalizes the directory part with pwd -P, so nothing depends on GNU -f anymore. The same helper also replaced the readlink -f || python3 chain in the session-start upgrade-command detection, in both the Claude Code and Codex hook copies.

Regression coverage matches your repro: a parametrized test sources each plugin's common.sh, puts a relative-target symlinked entry point on PATH (uv tool layout), and shadows readlink with a BSD-like shim that rejects -f; a second test drives the full Claude Code SessionStart hook through a symlinked local-bin/memsearch -> share/uv/tools/memsearch/bin/memsearch layout and asserts the version comes from dist-info (no --version call) and the update hint resolves to uv tool upgrade memsearch. All three fail against the previous code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants