terminal: Update title changed info on the background (#48000)

Lukas Wirth created

This can take several milliseconds on windows blocking the main thread
noticably

Release Notes:

- N/A *or* Added/Fixed/Improved ...

Change summary

crates/terminal/Cargo.toml      |  2 
crates/terminal/src/pty_info.rs | 74 ++++++++++++++++++++++------------
crates/terminal/src/terminal.rs | 23 +++++-----
3 files changed, 61 insertions(+), 38 deletions(-)

Detailed changes

crates/terminal/Cargo.toml 🔗

@@ -41,13 +41,13 @@ thiserror.workspace = true
 url.workspace = true
 util.workspace = true
 urlencoding.workspace = true
+parking_lot.workspace = true
 
 [target.'cfg(windows)'.dependencies]
 windows.workspace = true
 
 [dev-dependencies]
 gpui = { workspace = true, features = ["test-support"] }
-parking_lot.workspace = true
 rand.workspace = true
 serde_json.workspace = true
 settings = { workspace = true, features = ["test-support"] }

crates/terminal/src/pty_info.rs 🔗

@@ -1,15 +1,20 @@
 use alacritty_terminal::tty::Pty;
+use gpui::{Context, Task};
+use parking_lot::{MappedRwLockReadGuard, Mutex, RwLock, RwLockReadGuard};
 #[cfg(target_os = "windows")]
 use std::num::NonZeroU32;
 #[cfg(unix)]
 use std::os::fd::AsRawFd;
-use std::path::PathBuf;
+use std::{path::PathBuf, sync::Arc};
 
 #[cfg(target_os = "windows")]
 use windows::Win32::{Foundation::HANDLE, System::Threading::GetProcessId};
 
 use sysinfo::{Pid, Process, ProcessRefreshKind, RefreshKind, System, UpdateKind};
 
+use crate::{Event, Terminal};
+
+#[derive(Clone, Copy)]
 pub struct ProcessIdGetter {
     handle: i32,
     fallback_pid: u32,
@@ -79,10 +84,11 @@ pub struct ProcessInfo {
 
 /// Fetches Zed-relevant Pseudo-Terminal (PTY) process information
 pub struct PtyProcessInfo {
-    system: System,
+    system: RwLock<System>,
     refresh_kind: ProcessRefreshKind,
     pid_getter: ProcessIdGetter,
-    pub current: Option<ProcessInfo>,
+    pub current: RwLock<Option<ProcessInfo>>,
+    task: Mutex<Option<Task<()>>>,
 }
 
 impl PtyProcessInfo {
@@ -95,10 +101,11 @@ impl PtyProcessInfo {
         let system = System::new_with_specifics(refresh_kind);
 
         PtyProcessInfo {
-            system,
+            system: RwLock::new(system),
             refresh_kind: process_refresh_kind,
             pid_getter: ProcessIdGetter::new(pty),
-            current: None,
+            current: RwLock::new(None),
+            task: Mutex::new(None),
         }
     }
 
@@ -106,27 +113,27 @@ impl PtyProcessInfo {
         &self.pid_getter
     }
 
-    fn refresh(&mut self) -> Option<&Process> {
+    fn refresh(&self) -> Option<MappedRwLockReadGuard<'_, Process>> {
         let pid = self.pid_getter.pid()?;
-        if self.system.refresh_processes_specifics(
+        if self.system.write().refresh_processes_specifics(
             sysinfo::ProcessesToUpdate::Some(&[pid]),
             true,
             self.refresh_kind,
         ) == 1
         {
-            self.system.process(pid)
+            RwLockReadGuard::try_map(self.system.read(), |system| system.process(pid)).ok()
         } else {
             None
         }
     }
 
-    fn get_child(&self) -> Option<&Process> {
+    fn get_child(&self) -> Option<MappedRwLockReadGuard<'_, Process>> {
         let pid = self.pid_getter.fallback_pid();
-        self.system.process(pid)
+        RwLockReadGuard::try_map(self.system.read(), |system| system.process(pid)).ok()
     }
 
     #[cfg(unix)]
-    pub(crate) fn kill_current_process(&mut self) -> bool {
+    pub(crate) fn kill_current_process(&self) -> bool {
         let Some(pid) = self.pid_getter.pid() else {
             return false;
         };
@@ -134,15 +141,15 @@ impl PtyProcessInfo {
     }
 
     #[cfg(not(unix))]
-    pub(crate) fn kill_current_process(&mut self) -> bool {
+    pub(crate) fn kill_current_process(&self) -> bool {
         self.refresh().is_some_and(|process| process.kill())
     }
 
-    pub(crate) fn kill_child_process(&mut self) -> bool {
+    pub(crate) fn kill_child_process(&self) -> bool {
         self.get_child().is_some_and(|process| process.kill())
     }
 
-    fn load(&mut self) -> Option<ProcessInfo> {
+    fn load(&self) -> Option<ProcessInfo> {
         let process = self.refresh()?;
         let cwd = process.cwd().map_or(PathBuf::new(), |p| p.to_owned());
 
@@ -155,22 +162,37 @@ impl PtyProcessInfo {
                 .filter_map(|s| s.to_str().map(ToOwned::to_owned))
                 .collect(),
         };
-        self.current = Some(info.clone());
+        *self.current.write() = Some(info.clone());
         Some(info)
     }
 
-    /// Updates the cached process info, returns whether the Zed-relevant info has changed
-    pub fn has_changed(&mut self) -> bool {
-        let current = self.load();
-        let has_changed = match (self.current.as_ref(), current.as_ref()) {
-            (None, None) => false,
-            (Some(prev), Some(now)) => prev.cwd != now.cwd || prev.name != now.name,
-            _ => true,
-        };
-        if has_changed {
-            self.current = current;
+    /// Updates the cached process info, emitting a [`Event::TitleChanged`] event if the Zed-relevant info has changed
+    pub fn emit_title_changed_if_changed(self: &Arc<Self>, cx: &mut Context<'_, Terminal>) {
+        if self.task.lock().is_some() {
+            return;
         }
-        has_changed
+        let this = self.clone();
+        let has_changed = cx.background_executor().spawn(async move {
+            let current = this.load();
+            let has_changed = match (this.current.read().as_ref(), current.as_ref()) {
+                (None, None) => false,
+                (Some(prev), Some(now)) => prev.cwd != now.cwd || prev.name != now.name,
+                _ => true,
+            };
+            if has_changed {
+                *this.current.write() = current;
+            }
+            has_changed
+        });
+        let this = Arc::downgrade(self);
+        *self.task.lock() = Some(cx.spawn(async move |term, cx| {
+            if has_changed.await {
+                term.update(cx, |_, cx| cx.emit(Event::TitleChanged)).ok();
+            }
+            if let Some(this) = this.upgrade() {
+                this.task.lock().take();
+            }
+        }));
     }
 
     pub fn pid(&self) -> Option<Pid> {

crates/terminal/src/terminal.rs 🔗

@@ -604,7 +604,7 @@ impl TerminalBuilder {
                 task,
                 terminal_type: TerminalType::Pty {
                     pty_tx: Notifier(pty_tx),
-                    info: pty_info,
+                    info: Arc::new(pty_info),
                 },
                 completion_tx,
                 term,
@@ -836,7 +836,7 @@ pub enum SelectionPhase {
 enum TerminalType {
     Pty {
         pty_tx: Notifier,
-        info: PtyProcessInfo,
+        info: Arc<PtyProcessInfo>,
     },
     DisplayOnly,
 }
@@ -978,10 +978,8 @@ impl Terminal {
             AlacTermEvent::Wakeup => {
                 cx.emit(Event::Wakeup);
 
-                if let TerminalType::Pty { info, .. } = &mut self.terminal_type {
-                    if info.has_changed() {
-                        cx.emit(Event::TitleChanged);
-                    }
+                if let TerminalType::Pty { info, .. } = &self.terminal_type {
+                    info.emit_title_changed_if_changed(cx);
                 }
             }
             AlacTermEvent::ColorRequest(index, format) => {
@@ -2104,9 +2102,11 @@ impl Terminal {
     /// remote host, in case Zed is connected to a remote host.
     fn client_side_working_directory(&self) -> Option<PathBuf> {
         match &self.terminal_type {
-            TerminalType::Pty { info, .. } => {
-                info.current.as_ref().map(|process| process.cwd.clone())
-            }
+            TerminalType::Pty { info, .. } => info
+                .current
+                .read()
+                .as_ref()
+                .map(|process| process.cwd.clone()),
             TerminalType::DisplayOnly => None,
         }
     }
@@ -2128,6 +2128,7 @@ impl Terminal {
                 .unwrap_or_else(|| match &self.terminal_type {
                     TerminalType::Pty { info, .. } => info
                         .current
+                        .read()
                         .as_ref()
                         .map(|fpi| {
                             let process_file = fpi
@@ -2166,7 +2167,7 @@ impl Terminal {
         if let Some(task) = self.task()
             && task.status == TaskStatus::Running
         {
-            if let TerminalType::Pty { info, .. } = &mut self.terminal_type {
+            if let TerminalType::Pty { info, .. } = &self.terminal_type {
                 // First kill the foreground process group (the command running in the shell)
                 info.kill_current_process();
                 // Then kill the shell itself so that the terminal exits properly
@@ -2388,7 +2389,7 @@ unsafe fn append_text_to_term(term: &mut Term<ZedListener>, text_lines: &[&str])
 
 impl Drop for Terminal {
     fn drop(&mut self) {
-        if let TerminalType::Pty { pty_tx, mut info } =
+        if let TerminalType::Pty { pty_tx, info } =
             std::mem::replace(&mut self.terminal_type, TerminalType::DisplayOnly)
         {
             pty_tx.0.send(Msg::Shutdown).ok();