From 8b9a2df705bea719cb0ad0da0314d72812a4230b Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Wed, 16 Sep 2026 16:03:09 +0000 Subject: [PATCH] Share application adapter implementations (reduce LLVM IR bloat again) (#64262) Move settings-update routing, fallible deserialization recovery, selection effects, context-menu initialization, picker resize handles, and ACP client setup into shared implementations. Inline the small button callback adapters. `cargo llvm-lines --locked -p settings_ui`: Before (bec63dee23): 1571017 lines, 53634 copies After: 1177376 lines (-25.1%), 43380 copies (-19.1%) With compiler wrappers disabled, `cargo clean --target-dir target/binary-size-split-build` and `cargo build --locked -p zed`: Before: 135.6s wall, 1569.8s CPU, zed binary 1175563800 bytes After: 140.9s wall (+3.9%), 1588.8s CPU (+1.2%), zed binary 1157304600 bytes (-1.6%, -18.3MB) Release Notes: - N/A --- crates/agent_servers/src/acp.rs | 42 +++-- crates/editor/src/selection.rs | 64 +++++--- crates/picker/src/render/window_controls.rs | 99 ++++++------ .../settings_content/src/fallible_options.rs | 33 +++- crates/settings_ui/src/settings_ui.rs | 21 ++- crates/ui/src/components/button/button.rs | 2 + .../ui/src/components/button/button_like.rs | 2 + .../ui/src/components/button/icon_button.rs | 2 + crates/ui/src/components/context_menu.rs | 145 +++++++++--------- 9 files changed, 245 insertions(+), 165 deletions(-) diff --git a/crates/agent_servers/src/acp.rs b/crates/agent_servers/src/acp.rs index 81f099bf09..9da6f3e1ca 100644 --- a/crates/agent_servers/src/acp.rs +++ b/crates/agent_servers/src/acp.rs @@ -7,7 +7,9 @@ use agent_client_protocol::schema::{ ProtocolVersion, v1::{self as acp, ErrorCode}, }; -use agent_client_protocol::{Agent, Client, ConnectionTo, JsonRpcResponse, Lines, Responder}; +use agent_client_protocol::{ + Agent, Builder, Client, ConnectionTo, HandleDispatchFrom, JsonRpcResponse, Lines, Responder, +}; use anyhow::anyhow; use async_channel; use collections::{HashMap, HashSet}; @@ -667,23 +669,39 @@ fn connect_client_future( dispatch_tx: mpsc::UnboundedSender, connection_tx: futures::channel::oneshot::Sender>, ) -> impl Future> { + client_builder(name, dispatch_tx).connect_with( + transport, + move |connection: ConnectionTo| async move { + if connection_tx.send(connection).is_err() { + log::error!("failed to send ACP connection handle — receiver was dropped"); + } + // Keep the connection alive until the transport closes. + futures::future::pending::>().await + }, + ) +} + +fn client_builder( + name: &'static str, + dispatch_sender: mpsc::UnboundedSender, +) -> Builder> { // Each handler forwards its inputs onto the foreground dispatch queue. // The SDK requires the closure to be `Send`, so we move a clone of // `dispatch_tx` into each one. macro_rules! on_request { ($handler:ident) => {{ - let dispatch_tx = dispatch_tx.clone(); - async move |req, responder, _connection| { - enqueue_request(&dispatch_tx, req, responder, $handler); + let dispatch_sender = dispatch_sender.clone(); + async move |request, responder, _connection| { + enqueue_request(&dispatch_sender, request, responder, $handler); Ok(()) } }}; } macro_rules! on_notification { ($handler:ident) => {{ - let dispatch_tx = dispatch_tx.clone(); - async move |notif, _connection| { - enqueue_notification(&dispatch_tx, notif, $handler); + let dispatch_sender = dispatch_sender.clone(); + async move |notification, _connection| { + enqueue_notification(&dispatch_sender, notification, $handler); Ok(()) } }}; @@ -738,16 +756,6 @@ fn connect_client_future( on_notification!(handle_complete_elicitation), agent_client_protocol::on_receive_notification!(), ) - .connect_with( - transport, - move |connection: ConnectionTo| async move { - if connection_tx.send(connection).is_err() { - log::error!("failed to send ACP connection handle — receiver was dropped"); - } - // Keep the connection alive until the transport closes. - futures::future::pending::>().await - }, - ) } fn client_capabilities_for_agent( diff --git a/crates/editor/src/selection.rs b/crates/editor/src/selection.rs index f01ccf85de..d8d54ba48c 100644 --- a/crates/editor/src/selection.rs +++ b/crates/editor/src/selection.rs @@ -66,24 +66,10 @@ impl Editor { state.changed |= changed; return result; } - let mut state = DeferredSelectionEffectsState { - changed: false, - effects, - old_cursor_position: self.selections.newest_anchor().head(), - history_entry: SelectionHistoryEntry { - selections: self.selections.disjoint_anchors_arc(), - select_next_state: self.select_next_state.clone(), - select_prev_state: self.select_prev_state.clone(), - add_selections_state: self.add_selections_state.clone(), - }, - }; + let mut state = self.prepare_selection_effects(effects); let (changed, result) = self.selections.change_with(&snapshot, change); state.changed = state.changed || changed; - if self.defer_selection_effects { - self.deferred_selection_effects_state = Some(state); - } else { - self.apply_selection_effects(state, window, cx); - } + self.apply_selection_effects(state, window, cx); result } @@ -119,12 +105,7 @@ impl Editor { let already_deferred = self.defer_selection_effects; self.defer_selection_effects = true; let result = update(self, window, cx); - if !already_deferred { - self.defer_selection_effects = false; - if let Some(state) = self.deferred_selection_effects_state.take() { - self.apply_selection_effects(state, window, cx); - } - } + self.finish_deferred_selection_effects(already_deferred, window, cx); result } @@ -1776,12 +1757,51 @@ impl Editor { cx.notify(); } + #[inline(never)] + fn prepare_selection_effects( + &self, + effects: SelectionEffects, + ) -> DeferredSelectionEffectsState { + DeferredSelectionEffectsState { + changed: false, + effects, + old_cursor_position: self.selections.newest_anchor().head(), + history_entry: SelectionHistoryEntry { + selections: self.selections.disjoint_anchors_arc(), + select_next_state: self.select_next_state.clone(), + select_prev_state: self.select_prev_state.clone(), + add_selections_state: self.add_selections_state.clone(), + }, + } + } + + #[inline(never)] + fn finish_deferred_selection_effects( + &mut self, + already_deferred: bool, + window: &mut Window, + cx: &mut Context, + ) { + if !already_deferred { + self.defer_selection_effects = false; + if let Some(state) = self.deferred_selection_effects_state.take() { + self.apply_selection_effects(state, window, cx); + } + } + } + + #[inline(never)] fn apply_selection_effects( &mut self, state: DeferredSelectionEffectsState, window: &mut Window, cx: &mut Context, ) { + if self.defer_selection_effects { + self.deferred_selection_effects_state = Some(state); + return; + } + if state.changed { self.selection_history.push(state.history_entry); diff --git a/crates/picker/src/render/window_controls.rs b/crates/picker/src/render/window_controls.rs index cc7fbf2dc7..0fdb62a317 100644 --- a/crates/picker/src/render/window_controls.rs +++ b/crates/picker/src/render/window_controls.rs @@ -58,7 +58,9 @@ use std::{any::type_name, marker::PhantomData}; -use gpui::{ClickEvent, Context, CursorStyle, DragMoveEvent, MouseButton, Point, Styled, Window}; +use gpui::{ + ClickEvent, Context, CursorStyle, DragMoveEvent, MouseButton, Point, Stateful, Styled, Window, +}; use ui::prelude::*; use crate::shape::{Centered, PositionAndShape, Shape, SizeBounds}; @@ -430,50 +432,31 @@ impl Picker { window: &mut Window, cx: &mut Context, ) -> impl IntoElement { - div() - .id(S::id()) - .absolute() - .cursor(side.cursor()) - .map(|this| { - side.position( - this, - self.shape.clamped_position_and_size( - self.preview_layout_rendered(window), - &self.size_bounds, - window, - ), + render_resize_handle( + side, + self.shape, + &self.size_bounds, + self.preview_layout_rendered(window), + window, + ) + .on_drag_move::>(cx.listener( + move |picker, event: &DragMoveEvent>, window, cx| { + let drag = event.drag(cx); + let delta = event.event.position - drag.mouse_pos_before; + let mut working = side.current_position_and_shape(drag.shape_before, delta); + side.clamp( + &mut working, + &picker.size_bounds, + picker.preview_layout_rendered(window), window, - ) - }) - .block_mouse_except_scroll() - .on_mouse_down(MouseButton::Left, do_nothing) - .on_drag( - ResizeDrag::::start_new( - self.shape, - &self.size_bounds, - self.preview_layout_rendered(window), - window, - ), - |_, _, _, cx| cx.new(|_| DragPreview), - ) - .on_drag_move::>(cx.listener( - move |this, event: &DragMoveEvent>, window, cx| { - let drag = event.drag(cx); - let delta = event.event.position - drag.mouse_pos_before; - let mut working = side.current_position_and_shape(drag.shape_before, delta); - side.clamp( - &mut working, - &this.size_bounds, - this.preview_layout_rendered(window), - window, - ); - this.shape = Shape::Resizing(working); - cx.notify(); - }, - )) - .on_click(cx.listener(move |this, event: &ClickEvent, window, cx| { - this.reset_size_to_default_on_double_click(side, event, window, cx) - })) + ); + picker.shape = Shape::Resizing(working); + cx.notify(); + }, + )) + .on_click(cx.listener(move |picker, event: &ClickEvent, window, cx| { + picker.reset_size_to_default_on_double_click(side, event, window, cx) + })) } fn reset_size_to_default_on_double_click( @@ -497,6 +480,34 @@ impl Picker { } } +#[inline(never)] +#[track_caller] +fn render_resize_handle( + side: S, + shape: Shape, + bounds: &SizeBounds, + layout: Option, + window: &mut Window, +) -> Stateful
{ + div() + .id(S::id()) + .absolute() + .cursor(side.cursor()) + .map(|handle| { + side.position( + handle, + shape.clamped_position_and_size(layout, bounds, window), + window, + ) + }) + .block_mouse_except_scroll() + .on_mouse_down(MouseButton::Left, do_nothing) + .on_drag( + ResizeDrag::::start_new(shape, bounds, layout, window), + |_, _, _, cx| cx.new(|_| DragPreview), + ) +} + fn do_nothing(_: &gpui::MouseDownEvent, window: &mut Window, cx: &mut App) { window.prevent_default(); cx.stop_propagation(); diff --git a/crates/settings_content/src/fallible_options.rs b/crates/settings_content/src/fallible_options.rs index 91a8e183e0..731b83b464 100644 --- a/crates/settings_content/src/fallible_options.rs +++ b/crates/settings_content/src/fallible_options.rs @@ -1,4 +1,4 @@ -use std::cell::RefCell; +use std::{cell::RefCell, fmt::Display}; use serde::Deserialize; @@ -50,14 +50,22 @@ where { match T::deserialize(deserializer) { Ok(value) => Ok(value), - Err(e) => ERRORS.with_borrow_mut(|errors| { - if let Some(errors) = errors { - errors.push(anyhow::anyhow!("{}", e)); - Ok(Default::default()) - } else { - Err(e) + Err(error) => { + let mut error = Some(error); + let mut fallback = None; + recover_deserialization_error(&mut |record_error| { + if let Some(error) = error.take() { + record_error(&error); + let value = T::default(); + drop(error); + fallback = Some(value); + } + }); + match error { + Some(error) => Err(error), + None => Ok(fallback.expect("recovery creates a fallback")), } - }), + } } } @@ -146,6 +154,15 @@ where T::deserialize(rest) } +#[inline(never)] +fn recover_deserialization_error(recover: &mut dyn FnMut(&mut dyn FnMut(&dyn Display))) { + ERRORS.with_borrow_mut(|errors| { + if let Some(errors) = errors { + recover(&mut |error| errors.push(anyhow::anyhow!("{error}"))); + } + }); +} + #[cfg(test)] mod tests { use serde::Deserialize; diff --git a/crates/settings_ui/src/settings_ui.rs b/crates/settings_ui/src/settings_ui.rs index 12ede658ba..e139245a56 100644 --- a/crates/settings_ui/src/settings_ui.rs +++ b/crates/settings_ui/src/settings_ui.rs @@ -4694,6 +4694,20 @@ fn update_settings_file( window: &mut Window, cx: &mut App, update: impl 'static + Send + FnOnce(&mut SettingsContent, &App), +) -> Result<()> { + let mut update = Some(update); + update_settings_file_inner(file, file_name, window, cx, &mut || { + Box::new(update.take().expect("called once")) + }) +} + +#[inline(never)] +fn update_settings_file_inner( + file: SettingsUiFile, + file_name: Option<&'static str>, + window: &mut Window, + cx: &mut App, + update: &mut dyn FnMut() -> Box, ) -> Result<()> { telemetry::event!("Settings Change", setting = file_name, type = file.setting_type()); @@ -4708,7 +4722,7 @@ fn update_settings_file( } SettingsUiFile::User => { // todo(settings_ui) error? - SettingsStore::global(cx).update_settings_file(::global(cx), update); + SettingsStore::global(cx).update_settings_file(::global(cx), update()); Ok(()) } SettingsUiFile::Server(_) => unimplemented!(), @@ -4833,10 +4847,11 @@ impl ProjectSettingsUpdateQueue { } } +#[inline(never)] fn update_project_setting_file( worktree_id: WorktreeId, rel_path: Arc, - update: impl 'static + FnOnce(&mut SettingsContent, &App), + update: &mut dyn FnMut() -> Box, settings_window: Entity, cx: &mut App, ) -> Result<()> { @@ -4857,7 +4872,7 @@ fn update_project_setting_file( settings_window: settings_window.downgrade(), project: project.downgrade(), worktree: worktree.downgrade(), - update: Box::new(update), + update: update(), }; ProjectSettingsUpdateQueue::enqueue(cx, entry); diff --git a/crates/ui/src/components/button/button.rs b/crates/ui/src/components/button/button.rs index e8e55836c4..d461b8ed26 100644 --- a/crates/ui/src/components/button/button.rs +++ b/crates/ui/src/components/button/button.rs @@ -314,6 +314,7 @@ impl Disableable for Button { } impl Clickable for Button { + #[inline(always)] fn on_click( mut self, handler: impl Fn(&gpui::ClickEvent, &mut Window, &mut App) + 'static, @@ -393,6 +394,7 @@ impl ButtonCommon for Button { /// Button::new("tooltip_button", "Hover Me") /// .tooltip(Tooltip::text("This is a tooltip")); /// ``` + #[inline(always)] fn tooltip(mut self, tooltip: impl Fn(&mut Window, &mut App) -> AnyView + 'static) -> Self { self.base = self.base.tooltip(tooltip); self diff --git a/crates/ui/src/components/button/button_like.rs b/crates/ui/src/components/button/button_like.rs index bc7fb9d2fd..493b12dea3 100644 --- a/crates/ui/src/components/button/button_like.rs +++ b/crates/ui/src/components/button/button_like.rs @@ -670,6 +670,7 @@ impl SelectableButton for ButtonLike { } impl Clickable for ButtonLike { + #[inline(always)] fn on_click(mut self, handler: impl Fn(&ClickEvent, &mut Window, &mut App) + 'static) -> Self { self.on_click = Some(Box::new(handler)); self @@ -708,6 +709,7 @@ impl ButtonCommon for ButtonLike { self } + #[inline(always)] fn tooltip(mut self, tooltip: impl Fn(&mut Window, &mut App) -> AnyView + 'static) -> Self { self.tooltip = Some(Box::new(tooltip)); self diff --git a/crates/ui/src/components/button/icon_button.rs b/crates/ui/src/components/button/icon_button.rs index 2da97d4bc5..c5b039532b 100644 --- a/crates/ui/src/components/button/icon_button.rs +++ b/crates/ui/src/components/button/icon_button.rs @@ -160,6 +160,7 @@ impl SelectableButton for IconButton { } impl Clickable for IconButton { + #[inline(always)] fn on_click( mut self, handler: impl Fn(&gpui::ClickEvent, &mut Window, &mut App) + 'static, @@ -201,6 +202,7 @@ impl ButtonCommon for IconButton { self } + #[inline(always)] fn tooltip(mut self, tooltip: impl Fn(&mut Window, &mut App) -> AnyView + 'static) -> Self { self.base = self.base.tooltip(tooltip); self diff --git a/crates/ui/src/components/context_menu.rs b/crates/ui/src/components/context_menu.rs index 9662b5ed00..21d5ce3dd3 100644 --- a/crates/ui/src/components/context_menu.rs +++ b/crates/ui/src/components/context_menu.rs @@ -271,81 +271,14 @@ impl EventEmitter for ContextMenu {} impl FluentBuilder for ContextMenu {} impl ContextMenu { + #[inline(always)] pub fn new( window: &mut Window, cx: &mut Context, - f: impl FnOnce(Self, &mut Window, &mut Context) -> Self, + build_menu: impl FnOnce(Self, &mut Window, &mut Context) -> Self, ) -> Self { - let focus_handle = cx.focus_handle(); - let _on_blur_subscription = cx.on_blur( - &focus_handle, - window, - |this: &mut ContextMenu, window, cx| { - if let Some(ignore_until) = this.ignore_blur_until { - if Instant::now() < ignore_until { - return; - } else { - this.ignore_blur_until = None; - } - } - - if this.main_menu.is_none() { - if let SubmenuState::Open(open_submenu) = &this.submenu_state { - let submenu_focus = open_submenu.entity.read(cx).focus_handle.clone(); - if submenu_focus.contains_focused(window, cx) { - return; - } - } - } - - this.cancel(&menu::Cancel, window, cx) - }, - ); - window.refresh(); - - // When the menu first receives focus (i.e. when it opens), move the - // selection onto a menu item so assistive technology announces a real - // item rather than the bare menu container. Per the ARIA menu button - // pattern, opening a menu places focus on a menu item; for select-style - // menus we prefer the currently-checked item. We only do this when - // nothing is selected yet so we don't override an existing selection. - cx.on_focus_in(&focus_handle, window, |this, window, cx| { - if this.selected_index.is_none() && !this.suppress_focus_selection { - this.select_toggled_or_first(window, cx); - } - this.suppress_focus_selection = false; - }) - .detach(); - - f( - Self { - builder: None, - items: Default::default(), - focus_handle, - action_context: None, - selected_index: None, - delayed: false, - clicked: false, - end_slot_action: None, - key_context: "menu".into(), - _on_blur_subscription, - keep_open_on_confirm: false, - fixed_width: None, - main_menu: None, - main_menu_observed_bounds: Rc::new(Cell::new(None)), - documentation_aside: None, - aside_trigger_bounds: Rc::new(RefCell::new(HashMap::default())), - submenu_state: SubmenuState::Closed, - hover_target: HoverTarget::MainMenu, - submenu_safety_threshold_x: None, - submenu_trigger_bounds: Rc::new(Cell::new(None)), - submenu_trigger_mouse_down: false, - ignore_blur_until: None, - suppress_focus_selection: false, - }, - window, - cx, - ) + let menu = Self::new_inner(window, cx); + build_menu(menu, window, cx) } pub fn build( @@ -2175,6 +2108,76 @@ impl ContextMenu { ) .into_any_element() } + + #[inline(never)] + fn new_inner(window: &mut Window, cx: &mut Context) -> Self { + let focus_handle = cx.focus_handle(); + let _on_blur_subscription = cx.on_blur( + &focus_handle, + window, + |context_menu: &mut ContextMenu, window, cx| { + if let Some(ignore_until) = context_menu.ignore_blur_until { + if Instant::now() < ignore_until { + return; + } else { + context_menu.ignore_blur_until = None; + } + } + + if context_menu.main_menu.is_none() { + if let SubmenuState::Open(open_submenu) = &context_menu.submenu_state { + let submenu_focus = open_submenu.entity.read(cx).focus_handle.clone(); + if submenu_focus.contains_focused(window, cx) { + return; + } + } + } + + context_menu.cancel(&menu::Cancel, window, cx) + }, + ); + window.refresh(); + + // When the menu first receives focus (i.e. when it opens), move the + // selection onto a menu item so assistive technology announces a real + // item rather than the bare menu container. Per the ARIA menu button + // pattern, opening a menu places focus on a menu item; for select-style + // menus we prefer the currently-checked item. We only do this when + // nothing is selected yet so we don't override an existing selection. + cx.on_focus_in(&focus_handle, window, |context_menu, window, cx| { + if context_menu.selected_index.is_none() && !context_menu.suppress_focus_selection { + context_menu.select_toggled_or_first(window, cx); + } + context_menu.suppress_focus_selection = false; + }) + .detach(); + + Self { + builder: None, + items: Vec::new(), + focus_handle, + action_context: None, + selected_index: None, + delayed: false, + clicked: false, + end_slot_action: None, + key_context: SharedString::from("menu"), + _on_blur_subscription, + keep_open_on_confirm: false, + fixed_width: None, + main_menu: None, + main_menu_observed_bounds: Rc::new(Cell::new(None)), + documentation_aside: None, + aside_trigger_bounds: Rc::new(RefCell::new(HashMap::default())), + submenu_state: SubmenuState::Closed, + hover_target: HoverTarget::MainMenu, + submenu_safety_threshold_x: None, + submenu_trigger_bounds: Rc::new(Cell::new(None)), + submenu_trigger_mouse_down: false, + ignore_blur_until: None, + suppress_focus_selection: false, + } + } } impl ContextMenuItem {