From 83538be46ec4d5903d83282d7295f3d351b329c3 Mon Sep 17 00:00:00 2001 From: alex-ds13 <145657253+alex-ds13@users.noreply.github.com> Date: Fri, 21 Feb 2025 18:47:04 +0000 Subject: [PATCH] perf(reaper): switch to channel notifications This commit changes the way the reaper works. First this commit changed the `known_hwnds` held by the `WindowManager` to be a HashMap of window handles (isize) to a pair of monitor_idx, workspace_idx (usize, usize). This commit then changes the reaper to have a cache of hwnds which is updated by the `WindowManager` when they change. The reaper has a thread that is continuously checking this cache to see if there is any window handle that no longer exists. When it finds them, the thread sends a notification to a channel which is then received by the reaper on another thread that actually does the work on the `WindowManager` by removing said windows. This means that the reaper no longer tries to access and lock the `WindowManager` every second like it used to, but instead it only does it when it actually needs, when a window actually needs to be reaped. This means that we can make the thread that checks for orphan windows run much more frequently since it won't influence the rest of komorebi. Since now the `known_hwnds` have the monitor/workspace index pair of the window, we can simply get that info from the map and immediately access that monitor/workspace or use that index info. --- komorebi/src/main.rs | 2 +- komorebi/src/monitor_reconciliator/mod.rs | 11 +- komorebi/src/process_command.rs | 3 + komorebi/src/process_event.rs | 105 ++++-------- komorebi/src/reaper.rs | 199 +++++++++++++++++++--- komorebi/src/static_config.rs | 2 +- komorebi/src/window_manager.rs | 68 +++++++- 7 files changed, 284 insertions(+), 106 deletions(-) diff --git a/komorebi/src/main.rs b/komorebi/src/main.rs index b8ad0e0e..7d60d2af 100644 --- a/komorebi/src/main.rs +++ b/komorebi/src/main.rs @@ -291,7 +291,7 @@ fn main() -> Result<()> { transparency_manager::listen_for_notifications(wm.clone()); workspace_reconciliator::listen_for_notifications(wm.clone()); monitor_reconciliator::listen_for_notifications(wm.clone())?; - reaper::watch_for_orphans(wm.clone()); + reaper::listen_for_notifications(wm.clone()); focus_manager::listen_for_notifications(wm.clone()); theme_manager::listen_for_notifications(); diff --git a/komorebi/src/monitor_reconciliator/mod.rs b/komorebi/src/monitor_reconciliator/mod.rs index cbbd6aa2..636b4d47 100644 --- a/komorebi/src/monitor_reconciliator/mod.rs +++ b/komorebi/src/monitor_reconciliator/mod.rs @@ -386,7 +386,7 @@ pub fn handle_notifications(wm: Arc>) -> color_eyre::Result } // Update known_hwnds - wm.known_hwnds.retain(|i| !windows_to_remove.contains(i)); + wm.known_hwnds.retain(|i, _| !windows_to_remove.contains(i)); if !newly_removed_displays.is_empty() { // After we have cached them, remove them from our state @@ -504,7 +504,7 @@ pub fn handle_notifications(wm: Arc>) -> color_eyre::Result { container.windows_mut().retain(|window| { window.exe().is_ok() - && !known_hwnds.contains(&window.hwnd) + && !known_hwnds.contains_key(&window.hwnd) }); if container.windows().is_empty() { @@ -542,7 +542,7 @@ pub fn handle_notifications(wm: Arc>) -> color_eyre::Result if let Some(window) = workspace.maximized_window() { if window.exe().is_err() - || known_hwnds.contains(&window.hwnd) + || known_hwnds.contains_key(&window.hwnd) { workspace.set_maximized_window(None); } else if is_focused_workspace { @@ -553,7 +553,7 @@ pub fn handle_notifications(wm: Arc>) -> color_eyre::Result if let Some(container) = workspace.monocle_container_mut() { container.windows_mut().retain(|window| { window.exe().is_ok() - && !known_hwnds.contains(&window.hwnd) + && !known_hwnds.contains_key(&window.hwnd) }); if container.windows().is_empty() { @@ -575,7 +575,8 @@ pub fn handle_notifications(wm: Arc>) -> color_eyre::Result } workspace.floating_windows_mut().retain(|window| { - window.exe().is_ok() && !known_hwnds.contains(&window.hwnd) + window.exe().is_ok() + && !known_hwnds.contains_key(&window.hwnd) }); if is_focused_workspace { diff --git a/komorebi/src/process_command.rs b/komorebi/src/process_command.rs index 7f87c8d7..b1149f44 100644 --- a/komorebi/src/process_command.rs +++ b/komorebi/src/process_command.rs @@ -1904,6 +1904,9 @@ impl WindowManager { | SocketMessage::IdentifyBorderOverflowApplication(_, _) => {} }; + // Update list of known_hwnds and their monitor/workspace index pair + self.update_known_hwnds(); + notify_subscribers( Notification { event: NotificationEvent::Socket(message.clone()), diff --git a/komorebi/src/process_event.rs b/komorebi/src/process_event.rs index af471341..298d6d8d 100644 --- a/komorebi/src/process_event.rs +++ b/komorebi/src/process_event.rs @@ -1,4 +1,3 @@ -use std::fs::OpenOptions; use std::sync::atomic::Ordering; use std::sync::Arc; use std::time::Duration; @@ -34,7 +33,6 @@ use crate::workspace_reconciliator::ALT_TAB_HWND_INSTANT; use crate::Notification; use crate::NotificationEvent; use crate::State; -use crate::DATA_DIR; use crate::FLOATING_APPLICATIONS; use crate::HIDDEN_HWNDS; use crate::REGEX_IDENTIFIERS; @@ -310,30 +308,28 @@ impl WindowManager { let mut needs_reconciliation = false; - for (i, monitors) in self.monitors().iter().enumerate() { - for (j, workspace) in monitors.workspaces().iter().enumerate() { - if workspace.contains_window(window.hwnd) && focused_pair != (i, j) { - // At this point we know we are going to send a notification to the workspace reconciliator - // So we get the topmost window returned by EnumWindows, which is almost always the window - // that has been selected by alt-tab - if let Ok(alt_tab_windows) = WindowsApi::alt_tab_windows() { - if let Some(first) = - alt_tab_windows.iter().find(|w| w.title().is_ok()) - { - // If our record of this HWND hasn't been updated in over a minute - let mut instant = ALT_TAB_HWND_INSTANT.lock(); - if instant.elapsed().gt(&Duration::from_secs(1)) { - // Update our record with the HWND we just found - ALT_TAB_HWND.store(Some(first.hwnd)); - // Update the timestamp of our record - *instant = Instant::now(); - } + if let Some((m_idx, w_idx)) = self.known_hwnds.get(&window.hwnd) { + if focused_pair != (*m_idx, *w_idx) { + // At this point we know we are going to send a notification to the workspace reconciliator + // So we get the topmost window returned by EnumWindows, which is almost always the window + // that has been selected by alt-tab + if let Ok(alt_tab_windows) = WindowsApi::alt_tab_windows() { + if let Some(first) = + alt_tab_windows.iter().find(|w| w.title().is_ok()) + { + // If our record of this HWND hasn't been updated in over a minute + let mut instant = ALT_TAB_HWND_INSTANT.lock(); + if instant.elapsed().gt(&Duration::from_secs(1)) { + // Update our record with the HWND we just found + ALT_TAB_HWND.store(Some(first.hwnd)); + // Update the timestamp of our record + *instant = Instant::now(); } } - - workspace_reconciliator::send_notification(i, j); - needs_reconciliation = true; } + + workspace_reconciliator::send_notification(*m_idx, *w_idx); + needs_reconciliation = true; } } @@ -344,11 +340,14 @@ impl WindowManager { // duplicates across multiple workspaces, as it results in ghost layout tiles. let mut proceed = true; - for (i, monitor) in self.monitors().iter().enumerate() { - for (j, workspace) in monitor.workspaces().iter().enumerate() { - if workspace.contains_window(window.hwnd) - && i != self.focused_monitor_idx() - && j != monitor.focused_workspace_idx() + if let Some((m_idx, w_idx)) = self.known_hwnds.get(&window.hwnd) { + if let Some(focused_workspace_idx) = self + .monitors() + .get(*m_idx) + .map(|m| m.focused_workspace_idx()) + { + if *m_idx != self.focused_monitor_idx() + && *w_idx != focused_workspace_idx { tracing::debug!( "ignoring show event for window already associated with another workspace" @@ -510,15 +509,9 @@ impl WindowManager { // This will be true if we have moved to another monitor let mut moved_across_monitors = false; - for (i, monitors) in self.monitors().iter().enumerate() { - for workspace in monitors.workspaces() { - if workspace.contains_window(window.hwnd) && i != target_monitor_idx { - moved_across_monitors = true; - break; - } - } - if moved_across_monitors { - break; + if let Some((m_idx, _)) = self.known_hwnds.get(&window.hwnd) { + if *m_idx != target_monitor_idx { + moved_across_monitors = true; } } @@ -722,42 +715,8 @@ impl WindowManager { window.center(&self.focused_monitor_work_area()?)?; } - tracing::trace!("updating list of known hwnds"); - let mut known_hwnds = vec![]; - for monitor in self.monitors() { - for workspace in monitor.workspaces() { - for container in workspace.containers() { - for window in container.windows() { - known_hwnds.push(window.hwnd); - } - } - - for window in workspace.floating_windows() { - known_hwnds.push(window.hwnd); - } - - if let Some(window) = workspace.maximized_window() { - known_hwnds.push(window.hwnd); - } - - if let Some(container) = workspace.monocle_container() { - for window in container.windows() { - known_hwnds.push(window.hwnd); - } - } - } - } - - let hwnd_json = DATA_DIR.join("komorebi.hwnd.json"); - let file = OpenOptions::new() - .write(true) - .truncate(true) - .create(true) - .open(hwnd_json)?; - - serde_json::to_writer_pretty(&file, &known_hwnds)?; - - self.known_hwnds = known_hwnds; + // Update list of known_hwnds and their monitor/workspace index pair + self.update_known_hwnds(); notify_subscribers( Notification { diff --git a/komorebi/src/reaper.rs b/komorebi/src/reaper.rs index 1bd39635..ef919db7 100644 --- a/komorebi/src/reaper.rs +++ b/komorebi/src/reaper.rs @@ -1,14 +1,164 @@ #![deny(clippy::unwrap_used, clippy::expect_used)] use crate::border_manager; +use crate::notify_subscribers; +use crate::winevent::WinEvent; +use crate::NotificationEvent; +use crate::Window; use crate::WindowManager; +use crate::WindowManagerEvent; +use crate::DATA_DIR; + +use crossbeam_channel::Receiver; +use crossbeam_channel::Sender; +use lazy_static::lazy_static; use parking_lot::Mutex; +use std::collections::HashMap; +use std::fs::OpenOptions; use std::sync::Arc; +use std::sync::OnceLock; use std::time::Duration; -pub fn watch_for_orphans(wm: Arc>) { +lazy_static! { + pub static ref HWNDS_CACHE: Arc>> = + Arc::new(Mutex::new(HashMap::new())); +} + +pub struct ReaperNotification(pub HashMap); + +static CHANNEL: OnceLock<(Sender, Receiver)> = + OnceLock::new(); + +pub fn channel() -> &'static (Sender, Receiver) { + CHANNEL.get_or_init(|| crossbeam_channel::bounded(50)) +} + +fn event_tx() -> Sender { + channel().0.clone() +} + +fn event_rx() -> Receiver { + channel().1.clone() +} + +pub fn send_notification(hwnds: HashMap) { + if event_tx().try_send(ReaperNotification(hwnds)).is_err() { + tracing::warn!("channel is full; dropping notification") + } +} + +pub fn listen_for_notifications(wm: Arc>) { + watch_for_orphans(wm.clone()); + std::thread::spawn(move || loop { - match find_orphans(wm.clone()) { + match handle_notifications(wm.clone()) { + Ok(()) => { + tracing::warn!("restarting finished thread"); + } + Err(error) => { + tracing::warn!("restarting failed thread: {}", error); + } + } + }); +} + +fn handle_notifications(wm: Arc>) -> color_eyre::Result<()> { + tracing::info!("listening"); + + let receiver = event_rx(); + + for notification in receiver { + let orphan_hwnds = notification.0; + let mut wm = wm.lock(); + let offset = wm.work_area_offset; + + let mut update_borders = false; + + for (hwnd, (m_idx, w_idx)) in orphan_hwnds.iter() { + if let Some(monitor) = wm.monitors_mut().get_mut(*m_idx) { + let focused_workspace_idx = monitor.focused_workspace_idx(); + let work_area = *monitor.work_area_size(); + let window_based_work_area_offset = ( + monitor.window_based_work_area_offset_limit(), + monitor.window_based_work_area_offset(), + ); + + let offset = if monitor.work_area_offset().is_some() { + monitor.work_area_offset() + } else { + offset + }; + + if let Some(workspace) = monitor.workspaces_mut().get_mut(*w_idx) { + // Remove orphan window + if let Err(error) = workspace.remove_window(*hwnd) { + tracing::warn!( + "error reaping orphan window ({}) on monitor: {}, workspace: {}. Error: {}", + hwnd, + m_idx, + w_idx, + error, + ); + } + + if focused_workspace_idx == *w_idx { + // If this is not a focused workspace there is no need to update the + // workspace or the borders. That will already be done when the user + // changes to this workspace. + workspace.update(&work_area, offset, window_based_work_area_offset)?; + update_borders = true; + } + tracing::info!( + "reaped orphan window ({}) on monitor: {}, workspace: {}", + hwnd, + m_idx, + w_idx, + ); + } + } + + wm.known_hwnds.remove(hwnd); + + let window = Window::from(*hwnd); + notify_subscribers( + crate::Notification { + event: NotificationEvent::WindowManager(WindowManagerEvent::Destroy( + WinEvent::ObjectDestroy, + window, + )), + state: wm.as_ref().into(), + }, + true, + )?; + } + + if update_borders { + border_manager::send_notification(None); + } + + // Save to file + let hwnd_json = DATA_DIR.join("komorebi.hwnd.json"); + let file = OpenOptions::new() + .write(true) + .truncate(true) + .create(true) + .open(hwnd_json)?; + + serde_json::to_writer_pretty(&file, &wm.known_hwnds.keys().collect::>())?; + } + + Ok(()) +} + +fn watch_for_orphans(wm: Arc>) { + // Cache current hwnds + { + let mut cache = HWNDS_CACHE.lock(); + *cache = wm.lock().known_hwnds.clone(); + } + + std::thread::spawn(move || loop { + match find_orphans() { Ok(()) => { tracing::warn!("restarting finished thread"); } @@ -23,36 +173,37 @@ pub fn watch_for_orphans(wm: Arc>) { }); } -pub fn find_orphans(wm: Arc>) -> color_eyre::Result<()> { +fn find_orphans() -> color_eyre::Result<()> { tracing::info!("watching"); - let arc = wm.clone(); - loop { - std::thread::sleep(Duration::from_secs(1)); + std::thread::sleep(Duration::from_millis(20)); - let mut wm = arc.lock(); - let mut update_borders = false; + let mut cache = HWNDS_CACHE.lock(); + let mut orphan_hwnds = HashMap::new(); - for (i, monitor) in wm.monitors_mut().iter_mut().enumerate() { - for (j, workspace) in monitor.workspaces_mut().iter_mut().enumerate() { - let reaped_orphans = workspace.reap_orphans()?; - if reaped_orphans.0 > 0 || reaped_orphans.1 > 0 { - workspace.update()?; - update_borders = true; - tracing::info!( - "reaped {} orphan window(s) and {} orphaned container(s) on monitor: {}, workspace: {}", - reaped_orphans.0, - reaped_orphans.1, - i, - j - ); - } + for (hwnd, (m_idx, w_idx)) in cache.iter() { + let window = Window::from(*hwnd); + + if !window.is_window() + // This one is a hack because WINWORD.EXE is an absolute trainwreck of an app + // when multiple docs are open, it keeps open an invisible window, with WS_EX_LAYERED + // (A STYLE THAT THE REGULAR WINDOWS NEED IN ORDER TO BE MANAGED!) when one of the + // docs is closed + // + // I hate every single person who worked on Microsoft Office 365, especially Word + || !window.is_visible() + { + orphan_hwnds.insert(window.hwnd, (*m_idx, *w_idx)); } } - if update_borders { - border_manager::send_notification(None); + if !orphan_hwnds.is_empty() { + // Update reaper cache + cache.retain(|h, _| !orphan_hwnds.contains_key(h)); + + // Send handles to remove + event_tx().send(ReaperNotification(orphan_hwnds))?; } } } diff --git a/komorebi/src/static_config.rs b/komorebi/src/static_config.rs index cf8eba06..84770773 100644 --- a/komorebi/src/static_config.rs +++ b/komorebi/src/static_config.rs @@ -1235,7 +1235,7 @@ impl StaticConfig { pending_move_op: Arc::new(None), already_moved_window_handles: Arc::new(Mutex::new(HashSet::new())), uncloack_to_ignore: 0, - known_hwnds: Vec::new(), + known_hwnds: HashMap::new(), }; match value.focus_follows_mouse { diff --git a/komorebi/src/window_manager.rs b/komorebi/src/window_manager.rs index 074bca64..aaebe47d 100644 --- a/komorebi/src/window_manager.rs +++ b/komorebi/src/window_manager.rs @@ -2,6 +2,7 @@ use std::collections::HashMap; use std::collections::HashSet; use std::collections::VecDeque; use std::env::temp_dir; +use std::fs::OpenOptions; use std::io::ErrorKind; use std::net::Shutdown; use std::num::NonZeroUsize; @@ -120,7 +121,8 @@ pub struct WindowManager { pub pending_move_op: Arc>, pub already_moved_window_handles: Arc>>, pub uncloack_to_ignore: usize, - pub known_hwnds: Vec, + /// Maps each known window hwnd to the (monitor, workspace) index pair managing it + pub known_hwnds: HashMap, } #[allow(clippy::struct_excessive_bools)] @@ -431,7 +433,7 @@ impl WindowManager { pending_move_op: Arc::new(None), already_moved_window_handles: Arc::new(Mutex::new(HashSet::new())), uncloack_to_ignore: 0, - known_hwnds: Vec::new(), + known_hwnds: HashMap::new(), }) } @@ -3591,4 +3593,66 @@ impl WindowManager { .focused_window_mut() .ok_or_else(|| anyhow!("there is no window")) } + + /// Updates the list of `known_hwnds` and their monitor/workspace index pair + /// + /// [`known_hwnds`]: `Self.known_hwnds` + pub fn update_known_hwnds(&mut self) { + tracing::trace!("updating list of known hwnds"); + let mut known_hwnds = HashMap::new(); + for (m_idx, monitor) in self.monitors().iter().enumerate() { + for (w_idx, workspace) in monitor.workspaces().iter().enumerate() { + for container in workspace.containers() { + for window in container.windows() { + known_hwnds.insert(window.hwnd, (m_idx, w_idx)); + } + } + + for window in workspace.floating_windows() { + known_hwnds.insert(window.hwnd, (m_idx, w_idx)); + } + + if let Some(window) = workspace.maximized_window() { + known_hwnds.insert(window.hwnd, (m_idx, w_idx)); + } + + if let Some(container) = workspace.monocle_container() { + for window in container.windows() { + known_hwnds.insert(window.hwnd, (m_idx, w_idx)); + } + } + } + } + + if self.known_hwnds != known_hwnds { + // Update reaper cache + { + let mut reaper_cache = crate::reaper::HWNDS_CACHE.lock(); + *reaper_cache = known_hwnds.clone(); + } + + // Save to file + let hwnd_json = DATA_DIR.join("komorebi.hwnd.json"); + match OpenOptions::new() + .write(true) + .truncate(true) + .create(true) + .open(hwnd_json) + { + Ok(file) => { + if let Err(error) = + serde_json::to_writer_pretty(&file, &known_hwnds.keys().collect::>()) + { + tracing::error!("Failed to save list of known_hwnds on file: {}", error); + } + } + Err(error) => { + tracing::error!("Failed to save list of known_hwnds on file: {}", error); + } + } + + // Store new hwnds + self.known_hwnds = known_hwnds; + } + } }