-
-
Notifications
You must be signed in to change notification settings - Fork 769
perf(hook-env): add fast-path to skip initialization when nothing changed #7073
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
b1a9171
8ddd8df
ecd9921
dc82d49
484ec54
7834f94
dd1008e
f7738fc
355b9aa
bfe3e1d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| #!/usr/bin/env bash | ||
|
|
||
| # Test that hook-env fast-path correctly detects when nothing has changed | ||
| # and skips expensive initialization | ||
| # | ||
| # Key insight: When fast-path triggers, hook-env returns immediately with NO output. | ||
| # When fast-path is bypassed, hook-env outputs shell commands including __MISE_SESSION. | ||
| # | ||
| # Note: eval "$(mise activate bash)" already runs hook-env and sets __MISE_SESSION, | ||
| # so subsequent calls should take the fast-path if nothing changed. | ||
|
|
||
| # Activate mise - this runs hook-env internally and establishes a session | ||
| eval "$(mise activate bash)" | ||
|
|
||
| # Verify session was established by activate | ||
| if [[ -n $__MISE_SESSION ]]; then | ||
| ok "activate established __MISE_SESSION" | ||
| else | ||
| fail "activate should establish __MISE_SESSION" | ||
| fi | ||
|
|
||
| # Fast-path should work - nothing has changed since activate | ||
| output=$(mise hook-env -s bash) | ||
| if [[ -z $output ]]; then | ||
| ok "fast-path works when nothing changed (no output)" | ||
| else | ||
| fail "fast-path should produce no output but got: '$output'" | ||
| fi | ||
|
|
||
| # Test 1: Creating a new config file should bypass fast-path | ||
| sleep 0.1 # ensure mtime difference | ||
| echo '[tools]' >mise.toml | ||
| output=$(mise hook-env -s bash) | ||
| if [[ $output == *"__MISE_SESSION"* ]]; then | ||
| ok "new config file bypasses fast-path" | ||
| else | ||
| fail "new config file should bypass fast-path but got: '$output'" | ||
| fi | ||
| rm mise.toml | ||
| eval "$output" | ||
|
|
||
| # Removing the config file also changes directory mtime, so the first | ||
| # hook-env after removal will bypass fast-path. Run it to re-establish session. | ||
| output=$(mise hook-env -s bash) | ||
| eval "$output" | ||
|
|
||
| # Now fast-path should work | ||
| output=$(mise hook-env -s bash) | ||
| if [[ -z $output ]]; then | ||
| ok "fast-path works after session re-established" | ||
| else | ||
| fail "fast-path should work after session re-established but got: '$output'" | ||
| fi | ||
|
|
||
| # Test 2: Changing directories should bypass fast-path | ||
| # First ensure we have a clean session in the current directory | ||
| eval "$(mise activate bash)" | ||
| # Verify fast-path works here | ||
| output=$(mise hook-env -s bash) | ||
| if [[ -n $output ]]; then | ||
| fail "expected fast-path before dir change but got output" | ||
| fi | ||
|
|
||
| # Now change directory and verify fast-path is bypassed | ||
| # Use builtin cd to avoid the _mise_hook wrapper that activate installs | ||
| mkdir -p subdir | ||
| builtin cd subdir | ||
| output=$(mise hook-env -s bash) | ||
| if [[ $output == *"__MISE_SESSION"* ]]; then | ||
| ok "directory change bypasses fast-path" | ||
| else | ||
| fail "directory change should bypass fast-path but got: '$output'" | ||
| fi | ||
| builtin cd .. | ||
| rmdir subdir | ||
|
|
||
| # Test 3: Creating config in parent's .config/mise should be detected | ||
| # Re-establish session first | ||
| eval "$(mise activate bash)" | ||
|
|
||
| mkdir -p ../.config/mise | ||
| sleep 0.1 # ensure mtime difference | ||
| echo '[tools]' >../.config/mise/config.toml | ||
| output=$(mise hook-env -s bash) | ||
| if [[ $output == *"__MISE_SESSION"* ]]; then | ||
| ok "parent .config/mise config file bypasses fast-path" | ||
| else | ||
| fail "parent .config/mise should bypass fast-path but got: '$output'" | ||
| fi | ||
| rm -rf ../.config |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -4,7 +4,7 @@ use crate::task::TaskOutput; | |||||||||||||||||||||||||||||
| use crate::ui::{self, ctrlc}; | ||||||||||||||||||||||||||||||
| use crate::{Result, backend}; | ||||||||||||||||||||||||||||||
| use crate::{cli::args::ToolArg, path::PathExt}; | ||||||||||||||||||||||||||||||
| use crate::{logger, migrate, shims}; | ||||||||||||||||||||||||||||||
| use crate::{hook_env as hook_env_module, logger, migrate, shims}; | ||||||||||||||||||||||||||||||
| use clap::{ArgAction, CommandFactory, Parser, Subcommand}; | ||||||||||||||||||||||||||||||
| use eyre::bail; | ||||||||||||||||||||||||||||||
| use std::path::PathBuf; | ||||||||||||||||||||||||||||||
|
|
@@ -433,6 +433,11 @@ impl Cli { | |||||||||||||||||||||||||||||
| if *crate::env::MISE_TOOL_STUB && args.len() >= 2 { | ||||||||||||||||||||||||||||||
| tool_stub::short_circuit_stub(&args[2..]).await?; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| // Fast-path for hook-env: exit early if nothing has changed | ||||||||||||||||||||||||||||||
| // This avoids expensive backend::load_tools() and config loading | ||||||||||||||||||||||||||||||
| if hook_env_module::should_exit_early_fast() { | ||||||||||||||||||||||||||||||
| return Ok(()); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| measure!("logger", { logger::init() }); | ||||||||||||||||||||||||||||||
| check_working_directory(); | ||||||||||||||||||||||||||||||
|
Comment on lines
+436
to
442
|
||||||||||||||||||||||||||||||
| // Fast-path for hook-env: exit early if nothing has changed | |
| // This avoids expensive backend::load_tools() and config loading | |
| if hook_env_module::should_exit_early_fast() { | |
| return Ok(()); | |
| } | |
| measure!("logger", { logger::init() }); | |
| check_working_directory(); | |
| check_working_directory(); | |
| // Fast-path for hook-env: exit early if nothing has changed | |
| // This avoids expensive backend::load_tools() and config loading | |
| if hook_env_module::should_exit_early_fast() { | |
| return Ok(()); | |
| } | |
| measure!("logger", { logger::init() }); |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -19,7 +19,15 @@ use crate::env::PATH_KEY; | |||||||||||||
| use crate::env_diff::{EnvDiffOperation, EnvDiffPatches, EnvMap}; | ||||||||||||||
| use crate::hash::hash_to_str; | ||||||||||||||
| use crate::shell::Shell; | ||||||||||||||
| use crate::{dirs, env, hooks, watch_files}; | ||||||||||||||
| use crate::{dirs, env, file, hooks, watch_files}; | ||||||||||||||
|
|
||||||||||||||
| /// Convert a SystemTime to milliseconds since Unix epoch | ||||||||||||||
| fn mtime_to_millis(mtime: SystemTime) -> u128 { | ||||||||||||||
| mtime | ||||||||||||||
| .duration_since(UNIX_EPOCH) | ||||||||||||||
| .unwrap_or_default() | ||||||||||||||
| .as_millis() | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| pub static PREV_SESSION: Lazy<HookEnvSession> = Lazy::new(|| { | ||||||||||||||
| env::var("__MISE_SESSION") | ||||||||||||||
|
|
@@ -59,28 +67,111 @@ impl From<PathBuf> for WatchFilePattern { | |||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /// this function will early-exit the application if hook-env is being | ||||||||||||||
| /// called and it does not need to be | ||||||||||||||
| pub fn should_exit_early( | ||||||||||||||
| watch_files: impl IntoIterator<Item = WatchFilePattern>, | ||||||||||||||
| reason: Option<HookReason>, | ||||||||||||||
| ) -> bool { | ||||||||||||||
| /// Fast-path early exit check that can be called BEFORE loading config/tools. | ||||||||||||||
| /// This checks basic conditions using only the previous session data. | ||||||||||||||
| /// Returns true if we can definitely skip hook-env, false if we need to continue. | ||||||||||||||
| pub fn should_exit_early_fast() -> bool { | ||||||||||||||
| let args = env::ARGS.read().unwrap(); | ||||||||||||||
| if args.len() < 2 || args[1] != "hook-env" { | ||||||||||||||
| return false; | ||||||||||||||
| } | ||||||||||||||
| // Can't exit early if no previous session | ||||||||||||||
| // Check for dir being set as a proxy for "has valid session" | ||||||||||||||
| // (loaded_configs can be empty if there are no config files) | ||||||||||||||
| if PREV_SESSION.dir.is_none() { | ||||||||||||||
| return false; | ||||||||||||||
| } | ||||||||||||||
| // Can't exit early if --force flag is present | ||||||||||||||
| if args.iter().any(|a| a == "--force" || a == "-f") { | ||||||||||||||
| return false; | ||||||||||||||
| } | ||||||||||||||
| // Check if running from precmd for the first time | ||||||||||||||
| // Handle both "--reason=precmd" and "--reason precmd" forms | ||||||||||||||
| let is_precmd = args.iter().any(|a| a == "--reason=precmd") | ||||||||||||||
| || args | ||||||||||||||
| .windows(2) | ||||||||||||||
| .any(|w| w[0] == "--reason" && w[1] == "precmd"); | ||||||||||||||
| if is_precmd && !*env::__MISE_ZSH_PRECMD_RUN { | ||||||||||||||
| return false; | ||||||||||||||
| } | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Fast-path fails to detect space-separated reason argumentThe check for |
||||||||||||||
| // Can't exit early if directory changed | ||||||||||||||
| if dir_change().is_some() { | ||||||||||||||
| return false; | ||||||||||||||
| } | ||||||||||||||
| // Can't exit early if MISE_ env vars changed | ||||||||||||||
|
||||||||||||||
| // Can't exit early if MISE_ env vars changed | |
| // Can't exit early if MISE_ env vars changed | |
| // Only environment variables with the `MISE_` prefix are hashed and compared by | |
| // `have_mise_env_vars_been_modified()`. Non-`MISE_` variables that might affect | |
| // resolution are intentionally excluded from this check. This assumes that only | |
| // `MISE_` variables influence the fast path's correctness. |
Copilot
AI
Nov 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If metadata() fails for reasons other than non-existence (e.g., permissions), this logic continues without invalidating the fast path, potentially skipping necessary re-initialization. To be safe, any failure to read metadata should cause a full run, not an early exit.
| } else if !config_path.exists() { | |
| } else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Importing
hook_envashook_env_moduleis unconventional and can be confusing. Prefer a directuse crate::hook_env;and callhook_env::should_exit_early_fast()for clarity.