From c17b48782bf47c94b958775d456e318d3a52ab1a Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 8 Jan 2026 19:04:18 -0800 Subject: [PATCH] Revert "Make tasks inherit their callers priority" (#46413) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverts zed-industries/zed#46179 ⚠️ Don't merge this until we confirm that it fixes the performance regression when saving buffers while scrolling ⚠️ --- crates/git_ui/src/git_panel.rs | 7 +- crates/gpui/src/executor.rs | 46 +++---- crates/gpui/src/platform.rs | 75 ++--------- crates/gpui/src/platform/linux/dispatcher.rs | 98 ++++++++++++--- .../src/platform/linux/headless/client.rs | 5 +- crates/gpui/src/platform/linux/platform.rs | 10 +- .../gpui/src/platform/linux/wayland/client.rs | 41 +++++- crates/gpui/src/platform/linux/x11/client.rs | 34 ++++- crates/gpui/src/platform/mac/dispatcher.rs | 117 ++++++++++++++---- crates/gpui/src/platform/test/dispatcher.rs | 33 +++-- .../gpui/src/platform/windows/dispatcher.rs | 81 ++++++++---- crates/gpui/src/platform/windows/events.rs | 2 +- crates/gpui/src/platform/windows/platform.rs | 13 +- crates/gpui/src/platform/windows/window.rs | 4 +- crates/repl/src/repl.rs | 6 +- 15 files changed, 365 insertions(+), 207 deletions(-) diff --git a/crates/git_ui/src/git_panel.rs b/crates/git_ui/src/git_panel.rs index 5f98b547e1eee58df24fb3ce4f2d852abf0d8c70..343875c60dcc16c23007e37c42ffe8f74e5e3609 100644 --- a/crates/git_ui/src/git_panel.rs +++ b/crates/git_ui/src/git_panel.rs @@ -80,7 +80,6 @@ use workspace::{ dock::{DockPosition, Panel, PanelEvent}, notifications::{DetachAndPromptErr, ErrorMessagePrompt, NotificationId, NotifyResultExt}, }; -use ztracing::instrument; actions!( git_panel, [ @@ -1198,7 +1197,6 @@ impl GitPanel { self.selected_entry.and_then(|i| self.entries.get(i)) } - #[instrument(skip_all)] fn open_diff(&mut self, _: &menu::Confirm, window: &mut Window, cx: &mut Context) { maybe!({ let entry = self.entries.get(self.selected_entry?)?.status_entry()?; @@ -1249,7 +1247,6 @@ impl GitPanel { }); } - #[instrument(skip_all)] fn open_file( &mut self, _: &menu::SecondaryConfirm, @@ -5080,9 +5077,9 @@ impl GitPanel { this.selected_entry = Some(ix); cx.notify(); if event.modifiers().secondary() { - this.open_file(&Default::default(), window, cx) // here? + this.open_file(&Default::default(), window, cx) } else { - this.open_diff(&Default::default(), window, cx); // here? + this.open_diff(&Default::default(), window, cx); this.focus_handle.focus(window, cx); } }) diff --git a/crates/gpui/src/executor.rs b/crates/gpui/src/executor.rs index 57b199e1dc01226638a78dd57a7cb2b3f7455148..fedd285dca4939aadac2ab1a050291430288e60b 100644 --- a/crates/gpui/src/executor.rs +++ b/crates/gpui/src/executor.rs @@ -1,10 +1,9 @@ -use crate::{App, GpuiRunnable, PlatformDispatcher, RunnableMeta, TaskTiming, profiler}; +use crate::{App, PlatformDispatcher, RunnableMeta, RunnableVariant, TaskTiming, profiler}; use async_task::Runnable; use futures::channel::mpsc; use parking_lot::{Condvar, Mutex}; use smol::prelude::*; use std::{ - cell::Cell, fmt::Debug, marker::PhantomData, mem::{self, ManuallyDrop}, @@ -82,21 +81,7 @@ pub enum Priority { Low, } -thread_local! { -static CURRENT_TASKS_PRIORITY: Cell = const { Cell::new(Priority::Medium) }; } - impl Priority { - /// Sets the priority any spawn call from the runnable about - /// to be run will use - pub(crate) fn set_as_default_for_spawns(&self) { - CURRENT_TASKS_PRIORITY.set(*self); - } - - /// Returns the priority from the currently running task - pub fn inherit() -> Self { - CURRENT_TASKS_PRIORITY.get() - } - #[allow(dead_code)] pub(crate) const fn probability(&self) -> u32 { match self { @@ -344,14 +329,19 @@ impl BackgroundExecutor { .metadata(RunnableMeta { location, app: None, - priority: Priority::inherit(), }) .spawn_unchecked( move |_| async { let _notify_guard = NotifyOnDrop(pair); future.await }, - move |runnable| dispatcher.dispatch(GpuiRunnable::GpuiSpawned(runnable), None), + move |runnable| { + dispatcher.dispatch( + RunnableVariant::Meta(runnable), + None, + Priority::default(), + ) + }, ) }; runnable.schedule(); @@ -397,7 +387,6 @@ impl BackgroundExecutor { }; profiler::add_task_timing(timing); - Priority::Realtime(realtime).set_as_default_for_spawns(); runnable.run(); let end = Instant::now(); @@ -410,7 +399,6 @@ impl BackgroundExecutor { async_task::Builder::new() .metadata(RunnableMeta { location, - priority, app: None, }) .spawn( @@ -424,12 +412,13 @@ impl BackgroundExecutor { async_task::Builder::new() .metadata(RunnableMeta { location, - priority, app: None, }) .spawn( move |_| future, - move |runnable| dispatcher.dispatch(GpuiRunnable::GpuiSpawned(runnable), label), + move |runnable| { + dispatcher.dispatch(RunnableVariant::Meta(runnable), label, priority) + }, ) }; @@ -685,14 +674,11 @@ impl BackgroundExecutor { let (runnable, task) = async_task::Builder::new() .metadata(RunnableMeta { location, - priority: Priority::inherit(), app: None, }) .spawn(move |_| async move {}, { let dispatcher = self.dispatcher.clone(); - move |runnable| { - dispatcher.dispatch_after(duration, GpuiRunnable::GpuiSpawned(runnable)) - } + move |runnable| dispatcher.dispatch_after(duration, RunnableVariant::Meta(runnable)) }); runnable.schedule(); Task(TaskState::Spawned(task)) @@ -799,10 +785,7 @@ impl ForegroundExecutor { } } - /// Enqueues the given Task to run on the main thread at some point in the - /// future. This inherits the priority of the caller. Use - /// [`spawn_with_priority`](Self::spawn_with_priority) if you want to - /// overwrite that. + /// Enqueues the given Task to run on the main thread at some point in the future. #[track_caller] pub fn spawn(&self, future: impl Future + 'static) -> Task where @@ -848,11 +831,10 @@ impl ForegroundExecutor { let (runnable, task) = spawn_local_with_source_location( future, move |runnable| { - dispatcher.dispatch_on_main_thread(GpuiRunnable::GpuiSpawned(runnable)) + dispatcher.dispatch_on_main_thread(RunnableVariant::Meta(runnable), priority) }, RunnableMeta { location, - priority, app: Some(app), }, ); diff --git a/crates/gpui/src/platform.rs b/crates/gpui/src/platform.rs index b37184ed151c473419a59e29d273c6f33cc0f988..d91ccbf0c6d692eb4acd1a2596311917b2f47448 100644 --- a/crates/gpui/src/platform.rs +++ b/crates/gpui/src/platform.rs @@ -596,8 +596,7 @@ pub(crate) trait PlatformWindow: HasWindowHandle + HasDisplayHandle { /// be considered part of our public API. #[doc(hidden)] pub struct RunnableMeta { - pub priority: Priority, - /// Location of the runnable, only set for futures we spawn + /// Location of the runnable pub location: &'static core::panic::Location<'static>, /// Weak reference to check if the app is still alive before running this task pub app: Option>, @@ -606,7 +605,6 @@ pub struct RunnableMeta { impl std::fmt::Debug for RunnableMeta { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("RunnableMeta") - .field("priority", &self.priority) .field("location", &self.location) .field("app_alive", &self.is_app_alive()) .finish() @@ -623,69 +621,10 @@ impl RunnableMeta { } } -/// Both of these need meta for priority #[doc(hidden)] -pub enum GpuiRunnable { - /// Spawned by us, we set useful metadata for profiling and scheduling. - /// Yay we have nice things! - GpuiSpawned(Runnable), - /// Spawned by a dependency through runtimelib. We only have the - /// runnable ('task'). No access to metadata. - DependencySpawned(Runnable<()>), -} - -impl GpuiRunnable { - fn run_and_profile(self) -> Instant { - if self.app_dropped() { - // optimizer will cut it out if it doesnt it does not really matter - // cause we are closing the app anyway. - return Instant::now(); - } - - let mut timing = TaskTiming { - // use a placeholder location if we dont have one - location: self.location().unwrap_or(core::panic::Location::caller()), - start: Instant::now(), - end: None, - }; - - crate::profiler::add_task_timing(timing); - self.run_unprofiled(); // surrounded by profiling so its ok - timing.end = Some(Instant::now()); - crate::profiler::add_task_timing(timing); - timing.start - } - - fn app_dropped(&self) -> bool { - match self { - GpuiRunnable::GpuiSpawned(runnable) => !runnable.metadata().is_app_alive(), - GpuiRunnable::DependencySpawned(_) => false, - } - } - - fn location(&self) -> Option<&'static core::panic::Location<'static>> { - match self { - GpuiRunnable::GpuiSpawned(runnable) => runnable.metadata().location.into(), - GpuiRunnable::DependencySpawned(_) => None, - } - } - - /// ONLY use for tests or headless client. - /// ideally everything should be profiled - fn run_unprofiled(self) { - self.priority().set_as_default_for_spawns(); - match self { - GpuiRunnable::GpuiSpawned(r) => r.run(), - GpuiRunnable::DependencySpawned(r) => r.run(), - }; - } - - fn priority(&self) -> Priority { - match self { - GpuiRunnable::GpuiSpawned(r) => r.metadata().priority, - GpuiRunnable::DependencySpawned(_) => Priority::Medium, - } - } +pub enum RunnableVariant { + Meta(Runnable), + Compat(Runnable), } /// This type is public so that our test macro can generate and use it, but it should not @@ -695,9 +634,9 @@ pub trait PlatformDispatcher: Send + Sync { fn get_all_timings(&self) -> Vec; fn get_current_thread_timings(&self) -> Vec; fn is_main_thread(&self) -> bool; - fn dispatch(&self, runnable: GpuiRunnable, label: Option); - fn dispatch_on_main_thread(&self, runnable: GpuiRunnable); - fn dispatch_after(&self, duration: Duration, runnable: GpuiRunnable); + fn dispatch(&self, runnable: RunnableVariant, label: Option, priority: Priority); + fn dispatch_on_main_thread(&self, runnable: RunnableVariant, priority: Priority); + fn dispatch_after(&self, duration: Duration, runnable: RunnableVariant); fn spawn_realtime(&self, priority: RealtimePriority, f: Box); fn now(&self) -> Instant { diff --git a/crates/gpui/src/platform/linux/dispatcher.rs b/crates/gpui/src/platform/linux/dispatcher.rs index 40243be6bba941de0a8f01f0b56c51cda2adc9ab..c8ae7269edd495669baa6ab0e22e745917f143b2 100644 --- a/crates/gpui/src/platform/linux/dispatcher.rs +++ b/crates/gpui/src/platform/linux/dispatcher.rs @@ -5,22 +5,27 @@ use calloop::{ }; use util::ResultExt; -use std::{mem::MaybeUninit, thread, time::Duration}; +use std::{ + mem::MaybeUninit, + thread, + time::{Duration, Instant}, +}; use crate::{ - GLOBAL_THREAD_TIMINGS, GpuiRunnable, PlatformDispatcher, Priority, PriorityQueueReceiver, - PriorityQueueSender, RealtimePriority, THREAD_TIMINGS, TaskLabel, ThreadTaskTimings, + GLOBAL_THREAD_TIMINGS, PlatformDispatcher, Priority, PriorityQueueReceiver, + PriorityQueueSender, RealtimePriority, RunnableVariant, THREAD_TIMINGS, TaskLabel, TaskTiming, + ThreadTaskTimings, profiler, }; struct TimerAfter { duration: Duration, - runnable: GpuiRunnable, + runnable: RunnableVariant, } pub(crate) struct LinuxDispatcher { - main_sender: PriorityQueueCalloopSender, + main_sender: PriorityQueueCalloopSender, timer_sender: Sender, - background_sender: PriorityQueueSender, + background_sender: PriorityQueueSender, _background_threads: Vec>, main_thread_id: thread::ThreadId, } @@ -28,7 +33,7 @@ pub(crate) struct LinuxDispatcher { const MIN_THREADS: usize = 2; impl LinuxDispatcher { - pub fn new(main_sender: PriorityQueueCalloopSender) -> Self { + pub fn new(main_sender: PriorityQueueCalloopSender) -> Self { let (background_sender, background_receiver) = PriorityQueueReceiver::new(); let thread_count = std::thread::available_parallelism().map_or(MIN_THREADS, |i| i.get().max(MIN_THREADS)); @@ -37,16 +42,48 @@ impl LinuxDispatcher { // executor let mut background_threads = (0..thread_count) .map(|i| { - let mut receiver: PriorityQueueReceiver = background_receiver.clone(); + let mut receiver = background_receiver.clone(); std::thread::Builder::new() .name(format!("Worker-{i}")) .spawn(move || { for runnable in receiver.iter() { - let started = runnable.run_and_profile(); + let start = Instant::now(); + + let mut location = match runnable { + RunnableVariant::Meta(runnable) => { + let location = runnable.metadata().location; + let timing = TaskTiming { + location, + start, + end: None, + }; + profiler::add_task_timing(timing); + + runnable.run(); + timing + } + RunnableVariant::Compat(runnable) => { + let location = core::panic::Location::caller(); + let timing = TaskTiming { + location, + start, + end: None, + }; + profiler::add_task_timing(timing); + + runnable.run(); + timing + } + }; + + let end = Instant::now(); + location.end = Some(end); + profiler::add_task_timing(location); + log::trace!( "background thread {}: ran runnable. took: {:?}", i, - started.elapsed() + start.elapsed() ); } }) @@ -73,7 +110,36 @@ impl LinuxDispatcher { calloop::timer::Timer::from_duration(timer.duration), move |_, _, _| { if let Some(runnable) = runnable.take() { - runnable.run_and_profile(); + let start = Instant::now(); + let mut timing = match runnable { + RunnableVariant::Meta(runnable) => { + let location = runnable.metadata().location; + let timing = TaskTiming { + location, + start, + end: None, + }; + profiler::add_task_timing(timing); + + runnable.run(); + timing + } + RunnableVariant::Compat(runnable) => { + let timing = TaskTiming { + location: core::panic::Location::caller(), + start, + end: None, + }; + profiler::add_task_timing(timing); + + runnable.run(); + timing + } + }; + let end = Instant::now(); + + timing.end = Some(end); + profiler::add_task_timing(timing); } TimeoutAction::Drop }, @@ -123,15 +189,15 @@ impl PlatformDispatcher for LinuxDispatcher { thread::current().id() == self.main_thread_id } - fn dispatch(&self, runnable: GpuiRunnable, _: Option) { + fn dispatch(&self, runnable: RunnableVariant, _: Option, priority: Priority) { self.background_sender - .send(runnable.priority(), runnable) + .send(priority, runnable) .unwrap_or_else(|_| panic!("blocking sender returned without value")); } - fn dispatch_on_main_thread(&self, runnable: GpuiRunnable) { + fn dispatch_on_main_thread(&self, runnable: RunnableVariant, priority: Priority) { self.main_sender - .send(runnable.priority(), runnable) + .send(priority, runnable) .unwrap_or_else(|runnable| { // NOTE: Runnable may wrap a Future that is !Send. // @@ -145,7 +211,7 @@ impl PlatformDispatcher for LinuxDispatcher { }); } - fn dispatch_after(&self, duration: Duration, runnable: GpuiRunnable) { + fn dispatch_after(&self, duration: Duration, runnable: RunnableVariant) { self.timer_sender .send(TimerAfter { duration, runnable }) .ok(); diff --git a/crates/gpui/src/platform/linux/headless/client.rs b/crates/gpui/src/platform/linux/headless/client.rs index becedf920ae0754adb3f8eb4e0a3121fd4d7d03c..214e9c12aec9d8ae94f50ce00a88f091f04c635e 100644 --- a/crates/gpui/src/platform/linux/headless/client.rs +++ b/crates/gpui/src/platform/linux/headless/client.rs @@ -31,7 +31,10 @@ impl HeadlessClient { handle .insert_source(main_receiver, |event, _, _: &mut HeadlessClient| { if let calloop::channel::Event::Msg(runnable) = event { - runnable.run_unprofiled(); + match runnable { + crate::RunnableVariant::Meta(runnable) => runnable.run(), + crate::RunnableVariant::Compat(runnable) => runnable.run(), + }; } }) .ok(); diff --git a/crates/gpui/src/platform/linux/platform.rs b/crates/gpui/src/platform/linux/platform.rs index 1fd703bc4523a1743e055455a6bdacb45d3569f6..381ab7cf577ad4b83a81249156ccae5a732ac2c5 100644 --- a/crates/gpui/src/platform/linux/platform.rs +++ b/crates/gpui/src/platform/linux/platform.rs @@ -23,10 +23,10 @@ use xkbcommon::xkb::{self, Keycode, Keysym, State}; use crate::{ Action, AnyWindowHandle, BackgroundExecutor, ClipboardItem, CursorStyle, DisplayId, - ForegroundExecutor, GpuiRunnable, Keymap, LinuxDispatcher, Menu, MenuItem, OwnedMenu, - PathPromptOptions, Pixels, Platform, PlatformDisplay, PlatformKeyboardLayout, - PlatformKeyboardMapper, PlatformTextSystem, PlatformWindow, Point, - PriorityQueueCalloopReceiver, Result, Task, WindowAppearance, WindowParams, px, + ForegroundExecutor, Keymap, LinuxDispatcher, Menu, MenuItem, OwnedMenu, PathPromptOptions, + Pixels, Platform, PlatformDisplay, PlatformKeyboardLayout, PlatformKeyboardMapper, + PlatformTextSystem, PlatformWindow, Point, PriorityQueueCalloopReceiver, Result, + RunnableVariant, Task, WindowAppearance, WindowParams, px, }; #[cfg(any(feature = "wayland", feature = "x11"))] @@ -152,7 +152,7 @@ impl LinuxCommon { pub fn new( signal: LoopSignal, liveness: std::sync::Weak<()>, - ) -> (Self, PriorityQueueCalloopReceiver) { + ) -> (Self, PriorityQueueCalloopReceiver) { let (main_sender, main_receiver) = PriorityQueueCalloopReceiver::new(); #[cfg(any(feature = "wayland", feature = "x11"))] diff --git a/crates/gpui/src/platform/linux/wayland/client.rs b/crates/gpui/src/platform/linux/wayland/client.rs index bceb1eb0f9aa5d56ced975709655095e5a2c44a5..227791324efe550390c3679cd917d6ce145f277a 100644 --- a/crates/gpui/src/platform/linux/wayland/client.rs +++ b/crates/gpui/src/platform/linux/wayland/client.rs @@ -73,14 +73,17 @@ use super::{ window::{ImeInput, WaylandWindowStatePtr}, }; -use crate::platform::{PlatformWindow, blade::BladeContext}; use crate::{ AnyWindowHandle, Bounds, Capslock, CursorStyle, DOUBLE_CLICK_INTERVAL, DevicePixels, DisplayId, FileDropEvent, ForegroundExecutor, KeyDownEvent, KeyUpEvent, Keystroke, LinuxCommon, LinuxKeyboardLayout, Modifiers, ModifiersChangedEvent, MouseButton, MouseDownEvent, MouseExitEvent, MouseMoveEvent, MouseUpEvent, NavigationDirection, Pixels, PlatformDisplay, PlatformInput, PlatformKeyboardLayout, Point, ResultExt as _, SCROLL_LINES, ScrollDelta, - ScrollWheelEvent, Size, TouchPhase, WindowParams, point, px, size, + ScrollWheelEvent, Size, TouchPhase, WindowParams, point, profiler, px, size, +}; +use crate::{ + RunnableVariant, TaskTiming, + platform::{PlatformWindow, blade::BladeContext}, }; use crate::{ SharedString, @@ -495,8 +498,38 @@ impl WaylandClient { let handle = handle.clone(); move |event, _, _: &mut WaylandClientStatePtr| { if let calloop::channel::Event::Msg(runnable) = event { - handle.insert_idle(move |_| { - runnable.run_and_profile(); + handle.insert_idle(|_| { + let start = Instant::now(); + let mut timing = match runnable { + RunnableVariant::Meta(runnable) => { + let location = runnable.metadata().location; + let timing = TaskTiming { + location, + start, + end: None, + }; + profiler::add_task_timing(timing); + + runnable.run(); + timing + } + RunnableVariant::Compat(runnable) => { + let location = core::panic::Location::caller(); + let timing = TaskTiming { + location, + start, + end: None, + }; + profiler::add_task_timing(timing); + + runnable.run(); + timing + } + }; + + let end = Instant::now(); + timing.end = Some(end); + profiler::add_task_timing(timing); }); } } diff --git a/crates/gpui/src/platform/linux/x11/client.rs b/crates/gpui/src/platform/linux/x11/client.rs index 66e214c888703eeaa16e639917a5d9ae4dfe23ba..02e83518bc440cc9b01dd809f0f6ec86a7142fc2 100644 --- a/crates/gpui/src/platform/linux/x11/client.rs +++ b/crates/gpui/src/platform/linux/x11/client.rs @@ -1,4 +1,4 @@ -use crate::{Capslock, ResultExt as _, xcb_flush}; +use crate::{Capslock, ResultExt as _, RunnableVariant, TaskTiming, profiler, xcb_flush}; use anyhow::{Context as _, anyhow}; use ashpd::WindowIdentifier; use calloop::{ @@ -313,7 +313,37 @@ impl X11Client { // events have higher priority and runnables are only worked off after the event // callbacks. handle.insert_idle(|_| { - runnable.run_and_profile(); + let start = Instant::now(); + let mut timing = match runnable { + RunnableVariant::Meta(runnable) => { + let location = runnable.metadata().location; + let timing = TaskTiming { + location, + start, + end: None, + }; + profiler::add_task_timing(timing); + + runnable.run(); + timing + } + RunnableVariant::Compat(runnable) => { + let location = core::panic::Location::caller(); + let timing = TaskTiming { + location, + start, + end: None, + }; + profiler::add_task_timing(timing); + + runnable.run(); + timing + } + }; + + let end = Instant::now(); + timing.end = Some(end); + profiler::add_task_timing(timing); }); } } diff --git a/crates/gpui/src/platform/mac/dispatcher.rs b/crates/gpui/src/platform/mac/dispatcher.rs index ba9f37e1ccc2817db88afefa9adc96b06d38eecb..c10998ed3da49d1837929ab2ff93d19b035812df 100644 --- a/crates/gpui/src/platform/mac/dispatcher.rs +++ b/crates/gpui/src/platform/mac/dispatcher.rs @@ -3,8 +3,8 @@ #![allow(non_snake_case)] use crate::{ - GLOBAL_THREAD_TIMINGS, GpuiRunnable, PlatformDispatcher, Priority, RealtimePriority, - RunnableMeta, THREAD_TIMINGS, TaskLabel, TaskTiming, ThreadTaskTimings, + GLOBAL_THREAD_TIMINGS, PlatformDispatcher, Priority, RealtimePriority, RunnableMeta, + RunnableVariant, THREAD_TIMINGS, TaskLabel, TaskTiming, ThreadTaskTimings, }; use anyhow::Context; @@ -28,7 +28,7 @@ use std::{ ffi::c_void, mem::MaybeUninit, ptr::{NonNull, addr_of}, - time::Duration, + time::{Duration, Instant}, }; use util::ResultExt; @@ -69,25 +69,25 @@ impl PlatformDispatcher for MacDispatcher { is_main_thread == YES } - fn dispatch(&self, runnable: GpuiRunnable, _: Option) { - let queue_priority = match runnable.priority() { - Priority::Realtime(_) => unreachable!(), - Priority::High => DISPATCH_QUEUE_PRIORITY_HIGH as isize, - Priority::Medium => DISPATCH_QUEUE_PRIORITY_DEFAULT as isize, - Priority::Low => DISPATCH_QUEUE_PRIORITY_LOW as isize, - }; - + fn dispatch(&self, runnable: RunnableVariant, _: Option, priority: Priority) { let (context, trampoline) = match runnable { - GpuiRunnable::GpuiSpawned(runnable) => ( + RunnableVariant::Meta(runnable) => ( runnable.into_raw().as_ptr() as *mut c_void, Some(trampoline as unsafe extern "C" fn(*mut c_void)), ), - GpuiRunnable::DependencySpawned(runnable) => ( + RunnableVariant::Compat(runnable) => ( runnable.into_raw().as_ptr() as *mut c_void, Some(trampoline_compat as unsafe extern "C" fn(*mut c_void)), ), }; + let queue_priority = match priority { + Priority::Realtime(_) => unreachable!(), + Priority::High => DISPATCH_QUEUE_PRIORITY_HIGH as isize, + Priority::Medium => DISPATCH_QUEUE_PRIORITY_DEFAULT as isize, + Priority::Low => DISPATCH_QUEUE_PRIORITY_LOW as isize, + }; + unsafe { dispatch_async_f( dispatch_get_global_queue(queue_priority, 0), @@ -97,13 +97,13 @@ impl PlatformDispatcher for MacDispatcher { } } - fn dispatch_on_main_thread(&self, runnable: GpuiRunnable) { + fn dispatch_on_main_thread(&self, runnable: RunnableVariant, _priority: Priority) { let (context, trampoline) = match runnable { - GpuiRunnable::GpuiSpawned(runnable) => ( + RunnableVariant::Meta(runnable) => ( runnable.into_raw().as_ptr() as *mut c_void, Some(trampoline as unsafe extern "C" fn(*mut c_void)), ), - GpuiRunnable::DependencySpawned(runnable) => ( + RunnableVariant::Compat(runnable) => ( runnable.into_raw().as_ptr() as *mut c_void, Some(trampoline_compat as unsafe extern "C" fn(*mut c_void)), ), @@ -113,13 +113,13 @@ impl PlatformDispatcher for MacDispatcher { } } - fn dispatch_after(&self, duration: Duration, runnable: GpuiRunnable) { + fn dispatch_after(&self, duration: Duration, runnable: RunnableVariant) { let (context, trampoline) = match runnable { - GpuiRunnable::GpuiSpawned(runnable) => ( + RunnableVariant::Meta(runnable) => ( runnable.into_raw().as_ptr() as *mut c_void, Some(trampoline as unsafe extern "C" fn(*mut c_void)), ), - GpuiRunnable::DependencySpawned(runnable) => ( + RunnableVariant::Compat(runnable) => ( runnable.into_raw().as_ptr() as *mut c_void, Some(trampoline_compat as unsafe extern "C" fn(*mut c_void)), ), @@ -247,19 +247,82 @@ fn set_audio_thread_priority() -> anyhow::Result<()> { Ok(()) } -// Note we can not send through a GpuiRunnable as that would require allocating -// to keep the pointer alive. So we recreate it here. extern "C" fn trampoline(runnable: *mut c_void) { let task = unsafe { Runnable::::from_raw(NonNull::new_unchecked(runnable as *mut ())) }; - let task = GpuiRunnable::GpuiSpawned(task); - task.run_and_profile(); + + let metadata = task.metadata(); + let location = metadata.location; + + if !metadata.is_app_alive() { + drop(task); + return; + } + + let start = Instant::now(); + let timing = TaskTiming { + location, + start, + end: None, + }; + + THREAD_TIMINGS.with(|timings| { + let mut timings = timings.lock(); + let timings = &mut timings.timings; + if let Some(last_timing) = timings.iter_mut().rev().next() { + if last_timing.location == timing.location { + return; + } + } + + timings.push_back(timing); + }); + + task.run(); + let end = Instant::now(); + + THREAD_TIMINGS.with(|timings| { + let mut timings = timings.lock(); + let timings = &mut timings.timings; + let Some(last_timing) = timings.iter_mut().rev().next() else { + return; + }; + last_timing.end = Some(end); + }); } -// Note we can not send through a GpuiRunnable as that would require allocating -// to keep the pointer alive. So we recreate it here. extern "C" fn trampoline_compat(runnable: *mut c_void) { let task = unsafe { Runnable::<()>::from_raw(NonNull::new_unchecked(runnable as *mut ())) }; - let task = GpuiRunnable::DependencySpawned(task); - task.run_and_profile(); + + let location = core::panic::Location::caller(); + + let start = Instant::now(); + let timing = TaskTiming { + location, + start, + end: None, + }; + THREAD_TIMINGS.with(|timings| { + let mut timings = timings.lock(); + let timings = &mut timings.timings; + if let Some(last_timing) = timings.iter_mut().rev().next() { + if last_timing.location == timing.location { + return; + } + } + + timings.push_back(timing); + }); + + task.run(); + let end = Instant::now(); + + THREAD_TIMINGS.with(|timings| { + let mut timings = timings.lock(); + let timings = &mut timings.timings; + let Some(last_timing) = timings.iter_mut().rev().next() else { + return; + }; + last_timing.end = Some(end); + }); } diff --git a/crates/gpui/src/platform/test/dispatcher.rs b/crates/gpui/src/platform/test/dispatcher.rs index 05aa0687311c14b2c5c35c08c44373f789d3b54c..a94f0737f5e43d7196965995e382e8c964647702 100644 --- a/crates/gpui/src/platform/test/dispatcher.rs +++ b/crates/gpui/src/platform/test/dispatcher.rs @@ -1,4 +1,4 @@ -use crate::{GpuiRunnable, PlatformDispatcher, TaskLabel}; +use crate::{PlatformDispatcher, Priority, RunnableVariant, TaskLabel}; use backtrace::Backtrace; use collections::{HashMap, HashSet, VecDeque}; use parking::Unparker; @@ -25,10 +25,10 @@ pub struct TestDispatcher { struct TestDispatcherState { random: StdRng, - foreground: HashMap>, - background: Vec, - deprioritized_background: Vec, - delayed: Vec<(Duration, GpuiRunnable)>, + foreground: HashMap>, + background: Vec, + deprioritized_background: Vec, + delayed: Vec<(Duration, RunnableVariant)>, start_time: Instant, time: Duration, is_main_thread: bool, @@ -176,10 +176,21 @@ impl TestDispatcher { drop(state); // todo(localcc): add timings to tests - if !runnable.app_dropped() { - runnable.run_unprofiled() - } + match runnable { + RunnableVariant::Meta(runnable) => { + if !runnable.metadata().is_app_alive() { + drop(runnable); + } else { + runnable.run(); + } + } + RunnableVariant::Compat(runnable) => { + runnable.run(); + } + }; + self.state.lock().is_main_thread = was_main_thread; + true } @@ -281,7 +292,7 @@ impl PlatformDispatcher for TestDispatcher { state.start_time + state.time } - fn dispatch(&self, runnable: GpuiRunnable, label: Option) { + fn dispatch(&self, runnable: RunnableVariant, label: Option, _priority: Priority) { { let mut state = self.state.lock(); if label.is_some_and(|label| state.deprioritized_task_labels.contains(&label)) { @@ -293,7 +304,7 @@ impl PlatformDispatcher for TestDispatcher { self.unpark_all(); } - fn dispatch_on_main_thread(&self, runnable: GpuiRunnable) { + fn dispatch_on_main_thread(&self, runnable: RunnableVariant, _priority: Priority) { self.state .lock() .foreground @@ -303,7 +314,7 @@ impl PlatformDispatcher for TestDispatcher { self.unpark_all(); } - fn dispatch_after(&self, duration: std::time::Duration, runnable: GpuiRunnable) { + fn dispatch_after(&self, duration: std::time::Duration, runnable: RunnableVariant) { let mut state = self.state.lock(); let next_time = state.time + duration; let ix = match state.delayed.binary_search_by_key(&next_time, |e| e.0) { diff --git a/crates/gpui/src/platform/windows/dispatcher.rs b/crates/gpui/src/platform/windows/dispatcher.rs index 24990ce0613e3d0a27c137b72bb0f15e66bc24de..0720d414c9b44dec4a3bab5b50fd7dde47991989 100644 --- a/crates/gpui/src/platform/windows/dispatcher.rs +++ b/crates/gpui/src/platform/windows/dispatcher.rs @@ -1,7 +1,7 @@ use std::{ sync::atomic::{AtomicBool, Ordering}, thread::{ThreadId, current}, - time::Duration, + time::{Duration, Instant}, }; use anyhow::Context; @@ -21,14 +21,14 @@ use windows::{ }; use crate::{ - GLOBAL_THREAD_TIMINGS, GpuiRunnable, HWND, PlatformDispatcher, Priority, PriorityQueueSender, - RealtimePriority, SafeHwnd, THREAD_TIMINGS, TaskLabel, ThreadTaskTimings, - WM_GPUI_TASK_DISPATCHED_ON_MAIN_THREAD, + GLOBAL_THREAD_TIMINGS, HWND, PlatformDispatcher, Priority, PriorityQueueSender, + RealtimePriority, RunnableVariant, SafeHwnd, THREAD_TIMINGS, TaskLabel, TaskTiming, + ThreadTaskTimings, WM_GPUI_TASK_DISPATCHED_ON_MAIN_THREAD, profiler, }; pub(crate) struct WindowsDispatcher { pub(crate) wake_posted: AtomicBool, - main_sender: PriorityQueueSender, + main_sender: PriorityQueueSender, main_thread_id: ThreadId, pub(crate) platform_window_handle: SafeHwnd, validation_number: usize, @@ -36,7 +36,7 @@ pub(crate) struct WindowsDispatcher { impl WindowsDispatcher { pub(crate) fn new( - main_sender: PriorityQueueSender, + main_sender: PriorityQueueSender, platform_window_handle: HWND, validation_number: usize, ) -> Self { @@ -52,14 +52,11 @@ impl WindowsDispatcher { } } - fn dispatch_on_threadpool(&self, runnable: GpuiRunnable, priority: WorkItemPriority) { + fn dispatch_on_threadpool(&self, priority: WorkItemPriority, runnable: RunnableVariant) { let handler = { - let mut runnable = Some(runnable); + let mut task_wrapper = Some(runnable); WorkItemHandler::new(move |_| { - runnable - .take() - .expect("Takes FnMut but only runs once") - .run_and_profile(); + Self::execute_runnable(task_wrapper.take().unwrap()); Ok(()) }) }; @@ -67,19 +64,54 @@ impl WindowsDispatcher { ThreadPool::RunWithPriorityAsync(&handler, priority).log_err(); } - fn dispatch_on_threadpool_after(&self, runnable: GpuiRunnable, duration: Duration) { + fn dispatch_on_threadpool_after(&self, runnable: RunnableVariant, duration: Duration) { let handler = { - let mut runnable = Some(runnable); + let mut task_wrapper = Some(runnable); TimerElapsedHandler::new(move |_| { - runnable - .take() - .expect("Takes FnMut but only runs once") - .run_and_profile(); + Self::execute_runnable(task_wrapper.take().unwrap()); Ok(()) }) }; ThreadPoolTimer::CreateTimer(&handler, duration.into()).log_err(); } + + #[inline(always)] + pub(crate) fn execute_runnable(runnable: RunnableVariant) { + let start = Instant::now(); + + let mut timing = match runnable { + RunnableVariant::Meta(runnable) => { + let location = runnable.metadata().location; + let timing = TaskTiming { + location, + start, + end: None, + }; + profiler::add_task_timing(timing); + + runnable.run(); + + timing + } + RunnableVariant::Compat(runnable) => { + let timing = TaskTiming { + location: core::panic::Location::caller(), + start, + end: None, + }; + profiler::add_task_timing(timing); + + runnable.run(); + + timing + } + }; + + let end = Instant::now(); + timing.end = Some(end); + + profiler::add_task_timing(timing); + } } impl PlatformDispatcher for WindowsDispatcher { @@ -106,21 +138,22 @@ impl PlatformDispatcher for WindowsDispatcher { current().id() == self.main_thread_id } - fn dispatch(&self, runnable: GpuiRunnable, label: Option) { - let priority = match runnable.priority() { + fn dispatch(&self, runnable: RunnableVariant, label: Option, priority: Priority) { + let priority = match priority { Priority::Realtime(_) => unreachable!(), Priority::High => WorkItemPriority::High, Priority::Medium => WorkItemPriority::Normal, Priority::Low => WorkItemPriority::Low, }; - self.dispatch_on_threadpool(runnable, priority); + self.dispatch_on_threadpool(priority, runnable); + if let Some(label) = label { log::debug!("TaskLabel: {label:?}"); } } - fn dispatch_on_main_thread(&self, runnable: GpuiRunnable) { - match self.main_sender.send(runnable.priority(), runnable) { + fn dispatch_on_main_thread(&self, runnable: RunnableVariant, priority: Priority) { + match self.main_sender.send(priority, runnable) { Ok(_) => { if !self.wake_posted.swap(true, Ordering::AcqRel) { unsafe { @@ -148,7 +181,7 @@ impl PlatformDispatcher for WindowsDispatcher { } } - fn dispatch_after(&self, duration: Duration, runnable: GpuiRunnable) { + fn dispatch_after(&self, duration: Duration, runnable: RunnableVariant) { self.dispatch_on_threadpool_after(runnable, duration); } diff --git a/crates/gpui/src/platform/windows/events.rs b/crates/gpui/src/platform/windows/events.rs index 9ff2fc4e14cd947507eec3aaf2c1e6ca3c18df3e..97c3a12bb028ccf518c09da862a02846eca049f4 100644 --- a/crates/gpui/src/platform/windows/events.rs +++ b/crates/gpui/src/platform/windows/events.rs @@ -250,7 +250,7 @@ impl WindowsWindowInner { if wparam.0 == SIZE_MOVE_LOOP_TIMER_ID { let mut runnables = self.main_receiver.clone().try_iter(); while let Some(Ok(runnable)) = runnables.next() { - runnable.run_and_profile(); + WindowsDispatcher::execute_runnable(runnable); } self.handle_paint_msg(handle) } else { diff --git a/crates/gpui/src/platform/windows/platform.rs b/crates/gpui/src/platform/windows/platform.rs index 113a9c08a4871acf05e503c99439f30cb7270b2a..804715fd70c93cc65be732d97d39b3cae91f19da 100644 --- a/crates/gpui/src/platform/windows/platform.rs +++ b/crates/gpui/src/platform/windows/platform.rs @@ -51,7 +51,7 @@ struct WindowsPlatformInner { raw_window_handles: std::sync::Weak>>, // The below members will never change throughout the entire lifecycle of the app. validation_number: usize, - main_receiver: PriorityQueueReceiver, + main_receiver: PriorityQueueReceiver, dispatcher: Arc, } @@ -863,7 +863,7 @@ impl WindowsPlatformInner { } let mut main_receiver = self.main_receiver.clone(); match main_receiver.try_pop() { - Ok(Some(runnable)) => _ = runnable.run_and_profile(), + Ok(Some(runnable)) => WindowsDispatcher::execute_runnable(runnable), _ => break 'timeout_loop, } } @@ -875,7 +875,8 @@ impl WindowsPlatformInner { match main_receiver.try_pop() { Ok(Some(runnable)) => { self.dispatcher.wake_posted.store(true, Ordering::Release); - runnable.run_and_profile(); + + WindowsDispatcher::execute_runnable(runnable); } _ => break 'tasks, } @@ -939,7 +940,7 @@ pub(crate) struct WindowCreationInfo { pub(crate) windows_version: WindowsVersion, pub(crate) drop_target_helper: IDropTargetHelper, pub(crate) validation_number: usize, - pub(crate) main_receiver: PriorityQueueReceiver, + pub(crate) main_receiver: PriorityQueueReceiver, pub(crate) platform_window_handle: HWND, pub(crate) disable_direct_composition: bool, pub(crate) directx_devices: DirectXDevices, @@ -952,8 +953,8 @@ struct PlatformWindowCreateContext { inner: Option>>, raw_window_handles: std::sync::Weak>>, validation_number: usize, - main_sender: Option>, - main_receiver: Option>, + main_sender: Option>, + main_receiver: Option>, directx_devices: Option, dispatcher: Option>, } diff --git a/crates/gpui/src/platform/windows/window.rs b/crates/gpui/src/platform/windows/window.rs index 5d74b98e2a489febd0ea04c231ec3d989f416999..837474a9da3217f62a299a90a166e71065afa2ac 100644 --- a/crates/gpui/src/platform/windows/window.rs +++ b/crates/gpui/src/platform/windows/window.rs @@ -82,7 +82,7 @@ pub(crate) struct WindowsWindowInner { pub(crate) executor: ForegroundExecutor, pub(crate) windows_version: WindowsVersion, pub(crate) validation_number: usize, - pub(crate) main_receiver: PriorityQueueReceiver, + pub(crate) main_receiver: PriorityQueueReceiver, pub(crate) platform_window_handle: HWND, pub(crate) parent_hwnd: Option, } @@ -366,7 +366,7 @@ struct WindowCreateContext { windows_version: WindowsVersion, drop_target_helper: IDropTargetHelper, validation_number: usize, - main_receiver: PriorityQueueReceiver, + main_receiver: PriorityQueueReceiver, platform_window_handle: HWND, appearance: WindowAppearance, disable_direct_composition: bool, diff --git a/crates/repl/src/repl.rs b/crates/repl/src/repl.rs index 81ac8f72bc2a6a725fc41241b3e2174f9e1e85e4..346cca0211e43d6f254cb8300f8b0dae546b6004 100644 --- a/crates/repl/src/repl.rs +++ b/crates/repl/src/repl.rs @@ -12,7 +12,7 @@ mod session; use std::{sync::Arc, time::Duration}; use async_dispatcher::{Dispatcher, Runnable, set_dispatcher}; -use gpui::{App, GpuiRunnable, PlatformDispatcher}; +use gpui::{App, PlatformDispatcher, Priority, RunnableVariant}; use project::Fs; pub use runtimelib::ExecutionState; @@ -46,12 +46,12 @@ fn zed_dispatcher(cx: &mut App) -> impl Dispatcher { impl Dispatcher for ZedDispatcher { fn dispatch(&self, runnable: Runnable) { self.dispatcher - .dispatch(GpuiRunnable::DependencySpawned(runnable), None); + .dispatch(RunnableVariant::Compat(runnable), None, Priority::default()); } fn dispatch_after(&self, duration: Duration, runnable: Runnable) { self.dispatcher - .dispatch_after(duration, GpuiRunnable::DependencySpawned(runnable)); + .dispatch_after(duration, RunnableVariant::Compat(runnable)); } }