fix(aqua): remove unnecessary bin_paths disk cache#8383
Conversation
The aqua backend cached `list_bin_paths` results to `bin_paths.msgpack.z`, but this cache provided negligible performance benefit (reading + decompressing a msgpack file vs reading + parsing a small YAML registry file) while introducing bugs like #8372 where stale cached paths caused missing PATH entries after install. The cache invalidation added in #8374 to address that bug was itself ineffective — it couldn't prevent a concurrent `mise hook-env` process from repopulating the cache during install. Remove the cache entirely along with the now-unused `DashMap` dependency, `PathExt::mount`, and `PathExt::is_empty`. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the Aqua backend by removing an unnecessary bin path caching mechanism. The previous caching, which involved both an in-memory Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request effectively removes the bin_paths cache from the aqua backend, simplifying the code and addressing a bug concerning stale cache entries. The changes are logical and well-executed. I have one suggestion to enhance the list_bin_paths function by replacing a blocking I/O call with an asynchronous equivalent, which will improve performance and prevent potential runtime issues.
| let paths: Vec<PathBuf> = if srcs.is_empty() { | ||
| vec![install_path.clone()] | ||
| } else { | ||
| srcs.iter() | ||
| .map(|(_, dst)| dst.parent().unwrap().to_path_buf()) | ||
| .collect() | ||
| }; | ||
| Ok(paths.into_iter().unique().filter(|p| p.exists()).collect()) |
There was a problem hiding this comment.
The use of p.exists() within the filter closure performs synchronous I/O in an async function. This can block the async runtime and lead to performance issues, especially since this function is now called more frequently after removing the cache. It's recommended to use asynchronous file system operations to avoid blocking.
let paths: Vec<PathBuf> = if srcs.is_empty() {
vec![install_path.clone()]
} else {
srcs.iter()
.map(|(_, dst)| dst.parent().unwrap().to_path_buf())
.collect()
};
let mut existing_paths = Vec::new();
for p in paths.into_iter().unique() {
if tokio::fs::try_exists(&p).await.unwrap_or(false) {
existing_paths.push(p);
}
}
Ok(existing_paths)
Greptile SummaryThis PR eliminates the aqua backend's Key changes:
The new implementation returns absolute paths directly instead of the previous strip-prefix-then-remount pattern, which is functionally equivalent but simpler. Performance impact is negligible since Confidence Score: 5/5
Important Files Changed
Last reviewed commit: 5e5c5db |
Hyperfine Performance
|
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.2.23 x -- echo |
25.2 ± 0.6 | 24.3 | 27.7 | 1.00 |
mise x -- echo |
28.6 ± 0.5 | 27.7 | 32.5 | 1.14 ± 0.03 |
x -- echo is 14% |
mise env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.2.23 env |
24.8 ± 0.8 | 23.9 | 33.1 | 1.00 |
mise env |
28.1 ± 0.5 | 27.2 | 31.2 | 1.14 ± 0.04 |
env is 14% |
mise hook-env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.2.23 hook-env |
25.4 ± 0.6 | 24.3 | 28.6 | 1.00 |
mise hook-env |
29.8 ± 0.8 | 28.1 | 34.0 | 1.17 ± 0.04 |
hook-env is 17% |
mise ls
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.2.23 ls |
23.0 ± 0.8 | 22.1 | 27.5 | 1.00 |
mise ls |
23.3 ± 0.7 | 22.1 | 28.1 | 1.01 ± 0.04 |
xtasks/test/perf
| Command | mise-2026.2.23 | mise | Variance |
|---|---|---|---|
| install (cached) | 159ms | 164ms | -3% |
| ls (cached) | 90ms | 90ms | +0% |
| bin-paths (cached) | 94ms | 97ms | -3% |
| task-ls (cached) | 838ms | 848ms | -1% |
### 🐛 Bug Fixes - **(aqua)** remove unnecessary bin_paths disk cache by @jdx in [#8383](#8383) - **(hooks)** render tera templates and fix output masking by @jdx in [#8385](#8385) - **(install)** improve error when registry tool has no supported backends by @jdx in [#8388](#8388) - **(python)** remove deprecated venv_auto_create setting by @jdx in [#8384](#8384)
PR #8383 removed the aqua bin_paths disk cache to fix staleness bugs, but this caused ~8-17% regression on mise env/x/hook-env. Restore the cache using with_fresh_file(tv.install_path()) so it automatically invalidates when a tool is installed (install_path mtime changes), avoiding the original race condition without sacrificing performance. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…8398) ## Summary - Restore the aqua `bin_paths.msgpack.z` disk cache that was removed in #8383 - Use `with_fresh_file(tv.install_path())` for automatic cache invalidation when a tool is installed/updated - Restore `PathExt::mount` and `PathExt::is_empty` helpers needed for relative path handling ## Context PR #8383 removed the aqua bin_paths disk cache to fix staleness bugs (#8372) where concurrent `mise hook-env` during install could cache incomplete paths. However, this caused a ~8-17% performance regression on `mise env`, `mise x -- echo`, and `mise hook-env` (confirmed in CI hyperfine benchmarks on the 2026.2.24 release PR #8387). The root cause of the regression: each mise command is a separate process, so in-memory caches provide no benefit. The disk cache was what persisted across invocations. The original cache lacked `fresh_file` invalidation. This fix uses `with_fresh_file(tv.install_path())` — the same pattern used by `ExternalPluginCache` for asdf backends — so the cache automatically invalidates when the install directory's mtime changes (i.e., after tool installation). This avoids the race condition from #8372 without sacrificing performance. ### Local benchmark results (hyperfine, 100 runs) | Command | Baseline (2026.2.23) | No cache (2026.2.24) | **This fix** | |---------|---------------------|---------------------|-------------| | `mise env` | 24.5ms | 26.2ms (+7%) | **23.8ms** (restored) | | `mise x -- echo` | 23.5ms | 26.0ms (+11%) | **23.7ms** (restored) | ## Test plan - [x] `cargo check` passes - [x] All lints pass (`mise run lint-fix`) - [x] All 492 unit tests pass (`mise run test:unit`) - [x] Local hyperfine benchmarks confirm regression is eliminated - [ ] CI hyperfine benchmarks pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Medium risk because it changes `list_bin_paths` behavior and caching semantics for the aqua backend; incorrect invalidation or path mounting could lead to missing/incorrect PATH entries after installs/updates. > > **Overview** > Restores a persistent disk cache for aqua `list_bin_paths` (`bin_paths.msgpack.z`) to avoid recomputing bin directories on each invocation. > > The cache now uses `with_fresh_file(tv.install_path())` (plus an existing freshness duration) so it automatically invalidates when the install directory changes, and it stores paths relative to `install_path` then re-mounts them via newly reintroduced `PathExt::mount`/`PathExt::is_empty` helpers. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 8019d04. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Summary
bin_path_caches(DashMap+bin_paths.msgpack.zdisk cache) from aqua backendPathExt::mountandPathExt::is_emptylist_bin_pathsnow computes directly every callContext
The aqua backend cached
list_bin_pathsresults tobin_paths.msgpack.z, but this cache provided negligible performance benefit — reading + decompressing a msgpack file is comparable to reading + parsing a small YAML registry file (or parsing from the baked-in registry which doesn't even hit disk).Meanwhile, the cache was the root cause of #8372 where stale cached paths caused missing PATH entries after install. The fix attempted in #8374 was ineffective — it cleared the in-memory
DashMap(only helps same process) and deleted the disk cache (immediately raceable by concurrentmise hook-env). The real protection was already increate_install_dirs()which wipestv.cache_path()before install begins.Removing the cache entirely eliminates this class of bugs with no meaningful performance impact.
Test plan
cargo checkpasses with no warnings🤖 Generated with Claude Code
Note
Medium Risk
Changes how
list_bin_pathsis computed by removing both in-memory and on-disk caching, which can affect PATH resolution behavior and performance (though intended to reduce stale-path bugs). Scope is limited to the Aqua backend and removal of unused path helpers.Overview
Removes Aqua’s
bin_pathscaching layer (the per-versionDashMapandbin_paths.msgpack.zfile) and stops trying to invalidate it after installs.list_bin_pathsnow recomputes binary directories on each call by reading the registry package definition and deriving the paths directly, eliminating stale cached PATH entries.Cleans up
PathExtby deleting the now-unusedmountandis_emptyhelpers.Written by Cursor Bugbot for commit 5e5c5db. This will update automatically on new commits. Configure here.