Skip to content

Revert "Make tasks inherit their callers priority" #46413

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JosephTLyons merged 1 commit into from
Jan 9, 2026
Merged

Revert "Make tasks inherit their callers priority" #46413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions crates/git_ui/src/git_panel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ use workspace::{
dock::{DockPosition, Panel, PanelEvent},
notifications::{DetachAndPromptErr, ErrorMessagePrompt, NotificationId, NotifyResultExt},
};
use ztracing::instrument;
actions!(
git_panel,
[
Expand Down Expand Up @@ -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<Self>) {
maybe!({
let entry = self.entries.get(self.selected_entry?)?.status_entry()?;
Expand Down Expand Up @@ -1249,7 +1247,6 @@ impl GitPanel {
});
}

#[instrument(skip_all)]
fn open_file(
&mut self,
_: &menu::SecondaryConfirm,
Expand Down Expand Up @@ -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);
}
})
Expand Down
46 changes: 14 additions & 32 deletions crates/gpui/src/executor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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},
Expand Down Expand Up @@ -82,21 +81,7 @@ pub enum Priority {
Low,
}

thread_local! {
static CURRENT_TASKS_PRIORITY: Cell<Priority> = 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 {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand All @@ -410,7 +399,6 @@ impl BackgroundExecutor {
async_task::Builder::new()
.metadata(RunnableMeta {
location,
priority,
app: None,
})
.spawn(
Expand All @@ -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)
},
)
};

Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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<R>(&self, future: impl Future<Output = R> + 'static) -> Task<R>
where
Expand Down Expand Up @@ -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),
},
);
Expand Down
75 changes: 7 additions & 68 deletions crates/gpui/src/platform.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -590,8 +590,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<std::sync::Weak<()>>,
Expand All @@ -600,7 +599,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()
Expand All @@ -617,69 +615,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<RunnableMeta>),
/// 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<RunnableMeta>),
Compat(Runnable),
}

/// This type is public so that our test macro can generate and use it, but it should not
Expand All @@ -689,9 +628,9 @@ pub trait PlatformDispatcher: Send + Sync {
fn get_all_timings(&self) -> Vec<ThreadTaskTimings>;
fn get_current_thread_timings(&self) -> Vec<TaskTiming>;
fn is_main_thread(&self) -> bool;
fn dispatch(&self, runnable: GpuiRunnable, label: Option<TaskLabel>);
fn dispatch_on_main_thread(&self, runnable: GpuiRunnable);
fn dispatch_after(&self, duration: Duration, runnable: GpuiRunnable);
fn dispatch(&self, runnable: RunnableVariant, label: Option<TaskLabel>, 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<dyn FnOnce() + Send>);

fn now(&self) -> Instant {
Expand Down
Loading
Loading