diff --git a/src/commands/list/collect/mod.rs b/src/commands/list/collect/mod.rs index 0f5a76bb6d..fede7cf113 100644 --- a/src/commands/list/collect/mod.rs +++ b/src/commands/list/collect/mod.rs @@ -137,7 +137,7 @@ //! │ ├─ switch_previous() (5ms) //! │ ├─ capture_refs[_with_ahead_behind]() (ref snapshot) //! │ │ └─ prime_upstream_ahead_behind_cache() (nested; reads the snapshot) -//! │ ├─ start_fsmonitor_daemon × N worktrees (6ms each, all parallel) +//! │ ├─ start_fsmonitor_daemon × N worktrees (socket probe; fork only when no daemon answers) //! │ ) // joins on the slowest spawn //! ├─ integration_targets(snapshot) (sequential; needs the snapshot) //! ├─ populate ListItem.commit from cache (cache-hit lookups, sub-ms) @@ -163,7 +163,9 @@ //! //! **Why fsmonitor starts are in the parallel scope:** The `git fsmonitor--daemon start` //! command returns quickly after signaling the daemon. By the time the worker thread -//! starts executing `git status` commands, daemons have had time to initialize. +//! starts executing `git status` commands, daemons have had time to initialize. In the +//! steady state every daemon is already up, and the in-process socket probe in +//! `start_fsmonitor_daemon_at` returns without forking. //! //! When adding new features, ask: "Can this be computed after skeleton?" If yes, defer it. //! The skeleton shows `·` placeholder for gutter symbols, filled in when data loads. diff --git a/src/git/fsmonitor.rs b/src/git/fsmonitor.rs index d61bd4a8c3..27180fcd6f 100644 --- a/src/git/fsmonitor.rs +++ b/src/git/fsmonitor.rs @@ -66,7 +66,7 @@ use super::WorkingTree; /// Basename of every fsmonitor IPC socket. Also the exact string `lsof` /// prints when the socket's directory has been deleted (orphan class 1). -const IPC_SOCKET_NAME: &str = "fsmonitor--daemon.ipc"; +pub(crate) const IPC_SOCKET_NAME: &str = "fsmonitor--daemon.ipc"; /// How long to wait for `SIGTERM`'d daemons to exit before escalating to /// `SIGKILL`. Bounded so the sweep can never stall `wt`. Daemons are wedged diff --git a/src/git/remove.rs b/src/git/remove.rs index ab29630d46..fee6cff235 100644 --- a/src/git/remove.rs +++ b/src/git/remove.rs @@ -158,7 +158,7 @@ pub fn stop_fsmonitor_daemon(worktree: &WorkingTree) { // linked worktree uses — never hand-construct `/.git`). The daemon // binds its IPC socket at `/fsmonitor--daemon.ipc`. let socket = match worktree.git_dir() { - Ok(git_dir) => git_dir.join("fsmonitor--daemon.ipc"), + Ok(git_dir) => git_dir.join(super::fsmonitor::IPC_SOCKET_NAME), Err(e) => { tracing::debug!(error = %e, "fsmonitor: could not resolve git dir, skipping force-kill: {e}"); return; diff --git a/src/git/repository/mod.rs b/src/git/repository/mod.rs index 9566e1e938..b750f5af4a 100644 --- a/src/git/repository/mod.rs +++ b/src/git/repository/mod.rs @@ -1841,12 +1841,29 @@ impl Repository { /// Idempotent — if the daemon is already running, this is a no-op. /// Used to avoid auto-start races when running many parallel git commands. /// + /// A running daemon is detected in-process first, the way `git + /// fsmonitor--daemon start` itself checks before refusing with "already + /// running": connect to `/fsmonitor--daemon.ipc` and close. That + /// skips a ~20ms fork per worktree in the steady state, where every daemon + /// is already up. Any failure to connect (no daemon, stale socket, a path + /// too long for `sun_path`, an unresolvable git dir) falls through to the + /// fork, which starts the daemon or reports it running. + /// /// Uses `Command::status()` with null stdio instead of `Cmd::run()` to avoid /// pipe inheritance: the daemon process (`git fsmonitor--daemon run --detach`) /// inherits pipe file descriptors from its parent, keeping them open /// indefinitely. `read_to_end()` in `Command::output()` then blocks forever /// waiting for EOF that never comes. pub fn start_fsmonitor_daemon_at(&self, path: &Path) { + #[cfg(unix)] + if let Ok(git_dir) = self.worktree_at(path).git_dir() + && std::os::unix::net::UnixStream::connect( + git_dir.join(super::fsmonitor::IPC_SOCKET_NAME), + ) + .is_ok() + { + return; + } let context = path_to_logging_context(path); let cmd_str = "git fsmonitor--daemon start"; tracing::debug!(cmd = cmd_str, context = %context, "$ {cmd_str} [{context}]"); diff --git a/src/git/repository/tests.rs b/src/git/repository/tests.rs index dd6332e0b1..02888b7e5d 100644 --- a/src/git/repository/tests.rs +++ b/src/git/repository/tests.rs @@ -705,6 +705,84 @@ fn is_builtin_fsmonitor_enabled_variants() { assert!(!repo_with_fsmonitor(None).is_builtin_fsmonitor_enabled()); } +/// A daemon answering on the worktree's IPC socket means no `git +/// fsmonitor--daemon start` fork; with no socket to reach, the fork runs. +/// The listener stands in for a running daemon, so no real daemon starts. +#[cfg(all(unix, feature = "cli"))] +#[test] +fn start_fsmonitor_daemon_skips_fork_when_daemon_answers() { + use std::os::unix::net::UnixListener; + use std::sync::{Arc, Mutex}; + + use tracing::field::{Field, Visit}; + use tracing_subscriber::Registry; + use tracing_subscriber::layer::{Context, Layer, SubscriberExt}; + + use crate::git::Repository; + use crate::testing::{TestRepo, test_tempdir}; + use crate::trace::WT_TRACE_TARGET; + + /// Collects the `cmd` of every traced subprocess. + struct Commands(Arc>>); + struct CmdField(Option); + impl Visit for CmdField { + fn record_str(&mut self, field: &Field, value: &str) { + if field.name() == "cmd" { + self.0 = Some(value.to_string()); + } + } + fn record_debug(&mut self, _: &Field, _: &dyn std::fmt::Debug) {} + } + impl Layer for Commands { + fn on_event(&self, event: &tracing::Event<'_>, _: Context<'_, S>) { + if event.metadata().target() != WT_TRACE_TARGET { + return; + } + let mut cmd = CmdField(None); + event.record(&mut cmd); + self.0.lock().unwrap().extend(cmd.0); + } + } + let traced_starts = |start: &dyn Fn()| { + let commands = Arc::new(Mutex::new(Vec::new())); + let subscriber = Registry::default().with(Commands(commands.clone())); + tracing::subscriber::with_default(subscriber, start); + let commands = commands.lock().unwrap(); + commands + .iter() + .filter(|cmd| *cmd == "git fsmonitor--daemon start") + .count() + }; + + let test = TestRepo::with_initial_commit(); + let repo = Repository::at(test.root_path()).unwrap(); + let socket = repo + .worktree_at(test.root_path()) + .git_dir() + .unwrap() + .join(super::super::fsmonitor::IPC_SOCKET_NAME); + let listener = UnixListener::bind(&socket).unwrap_or_else(|e| { + panic!( + "bind {} ({} bytes; sun_path holds 103 on macOS): {e}", + socket.display(), + socket.as_os_str().len() + ) + }); + listener.set_nonblocking(true).unwrap(); + + let starts = traced_starts(&|| repo.start_fsmonitor_daemon_at(test.root_path())); + assert_eq!(starts, 0, "a daemon answered, so nothing should fork"); + listener + .accept() + .expect("the probe should have connected to the daemon socket"); + + // A directory outside any repository has no git dir to probe, so the + // start forks (and git exits "not a git repository"). + let outside = test_tempdir(); + let starts = traced_starts(&|| repo.start_fsmonitor_daemon_at(outside.path())); + assert_eq!(starts, 1, "nothing answered, so the start should fork"); +} + #[test] fn commit_details_many_returns_subject_with_spaces() { use crate::testing::TestRepo;