From 3d327c407c27324d0acd623f9bdcf47fca56ee86 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 | 211 ++++++++++++++++++---- komorebi/src/static_config.rs | 2 +- komorebi/src/window_manager.rs | 68 ++++++- 7 files changed, 283 insertions(+), 119 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 4c760f4c..99d60b23 100644 --- a/komorebi/src/monitor_reconciliator/mod.rs +++ b/komorebi/src/monitor_reconciliator/mod.rs @@ -385,7 +385,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 @@ -481,7 +481,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() { @@ -519,7 +519,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 { @@ -530,7 +530,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() { @@ -552,7 +552,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 b14c7b51..bf49ea45 100644 --- a/komorebi/src/process_command.rs +++ b/komorebi/src/process_command.rs @@ -1835,6 +1835,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 b71f3bb4..63a43d21 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; @@ -33,7 +32,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; @@ -307,30 +305,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; } } @@ -341,11 +337,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" @@ -504,15 +503,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; } } @@ -716,42 +709,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 6ca14e2f..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,50 +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 offset = wm.work_area_offset; + let mut cache = HWNDS_CACHE.lock(); + let mut orphan_hwnds = HashMap::new(); - let mut update_borders = false; + for (hwnd, (m_idx, w_idx)) in cache.iter() { + let window = Window::from(*hwnd); - for (i, monitor) in wm.monitors_mut().iter_mut().enumerate() { - 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 - }; - - 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(&work_area, offset, window_based_work_area_offset)?; - update_borders = true; - tracing::info!( - "reaped {} orphan window(s) and {} orphaned container(s) on monitor: {}, workspace: {}", - reaped_orphans.0, - reaped_orphans.1, - i, - j - ); - } + 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 45f24a18..53ae8252 100644 --- a/komorebi/src/static_config.rs +++ b/komorebi/src/static_config.rs @@ -1208,7 +1208,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 89f7d4ec..c5ff2698 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; @@ -117,7 +118,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)] @@ -424,7 +426,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(), }) } @@ -3361,4 +3363,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; + } + } }