perf(install): use Kahn's algorithm for dependency scheduling#7933
Conversation
Replace batch-based tool installation with channel-based Kahn's algorithm for better parallelization of dependency chains. Before: Install all leaves → wait for entire batch → recalculate After: As soon as a tool completes, immediately start any newly-ready dependents For chains A → B → C and D → E: - Old: [Install A,D] → wait → [Install B,E] → wait → [Install C] - New: Start A,D; when A finishes, start B immediately (don't wait for D) Changes: - Add new tool_deps.rs module with petgraph-based dependency graph - Replace batch loop in install_all_versions with tokio::select! loop - Remove unused get_leaf_dependencies and install_some_versions functions Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
There was a problem hiding this comment.
Pull request overview
This PR replaces the batch-based tool installation with a channel-based implementation using Kahn's algorithm for better parallelization of dependency chains. The new approach allows tools to be installed immediately when their dependencies complete, rather than waiting for entire batches to finish.
Changes:
- Introduced
tool_deps.rsmodule implementing petgraph-based dependency graph with Kahn's algorithm - Refactored
install_all_versionsto usetokio::select!loop instead of batch processing - Removed unused
get_leaf_dependenciesandinstall_some_versionshelper functions
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/toolset/toolset_install.rs | Replaced batch installation loop with dependency graph-based scheduling using channels |
| src/toolset/tool_deps.rs | New module implementing Kahn's algorithm for dependency ordering with petgraph |
| src/toolset/mod.rs | Added tool_deps module declaration |
| src/toolset/helpers.rs | Removed unused get_leaf_dependencies function |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| } | ||
|
|
||
| let (tx, _) = mpsc::unbounded_channel(); |
There was a problem hiding this comment.
The receiver from this channel is immediately dropped. This creates a channel that cannot receive messages until subscribe() is called. Consider moving channel creation to subscribe() or documenting why this initial channel is necessary.
| for tr2 in versions { | ||
| if tr2.ba() == ba { |
There was a problem hiding this comment.
This nested loop creates O(n²) complexity when collecting plugin errors. Since you've already filtered by backend in the outer loop (line 230-234), consider building a map of backend to tools beforehand to avoid this nested iteration.
| let mut jset: JoinSet<(ToolRequest, Result<ToolVersion>)> = JoinSet::new(); | ||
|
|
||
| loop { | ||
| tokio::select! { |
There was a problem hiding this comment.
The biased annotation ensures completed installations are handled before new ones are started. Consider adding a comment explaining this priority ordering is intentional for correctness.
| tokio::select! { | |
| tokio::select! { | |
| // Use `biased` so completed installations are handled before starting new ones. | |
| // This priority ordering is intentional and required for correctness of dependency tracking. |
| } | ||
|
|
||
| if self.sent.insert(key) { | ||
| trace!("Scheduling tool install: {}", tr); |
There was a problem hiding this comment.
Missing import for trace! macro. Ensure use tracing::trace; or equivalent is added to support this logging statement.
- Use StableGraph instead of DiGraph to fix stale node indices after removal - Add cycle detection to prevent infinite hangs on circular dependencies - Call complete_failure when semaphore acquisition fails to properly update graph - Add comment explaining biased priority ordering in tokio::select! - Document initial dummy channel pattern in ToolDeps Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
bugbot run |
- Handle task panics by tracking in-flight tools to recover ToolRequest - Move footer_inc to caller to ensure progress increments on all outcomes - Remove unused completed/failed fields from ToolDeps - Deduplicate tool requests to prevent orphan nodes - Update test_error_display to expect new error format Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
bugbot run |
The channel-based installation can complete tools in any order due to parallel execution. This caused the installed versions to be returned in completion order rather than the user's intended order, breaking tests like `mise use dummy@1 dummy@2` where the order matters. Add an index map to track original request order and sort the installed versions by that order before returning. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Hyperfine Performance
|
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.1.12 x -- echo |
19.6 ± 0.4 | 19.1 | 25.0 | 1.00 |
mise x -- echo |
19.8 ± 0.5 | 19.1 | 22.3 | 1.01 ± 0.03 |
mise env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.1.12 env |
19.1 ± 0.4 | 18.4 | 23.5 | 1.00 |
mise env |
19.3 ± 0.6 | 18.7 | 25.7 | 1.01 ± 0.04 |
mise hook-env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.1.12 hook-env |
19.9 ± 0.5 | 19.1 | 22.6 | 1.00 |
mise hook-env |
20.8 ± 1.0 | 19.3 | 31.0 | 1.04 ± 0.06 |
mise ls
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.1.12 ls |
17.8 ± 0.5 | 16.9 | 19.2 | 1.00 ± 0.03 |
mise ls |
17.7 ± 0.3 | 17.0 | 19.6 | 1.00 |
xtasks/test/perf
| Command | mise-2026.1.12 | mise | Variance |
|---|---|---|---|
| install (cached) | 109ms | 110ms | +0% |
| ls (cached) | 69ms | 69ms | +0% |
| bin-paths (cached) | 73ms | 73ms | +0% |
| task-ls (cached) | 533ms | 531ms | +0% |
#7936) ## Summary With parallel tool installation (introduced in #7933), failures complete in arbitrary order. This caused `test_error_display` to fail intermittently because the error message listed tools in completion order rather than a consistent order. This PR sorts failed tools alphabetically in the error message to ensure deterministic output regardless of parallel execution order. ## Test plan - [x] `mise run build` succeeds - [x] `mise run lint` passes - [ ] `e2e/cli/test_error_display` passes in CI 🤖 Generated with [Claude Code](https://claude.ai/code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk: only changes ordering of multi-tool install failure messages and updates an e2e assertion; no behavior changes beyond formatting. > > **Overview** > Makes multi-tool install failure output deterministic by sorting `failed_installations` by tool name before building the summary list and per-tool error details in `format_install_failures`. > > Updates `e2e/cli/test_error_display` to expect the new sorted ordering in the “invalid configuration” multi-failure case. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 160c509. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
### 🚀 Features - **(edit)** add interactive config editor (`mise edit`) by @jdx in [#7930](#7930) - **(lockfile)** graduate lockfiles from experimental by @jdx in [#7929](#7929) - **(task)** add support for usage values in task confirm dialog by @roele in [#7924](#7924) - **(task)** improve source freshness checking with edge case handling by @jdx in [#7932](#7932) ### 🐛 Bug Fixes - **(activate)** preserve ordering of paths appended after mise activate by @jdx in [#7919](#7919) - **(install)** sort failed installations for deterministic error output by @jdx in [#7936](#7936) - **(lockfile)** preserve URL and prefer sha256 when merging platform info by @jdx in [#7923](#7923) - **(lockfile)** add atomic writes and cache invalidation by @jdx in [#7927](#7927) - **(templates)** use sha256 for hash filter instead of blake3 by @jdx in [#7925](#7925) - **(upgrade)** respect tracked configs when pruning old versions by @jdx in [#7926](#7926) ### 🚜 Refactor - **(progress)** migrate from indicatif to clx by @jdx in [#7928](#7928) ### 📚 Documentation - improve clarity on uvx and pipx dependencies by @ygormutti in [#7878](#7878) ### ⚡ Performance - **(install)** use Kahn's algorithm for dependency scheduling by @jdx in [#7933](#7933) - use Aho-Corasick for efficient redaction by @jdx in [#7931](#7931) ### 🧪 Testing - remove flaky test_http_version_list test by @jdx in [#7934](#7934) ### Chore - use github backend instead of ubi in mise.lock by @jdx in [#7922](#7922) ### New Contributors - @ygormutti made their first contribution in [#7878](#7878)
## Summary - Replace batch-based tool installation with channel-based Kahn's algorithm for better parallelization of dependency chains - Add new `tool_deps.rs` module with petgraph-based dependency graph - Replace batch loop in `install_all_versions` with `tokio::select!` loop - Remove unused `get_leaf_dependencies` and `install_some_versions` functions ### Before Install all leaves in a batch → wait for entire batch to complete → recalculate leaves → repeat For chains `A → B → C` and `D → E`: ``` [Install A,D] → wait for both → [Install B,E] → wait for both → [Install C] ``` ### After As soon as a tool completes, immediately emit any newly-ready dependents For chains `A → B → C` and `D → E`: ``` Start A,D; when A finishes, start B immediately (don't wait for D) ``` ## Test plan - [x] Unit tests pass (`mise run test:unit`) - [x] Build succeeds - [x] Lint passes - [ ] E2E tests pass (tested locally with `test_install` suite) - [ ] `e2e/backend/test_pipx_direct_dependencies` tests 2-level dependency chain - [ ] `e2e/backend/test_pipx_deep_dependencies_slow` tests 3-level dependency chain 🤖 Generated with [Claude Code](https://claude.ai/code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Moderate risk because it rewrites core install orchestration, changing concurrency, dependency ordering, and failure propagation (including blocking dependents), which could cause subtle deadlocks/order regressions or different error surfaces. > > **Overview** > Switches `Toolset::install_all_versions` from a batch “install leaf set, recompute” loop to a streaming scheduler: tools are emitted as soon as their dependencies complete, improving parallelism across independent chains. > > Introduces `tool_deps.rs` (`ToolDeps`) using a petgraph-backed dependency graph to dedupe requests, detect cycles (marking them blocked), and block transitive dependents on failures; install execution is refactored into `install_with_deps`/`install_single_tool` with a `tokio::select!` loop and panic recovery. > > Updates plugin-install error handling to be collected per-tool and merged into the final `InstallFailed` result, and adjusts the E2E error-display expectation for “tool not found” to include the `Failed to install …` prefix. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit f9e3ce7. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
jdx#7936) ## Summary With parallel tool installation (introduced in jdx#7933), failures complete in arbitrary order. This caused `test_error_display` to fail intermittently because the error message listed tools in completion order rather than a consistent order. This PR sorts failed tools alphabetically in the error message to ensure deterministic output regardless of parallel execution order. ## Test plan - [x] `mise run build` succeeds - [x] `mise run lint` passes - [ ] `e2e/cli/test_error_display` passes in CI 🤖 Generated with [Claude Code](https://claude.ai/code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk: only changes ordering of multi-tool install failure messages and updates an e2e assertion; no behavior changes beyond formatting. > > **Overview** > Makes multi-tool install failure output deterministic by sorting `failed_installations` by tool name before building the summary list and per-tool error details in `format_install_failures`. > > Updates `e2e/cli/test_error_display` to expect the new sorted ordering in the “invalid configuration” multi-failure case. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 160c509. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
### 🚀 Features - **(edit)** add interactive config editor (`mise edit`) by @jdx in [jdx#7930](jdx#7930) - **(lockfile)** graduate lockfiles from experimental by @jdx in [jdx#7929](jdx#7929) - **(task)** add support for usage values in task confirm dialog by @roele in [jdx#7924](jdx#7924) - **(task)** improve source freshness checking with edge case handling by @jdx in [jdx#7932](jdx#7932) ### 🐛 Bug Fixes - **(activate)** preserve ordering of paths appended after mise activate by @jdx in [jdx#7919](jdx#7919) - **(install)** sort failed installations for deterministic error output by @jdx in [jdx#7936](jdx#7936) - **(lockfile)** preserve URL and prefer sha256 when merging platform info by @jdx in [jdx#7923](jdx#7923) - **(lockfile)** add atomic writes and cache invalidation by @jdx in [jdx#7927](jdx#7927) - **(templates)** use sha256 for hash filter instead of blake3 by @jdx in [jdx#7925](jdx#7925) - **(upgrade)** respect tracked configs when pruning old versions by @jdx in [jdx#7926](jdx#7926) ### 🚜 Refactor - **(progress)** migrate from indicatif to clx by @jdx in [jdx#7928](jdx#7928) ### 📚 Documentation - improve clarity on uvx and pipx dependencies by @ygormutti in [jdx#7878](jdx#7878) ### ⚡ Performance - **(install)** use Kahn's algorithm for dependency scheduling by @jdx in [jdx#7933](jdx#7933) - use Aho-Corasick for efficient redaction by @jdx in [jdx#7931](jdx#7931) ### 🧪 Testing - remove flaky test_http_version_list test by @jdx in [jdx#7934](jdx#7934) ### Chore - use github backend instead of ubi in mise.lock by @jdx in [jdx#7922](jdx#7922) ### New Contributors - @ygormutti made their first contribution in [jdx#7878](jdx#7878)
Summary
tool_deps.rsmodule with petgraph-based dependency graphinstall_all_versionswithtokio::select!loopget_leaf_dependenciesandinstall_some_versionsfunctionsBefore
Install all leaves in a batch → wait for entire batch to complete → recalculate leaves → repeat
For chains
A → B → CandD → E:After
As soon as a tool completes, immediately emit any newly-ready dependents
For chains
A → B → CandD → E:Test plan
mise run test:unit)test_installsuite)e2e/backend/test_pipx_direct_dependenciestests 2-level dependency chaine2e/backend/test_pipx_deep_dependencies_slowtests 3-level dependency chain🤖 Generated with Claude Code
Note
Medium Risk
Moderate risk because it rewrites core install orchestration, changing concurrency, dependency ordering, and failure propagation (including blocking dependents), which could cause subtle deadlocks/order regressions or different error surfaces.
Overview
Switches
Toolset::install_all_versionsfrom a batch “install leaf set, recompute” loop to a streaming scheduler: tools are emitted as soon as their dependencies complete, improving parallelism across independent chains.Introduces
tool_deps.rs(ToolDeps) using a petgraph-backed dependency graph to dedupe requests, detect cycles (marking them blocked), and block transitive dependents on failures; install execution is refactored intoinstall_with_deps/install_single_toolwith atokio::select!loop and panic recovery.Updates plugin-install error handling to be collected per-tool and merged into the final
InstallFailedresult, and adjusts the E2E error-display expectation for “tool not found” to include theFailed to install …prefix.Written by Cursor Bugbot for commit f9e3ce7. This will update automatically on new commits. Configure here.