From e1ca8e81bb83c4a45b0f9b2fc7b5adf089697f9f Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Fri, 15 Dec 2023 15:13:32 +0100 Subject: [PATCH 1/4] Use an Arena to reuse allocations for listeners --- crates/gpui2/src/arena.rs | 124 +++++++++++++++++++++++++++++++ crates/gpui2/src/gpui2.rs | 1 + crates/gpui2/src/key_dispatch.rs | 10 +-- crates/gpui2/src/window.rs | 105 +++++++++++++++----------- 4 files changed, 191 insertions(+), 49 deletions(-) create mode 100644 crates/gpui2/src/arena.rs diff --git a/crates/gpui2/src/arena.rs b/crates/gpui2/src/arena.rs new file mode 100644 index 0000000000..cf6b877ba3 --- /dev/null +++ b/crates/gpui2/src/arena.rs @@ -0,0 +1,124 @@ +use std::{ + alloc, + ptr::{self, NonNull}, +}; + +pub struct Arena { + start: NonNull, + offset: usize, + elements: Vec, +} + +impl Default for Arena { + fn default() -> Self { + unsafe { + let layout = alloc::Layout::from_size_align(16 * 1024 * 1024, 1).unwrap(); + let ptr = alloc::alloc(layout); + Self { + start: NonNull::new_unchecked(ptr), + offset: 0, + elements: Vec::new(), + } + } + } +} + +struct ArenaElement { + value: NonNull, + drop: unsafe fn(NonNull), +} + +impl Arena { + pub fn clear(&mut self) { + for element in self.elements.drain(..) { + unsafe { + (element.drop)(element.value); + } + } + self.offset = 0; + } + + #[inline(always)] + pub fn alloc(&mut self, value: T) -> ArenaRef { + unsafe fn drop(ptr: NonNull) { + std::ptr::drop_in_place(ptr.cast::().as_ptr()); + } + + unsafe { + let layout = alloc::Layout::for_value(&value).pad_to_align(); + let value_ptr = self.start.as_ptr().add(self.offset).cast::(); + ptr::write(value_ptr, value); + + let value = NonNull::new_unchecked(value_ptr); + self.elements.push(ArenaElement { + value: value.cast(), + drop: drop::, + }); + self.offset += layout.size(); + ArenaRef(value) + } + } +} + +pub struct ArenaRef(NonNull); + +impl Copy for ArenaRef {} + +impl Clone for ArenaRef { + fn clone(&self) -> Self { + Self(self.0) + } +} + +impl ArenaRef { + pub unsafe fn map(mut self, f: impl FnOnce(&mut T) -> &mut U) -> ArenaRef { + let u = f(self.get_mut()); + ArenaRef(NonNull::new_unchecked(u)) + } + + pub unsafe fn get_mut(&mut self) -> &mut T { + self.0.as_mut() + } +} + +#[cfg(test)] +mod tests { + use std::{cell::Cell, rc::Rc}; + + use super::*; + + #[test] + fn test_arena() { + let mut arena = Arena::default(); + let mut a = arena.alloc(1u64); + let mut b = arena.alloc(2u32); + let mut c = arena.alloc(3u16); + let mut d = arena.alloc(4u8); + assert_eq!(unsafe { *a.get_mut() }, 1); + assert_eq!(unsafe { *b.get_mut() }, 2); + assert_eq!(unsafe { *c.get_mut() }, 3); + assert_eq!(unsafe { *d.get_mut() }, 4); + + arena.clear(); + let mut a = arena.alloc(5u64); + let mut b = arena.alloc(6u32); + let mut c = arena.alloc(7u16); + let mut d = arena.alloc(8u8); + assert_eq!(unsafe { *a.get_mut() }, 5); + assert_eq!(unsafe { *b.get_mut() }, 6); + assert_eq!(unsafe { *c.get_mut() }, 7); + assert_eq!(unsafe { *d.get_mut() }, 8); + + // Ensure drop gets called. + let dropped = Rc::new(Cell::new(false)); + struct DropGuard(Rc>); + impl Drop for DropGuard { + fn drop(&mut self) { + self.0.set(true); + } + } + arena.alloc(DropGuard(dropped.clone())); + arena.clear(); + assert!(dropped.get()); + } +} diff --git a/crates/gpui2/src/gpui2.rs b/crates/gpui2/src/gpui2.rs index c711d1f5a2..322ca20608 100644 --- a/crates/gpui2/src/gpui2.rs +++ b/crates/gpui2/src/gpui2.rs @@ -2,6 +2,7 @@ mod action; mod app; +mod arena; mod assets; mod color; mod element; diff --git a/crates/gpui2/src/key_dispatch.rs b/crates/gpui2/src/key_dispatch.rs index ddb1f1e6ca..a9d717ea1a 100644 --- a/crates/gpui2/src/key_dispatch.rs +++ b/crates/gpui2/src/key_dispatch.rs @@ -1,6 +1,6 @@ use crate::{ - Action, ActionRegistry, DispatchPhase, FocusId, KeyBinding, KeyContext, KeyMatch, Keymap, - Keystroke, KeystrokeMatcher, WindowContext, + arena::ArenaRef, Action, ActionRegistry, DispatchPhase, FocusId, KeyBinding, KeyContext, + KeyMatch, Keymap, Keystroke, KeystrokeMatcher, WindowContext, }; use collections::HashMap; use parking_lot::Mutex; @@ -33,12 +33,12 @@ pub(crate) struct DispatchNode { parent: Option, } -type KeyListener = Rc; +type KeyListener = ArenaRef; #[derive(Clone)] pub(crate) struct DispatchActionListener { pub(crate) action_type: TypeId, - pub(crate) listener: Rc, + pub(crate) listener: ArenaRef, } impl DispatchTree { @@ -117,7 +117,7 @@ impl DispatchTree { pub fn on_action( &mut self, action_type: TypeId, - listener: Rc, + listener: ArenaRef, ) { self.active_node() .action_listeners diff --git a/crates/gpui2/src/window.rs b/crates/gpui2/src/window.rs index 1c0849785b..202489ef0e 100644 --- a/crates/gpui2/src/window.rs +++ b/crates/gpui2/src/window.rs @@ -1,15 +1,17 @@ use crate::{ - key_dispatch::DispatchActionListener, px, size, transparent_black, Action, AnyDrag, AnyView, - AppContext, AsyncWindowContext, AvailableSpace, Bounds, BoxShadow, Context, Corners, - CursorStyle, DevicePixels, DispatchNodeId, DispatchTree, DisplayId, Edges, Effect, Entity, - EntityId, EventEmitter, FileDropEvent, Flatten, FontId, GlobalElementId, GlyphId, Hsla, - ImageData, InputEvent, IsZero, KeyBinding, KeyContext, KeyDownEvent, KeystrokeEvent, LayoutId, - Model, ModelContext, Modifiers, MonochromeSprite, MouseButton, MouseMoveEvent, MouseUpEvent, - Path, Pixels, PlatformAtlas, PlatformDisplay, PlatformInputHandler, PlatformWindow, Point, - PolychromeSprite, PromptLevel, Quad, Render, RenderGlyphParams, RenderImageParams, - RenderSvgParams, ScaledPixels, Scene, SceneBuilder, Shadow, SharedString, Size, Style, - SubscriberSet, Subscription, Surface, TaffyLayoutEngine, Task, Underline, UnderlineStyle, View, - VisualContext, WeakView, WindowBounds, WindowOptions, SUBPIXEL_VARIANTS, + arena::{Arena, ArenaRef}, + key_dispatch::DispatchActionListener, + px, size, transparent_black, Action, AnyDrag, AnyView, AppContext, AsyncWindowContext, + AvailableSpace, Bounds, BoxShadow, Context, Corners, CursorStyle, DevicePixels, DispatchNodeId, + DispatchTree, DisplayId, Edges, Effect, Entity, EntityId, EventEmitter, FileDropEvent, Flatten, + FontId, GlobalElementId, GlyphId, Hsla, ImageData, InputEvent, IsZero, KeyBinding, KeyContext, + KeyDownEvent, KeystrokeEvent, LayoutId, Model, ModelContext, Modifiers, MonochromeSprite, + MouseButton, MouseMoveEvent, MouseUpEvent, Path, Pixels, PlatformAtlas, PlatformDisplay, + PlatformInputHandler, PlatformWindow, Point, PolychromeSprite, PromptLevel, Quad, Render, + RenderGlyphParams, RenderImageParams, RenderSvgParams, ScaledPixels, Scene, SceneBuilder, + Shadow, SharedString, Size, Style, SubscriberSet, Subscription, Surface, TaffyLayoutEngine, + Task, Underline, UnderlineStyle, View, VisualContext, WeakView, WindowBounds, WindowOptions, + SUBPIXEL_VARIANTS, }; use anyhow::{anyhow, Context as _, Result}; use collections::FxHashMap; @@ -85,7 +87,7 @@ impl DispatchPhase { } type AnyObserver = Box bool + 'static>; -type AnyMouseListener = Box; +type AnyMouseListener = ArenaRef; type AnyWindowFocusListener = Box bool + 'static>; struct FocusEvent { @@ -268,9 +270,9 @@ pub(crate) struct ElementStateBox { type_name: &'static str, } -// #[derive(Default)] pub(crate) struct Frame { focus: Option, + arena: Arena, pub(crate) element_states: FxHashMap, mouse_listeners: FxHashMap>, pub(crate) dispatch_tree: DispatchTree, @@ -285,6 +287,7 @@ impl Frame { fn new(dispatch_tree: DispatchTree) -> Self { Frame { focus: None, + arena: Arena::default(), element_states: FxHashMap::default(), mouse_listeners: FxHashMap::default(), dispatch_tree, @@ -299,6 +302,7 @@ impl Frame { fn clear(&mut self) { self.element_states.clear(); self.mouse_listeners.values_mut().for_each(Vec::clear); + self.arena.clear(); self.dispatch_tree.clear(); self.depth_map.clear(); } @@ -818,25 +822,23 @@ impl<'a> WindowContext<'a> { /// Register a mouse event listener on the window for the next frame. The type of event /// is determined by the first parameter of the given listener. When the next frame is rendered /// the listener will be cleared. - /// - /// This is a fairly low-level method, so prefer using event handlers on elements unless you have - /// a specific need to register a global listener. pub fn on_mouse_event( &mut self, mut handler: impl FnMut(&Event, DispatchPhase, &mut WindowContext) + 'static, ) { let order = self.window.next_frame.z_index_stack.clone(); + let handler = self.window.next_frame.arena.alloc( + move |event: &dyn Any, phase: DispatchPhase, cx: &mut WindowContext<'_>| { + handler(event.downcast_ref().unwrap(), phase, cx) + }, + ); + let handler = unsafe { handler.map(|handler| handler as _) }; self.window .next_frame .mouse_listeners .entry(TypeId::of::()) .or_default() - .push(( - order, - Box::new(move |event: &dyn Any, phase, cx| { - handler(event.downcast_ref().unwrap(), phase, cx) - }), - )) + .push((order, handler)) } /// Register a key event listener on the window for the next frame. The type of event @@ -847,16 +849,17 @@ impl<'a> WindowContext<'a> { /// a specific need to register a global listener. pub fn on_key_event( &mut self, - handler: impl Fn(&Event, DispatchPhase, &mut WindowContext) + 'static, + listener: impl Fn(&Event, DispatchPhase, &mut WindowContext) + 'static, ) { - self.window - .next_frame - .dispatch_tree - .on_key_event(Rc::new(move |event, phase, cx| { + let listener = self.window.next_frame.arena.alloc( + move |event: &dyn Any, phase, cx: &mut WindowContext<'_>| { if let Some(event) = event.downcast_ref::() { - handler(event, phase, cx) + listener(event, phase, cx) } - })); + }, + ); + let listener = unsafe { listener.map(|handler| handler as _) }; + self.window.next_frame.dispatch_tree.on_key_event(listener); } /// Register an action listener on the window for the next frame. The type of action @@ -868,12 +871,14 @@ impl<'a> WindowContext<'a> { pub fn on_action( &mut self, action_type: TypeId, - handler: impl Fn(&dyn Any, DispatchPhase, &mut WindowContext) + 'static, + listener: impl Fn(&dyn Any, DispatchPhase, &mut WindowContext) + 'static, ) { - self.window.next_frame.dispatch_tree.on_action( - action_type, - Rc::new(move |action, phase, cx| handler(action, phase, cx)), - ); + let listener = self.window.next_frame.arena.alloc(listener); + let listener = unsafe { listener.map(|handler| handler as _) }; + self.window + .next_frame + .dispatch_tree + .on_action(action_type, listener); } pub fn is_action_available(&self, action: &dyn Action) -> bool { @@ -1274,10 +1279,16 @@ impl<'a> WindowContext<'a> { cx.with_key_dispatch(Some(KeyContext::default()), None, |_, cx| { for (action_type, action_listeners) in &cx.app.global_action_listeners { for action_listener in action_listeners.iter().cloned() { - cx.window.next_frame.dispatch_tree.on_action( - *action_type, - Rc::new(move |action, phase, cx| action_listener(action, phase, cx)), - ) + let listener = cx.window.next_frame.arena.alloc( + move |action: &dyn Any, phase, cx: &mut WindowContext<'_>| { + action_listener(action, phase, cx) + }, + ); + let listener = unsafe { listener.map(|listener| listener as _) }; + cx.window + .next_frame + .dispatch_tree + .on_action(*action_type, listener) } } @@ -1460,6 +1471,7 @@ impl<'a> WindowContext<'a> { // Capture phase, events bubble from back to front. Handlers for this phase are used for // special purposes, such as detecting events outside of a given Bounds. for (_, handler) in &mut handlers { + let handler = unsafe { handler.get_mut() }; handler(event, DispatchPhase::Capture, self); if !self.app.propagate_event { break; @@ -1469,6 +1481,7 @@ impl<'a> WindowContext<'a> { // Bubble phase, where most normal handlers do their work. if self.app.propagate_event { for (_, handler) in handlers.iter_mut().rev() { + let handler = unsafe { handler.get_mut() }; handler(event, DispatchPhase::Bubble, self); if !self.app.propagate_event { break; @@ -1518,7 +1531,8 @@ impl<'a> WindowContext<'a> { context_stack.push(context); } - for key_listener in node.key_listeners.clone() { + for mut key_listener in node.key_listeners.clone() { + let key_listener = unsafe { key_listener.get_mut() }; key_listener(event, DispatchPhase::Capture, self); if !self.propagate_event { return; @@ -1530,7 +1544,8 @@ impl<'a> WindowContext<'a> { for node_id in dispatch_path.iter().rev() { // Handle low level key events let node = self.window.rendered_frame.dispatch_tree.node(*node_id); - for key_listener in node.key_listeners.clone() { + for mut key_listener in node.key_listeners.clone() { + let key_listener = unsafe { key_listener.get_mut() }; key_listener(event, DispatchPhase::Bubble, self); if !self.propagate_event { return; @@ -1582,11 +1597,12 @@ impl<'a> WindowContext<'a> { let node = self.window.rendered_frame.dispatch_tree.node(*node_id); for DispatchActionListener { action_type, - listener, + mut listener, } in node.action_listeners.clone() { let any_action = action.as_any(); if action_type == any_action.type_id() { + let listener = unsafe { listener.get_mut() }; listener(any_action, DispatchPhase::Capture, self); if !self.propagate_event { return; @@ -1599,12 +1615,13 @@ impl<'a> WindowContext<'a> { let node = self.window.rendered_frame.dispatch_tree.node(*node_id); for DispatchActionListener { action_type, - listener, + mut listener, } in node.action_listeners.clone() { let any_action = action.as_any(); if action_type == any_action.type_id() { self.propagate_event = false; // Actions stop propagation by default during the bubble phase + let listener = unsafe { listener.get_mut() }; listener(any_action, DispatchPhase::Bubble, self); if !self.propagate_event { return; @@ -2593,13 +2610,13 @@ impl<'a, V: 'static> ViewContext<'a, V> { pub fn on_action( &mut self, action_type: TypeId, - handler: impl Fn(&mut V, &dyn Any, DispatchPhase, &mut ViewContext) + 'static, + listener: impl Fn(&mut V, &dyn Any, DispatchPhase, &mut ViewContext) + 'static, ) { let handle = self.view().clone(); self.window_cx .on_action(action_type, move |action, phase, cx| { handle.update(cx, |view, cx| { - handler(view, action, phase, cx); + listener(view, action, phase, cx); }) }); } From be73dd852d2bf1b709bffd971fa2e92d871727fc Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Fri, 15 Dec 2023 16:18:05 +0100 Subject: [PATCH 2/4] Move `Arena` to a thread-local and use it to allocate `AnyElement` --- crates/gpui2/src/arena.rs | 4 ++++ crates/gpui2/src/element.rs | 30 ++++++++++++++------------ crates/gpui2/src/window.rs | 43 +++++++++++++++++++++---------------- 3 files changed, 45 insertions(+), 32 deletions(-) diff --git a/crates/gpui2/src/arena.rs b/crates/gpui2/src/arena.rs index cf6b877ba3..0e01c392b7 100644 --- a/crates/gpui2/src/arena.rs +++ b/crates/gpui2/src/arena.rs @@ -76,6 +76,10 @@ impl ArenaRef { ArenaRef(NonNull::new_unchecked(u)) } + pub unsafe fn get(&self) -> &T { + self.0.as_ref() + } + pub unsafe fn get_mut(&mut self) -> &mut T { self.0.as_mut() } diff --git a/crates/gpui2/src/element.rs b/crates/gpui2/src/element.rs index b446c2fe86..dcb103b61e 100644 --- a/crates/gpui2/src/element.rs +++ b/crates/gpui2/src/element.rs @@ -1,6 +1,6 @@ use crate::{ - AvailableSpace, BorrowWindow, Bounds, ElementId, LayoutId, Pixels, Point, Size, ViewContext, - WindowContext, + arena::ArenaRef, AvailableSpace, BorrowWindow, Bounds, ElementId, LayoutId, Pixels, Point, + Size, ViewContext, WindowContext, FRAME_ARENA, }; use derive_more::{Deref, DerefMut}; pub(crate) use smallvec::SmallVec; @@ -405,7 +405,7 @@ where } } -pub struct AnyElement(Box); +pub struct AnyElement(ArenaRef); impl AnyElement { pub fn new(element: E) -> Self @@ -413,15 +413,18 @@ impl AnyElement { E: 'static + Element, E::State: Any, { - AnyElement(Box::new(Some(DrawableElement::new(element))) as Box) + let element = + FRAME_ARENA.with_borrow_mut(|arena| arena.alloc(Some(DrawableElement::new(element)))); + let element = unsafe { element.map(|element| element as &mut dyn ElementObject) }; + AnyElement(element) } pub fn layout(&mut self, cx: &mut WindowContext) -> LayoutId { - self.0.layout(cx) + unsafe { self.0.get_mut() }.layout(cx) } pub fn paint(&mut self, cx: &mut WindowContext) { - self.0.paint(cx) + unsafe { self.0.get_mut() }.paint(cx) } /// Initializes this element and performs layout within the given available space to determine its size. @@ -430,7 +433,7 @@ impl AnyElement { available_space: Size, cx: &mut WindowContext, ) -> Size { - self.0.measure(available_space, cx) + unsafe { self.0.get_mut() }.measure(available_space, cx) } /// Initializes this element and performs layout in the available space, then paints it at the given origin. @@ -440,16 +443,11 @@ impl AnyElement { available_space: Size, cx: &mut WindowContext, ) { - self.0.draw(origin, available_space, cx) - } - - /// Converts this `AnyElement` into a trait object that can be stored and manipulated. - pub fn into_any(self) -> AnyElement { - AnyElement::new(self) + unsafe { self.0.get_mut() }.draw(origin, available_space, cx) } pub fn inner_id(&self) -> Option { - self.0.element_id() + unsafe { self.0.get() }.element_id() } } @@ -480,6 +478,10 @@ impl IntoElement for AnyElement { fn into_element(self) -> Self::Element { self } + + fn into_any_element(self) -> AnyElement { + self + } } /// The empty element, which renders nothing. diff --git a/crates/gpui2/src/window.rs b/crates/gpui2/src/window.rs index 202489ef0e..74b67503d5 100644 --- a/crates/gpui2/src/window.rs +++ b/crates/gpui2/src/window.rs @@ -27,6 +27,7 @@ use smallvec::SmallVec; use std::{ any::{Any, TypeId}, borrow::{Borrow, BorrowMut, Cow}, + cell::RefCell, fmt::Debug, future::Future, hash::{Hash, Hasher}, @@ -97,6 +98,10 @@ struct FocusEvent { slotmap::new_key_type! { pub struct FocusId; } +thread_local! { + pub static FRAME_ARENA: RefCell = RefCell::new(Arena::default()); +} + impl FocusId { /// Obtains whether the element associated with this handle is currently focused. pub fn is_focused(&self, cx: &WindowContext) -> bool { @@ -272,7 +277,6 @@ pub(crate) struct ElementStateBox { pub(crate) struct Frame { focus: Option, - arena: Arena, pub(crate) element_states: FxHashMap, mouse_listeners: FxHashMap>, pub(crate) dispatch_tree: DispatchTree, @@ -287,7 +291,6 @@ impl Frame { fn new(dispatch_tree: DispatchTree) -> Self { Frame { focus: None, - arena: Arena::default(), element_states: FxHashMap::default(), mouse_listeners: FxHashMap::default(), dispatch_tree, @@ -302,7 +305,6 @@ impl Frame { fn clear(&mut self) { self.element_states.clear(); self.mouse_listeners.values_mut().for_each(Vec::clear); - self.arena.clear(); self.dispatch_tree.clear(); self.depth_map.clear(); } @@ -827,11 +829,13 @@ impl<'a> WindowContext<'a> { mut handler: impl FnMut(&Event, DispatchPhase, &mut WindowContext) + 'static, ) { let order = self.window.next_frame.z_index_stack.clone(); - let handler = self.window.next_frame.arena.alloc( - move |event: &dyn Any, phase: DispatchPhase, cx: &mut WindowContext<'_>| { - handler(event.downcast_ref().unwrap(), phase, cx) - }, - ); + let handler = FRAME_ARENA.with_borrow_mut(|arena| { + arena.alloc( + move |event: &dyn Any, phase: DispatchPhase, cx: &mut WindowContext<'_>| { + handler(event.downcast_ref().unwrap(), phase, cx) + }, + ) + }); let handler = unsafe { handler.map(|handler| handler as _) }; self.window .next_frame @@ -851,13 +855,13 @@ impl<'a> WindowContext<'a> { &mut self, listener: impl Fn(&Event, DispatchPhase, &mut WindowContext) + 'static, ) { - let listener = self.window.next_frame.arena.alloc( - move |event: &dyn Any, phase, cx: &mut WindowContext<'_>| { + let listener = FRAME_ARENA.with_borrow_mut(|arena| { + arena.alloc(move |event: &dyn Any, phase, cx: &mut WindowContext<'_>| { if let Some(event) = event.downcast_ref::() { listener(event, phase, cx) } - }, - ); + }) + }); let listener = unsafe { listener.map(|handler| handler as _) }; self.window.next_frame.dispatch_tree.on_key_event(listener); } @@ -873,7 +877,7 @@ impl<'a> WindowContext<'a> { action_type: TypeId, listener: impl Fn(&dyn Any, DispatchPhase, &mut WindowContext) + 'static, ) { - let listener = self.window.next_frame.arena.alloc(listener); + let listener = FRAME_ARENA.with_borrow_mut(|arena| arena.alloc(listener)); let listener = unsafe { listener.map(|handler| handler as _) }; self.window .next_frame @@ -1273,17 +1277,20 @@ impl<'a> WindowContext<'a> { self.window.platform_window.clear_input_handler(); self.window.layout_engine.as_mut().unwrap().clear(); self.window.next_frame.clear(); + FRAME_ARENA.with_borrow_mut(|arena| arena.clear()); let root_view = self.window.root_view.take().unwrap(); self.with_z_index(0, |cx| { cx.with_key_dispatch(Some(KeyContext::default()), None, |_, cx| { for (action_type, action_listeners) in &cx.app.global_action_listeners { for action_listener in action_listeners.iter().cloned() { - let listener = cx.window.next_frame.arena.alloc( - move |action: &dyn Any, phase, cx: &mut WindowContext<'_>| { - action_listener(action, phase, cx) - }, - ); + let listener = FRAME_ARENA.with_borrow_mut(|arena| { + arena.alloc( + move |action: &dyn Any, phase, cx: &mut WindowContext<'_>| { + action_listener(action, phase, cx) + }, + ) + }); let listener = unsafe { listener.map(|listener| listener as _) }; cx.window .next_frame From 0a57171066400c64aa282d9350a80220e5cf485e Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Fri, 15 Dec 2023 18:30:32 +0100 Subject: [PATCH 3/4] Use a safe API for Arena --- crates/gpui2/src/arena.rs | 150 ++++++++++++++++++++++-------------- crates/gpui2/src/element.rs | 12 +-- crates/gpui2/src/window.rs | 70 ++++++++--------- 3 files changed, 134 insertions(+), 98 deletions(-) diff --git a/crates/gpui2/src/arena.rs b/crates/gpui2/src/arena.rs index 0e01c392b7..a362c720a2 100644 --- a/crates/gpui2/src/arena.rs +++ b/crates/gpui2/src/arena.rs @@ -1,40 +1,49 @@ use std::{ alloc, + cell::Cell, + ops::{Deref, DerefMut}, ptr::{self, NonNull}, + rc::Rc, }; -pub struct Arena { - start: NonNull, - offset: usize, - elements: Vec, -} - -impl Default for Arena { - fn default() -> Self { - unsafe { - let layout = alloc::Layout::from_size_align(16 * 1024 * 1024, 1).unwrap(); - let ptr = alloc::alloc(layout); - Self { - start: NonNull::new_unchecked(ptr), - offset: 0, - elements: Vec::new(), - } - } - } -} - struct ArenaElement { value: NonNull, drop: unsafe fn(NonNull), } +impl Drop for ArenaElement { + fn drop(&mut self) { + unsafe { + (self.drop)(self.value); + } + } +} + +pub struct Arena { + start: NonNull, + offset: usize, + elements: Vec, + valid: Rc>, +} + impl Arena { - pub fn clear(&mut self) { - for element in self.elements.drain(..) { - unsafe { - (element.drop)(element.value); + pub fn new(size_in_bytes: usize) -> Self { + unsafe { + let layout = alloc::Layout::from_size_align(size_in_bytes, 1).unwrap(); + let ptr = alloc::alloc(layout); + Self { + start: NonNull::new_unchecked(ptr), + offset: 0, + elements: Vec::new(), + valid: Rc::new(Cell::new(true)), } } + } + + pub fn clear(&mut self) { + self.valid.set(false); + self.valid = Rc::new(Cell::new(true)); + self.elements.clear(); self.offset = 0; } @@ -46,42 +55,71 @@ impl Arena { unsafe { let layout = alloc::Layout::for_value(&value).pad_to_align(); - let value_ptr = self.start.as_ptr().add(self.offset).cast::(); - ptr::write(value_ptr, value); + let ptr = NonNull::new_unchecked(self.start.as_ptr().add(self.offset).cast::()); + ptr::write(ptr.as_ptr(), value); - let value = NonNull::new_unchecked(value_ptr); self.elements.push(ArenaElement { - value: value.cast(), + value: ptr.cast(), drop: drop::, }); self.offset += layout.size(); - ArenaRef(value) + ArenaRef { + ptr, + valid: self.valid.clone(), + } } } } -pub struct ArenaRef(NonNull); +impl Drop for Arena { + fn drop(&mut self) { + self.clear(); + } +} -impl Copy for ArenaRef {} +pub struct ArenaRef { + ptr: NonNull, + valid: Rc>, +} impl Clone for ArenaRef { fn clone(&self) -> Self { - Self(self.0) + Self { + ptr: self.ptr, + valid: self.valid.clone(), + } } } impl ArenaRef { - pub unsafe fn map(mut self, f: impl FnOnce(&mut T) -> &mut U) -> ArenaRef { - let u = f(self.get_mut()); - ArenaRef(NonNull::new_unchecked(u)) + pub fn map(mut self, f: impl FnOnce(&mut T) -> &mut U) -> ArenaRef { + ArenaRef { + ptr: unsafe { NonNull::new_unchecked(f(&mut *self)) }, + valid: self.valid, + } } - pub unsafe fn get(&self) -> &T { - self.0.as_ref() + fn validate(&self) { + assert!( + self.valid.get(), + "attempted to dereference an ArenaRef after its Arena was cleared" + ); } +} - pub unsafe fn get_mut(&mut self) -> &mut T { - self.0.as_mut() +impl Deref for ArenaRef { + type Target = T; + + fn deref(&self) -> &Self::Target { + self.validate(); + unsafe { self.ptr.as_ref() } + } +} + +impl DerefMut for ArenaRef { + fn deref_mut(&mut self) -> &mut Self::Target { + self.validate(); + unsafe { self.ptr.as_mut() } } } @@ -93,25 +131,25 @@ mod tests { #[test] fn test_arena() { - let mut arena = Arena::default(); - let mut a = arena.alloc(1u64); - let mut b = arena.alloc(2u32); - let mut c = arena.alloc(3u16); - let mut d = arena.alloc(4u8); - assert_eq!(unsafe { *a.get_mut() }, 1); - assert_eq!(unsafe { *b.get_mut() }, 2); - assert_eq!(unsafe { *c.get_mut() }, 3); - assert_eq!(unsafe { *d.get_mut() }, 4); + let mut arena = Arena::new(1024); + let a = arena.alloc(1u64); + let b = arena.alloc(2u32); + let c = arena.alloc(3u16); + let d = arena.alloc(4u8); + assert_eq!(*a, 1); + assert_eq!(*b, 2); + assert_eq!(*c, 3); + assert_eq!(*d, 4); arena.clear(); - let mut a = arena.alloc(5u64); - let mut b = arena.alloc(6u32); - let mut c = arena.alloc(7u16); - let mut d = arena.alloc(8u8); - assert_eq!(unsafe { *a.get_mut() }, 5); - assert_eq!(unsafe { *b.get_mut() }, 6); - assert_eq!(unsafe { *c.get_mut() }, 7); - assert_eq!(unsafe { *d.get_mut() }, 8); + let a = arena.alloc(5u64); + let b = arena.alloc(6u32); + let c = arena.alloc(7u16); + let d = arena.alloc(8u8); + assert_eq!(*a, 5); + assert_eq!(*b, 6); + assert_eq!(*c, 7); + assert_eq!(*d, 8); // Ensure drop gets called. let dropped = Rc::new(Cell::new(false)); diff --git a/crates/gpui2/src/element.rs b/crates/gpui2/src/element.rs index dcb103b61e..d54d1c245d 100644 --- a/crates/gpui2/src/element.rs +++ b/crates/gpui2/src/element.rs @@ -415,16 +415,16 @@ impl AnyElement { { let element = FRAME_ARENA.with_borrow_mut(|arena| arena.alloc(Some(DrawableElement::new(element)))); - let element = unsafe { element.map(|element| element as &mut dyn ElementObject) }; + let element = element.map(|element| element as &mut dyn ElementObject); AnyElement(element) } pub fn layout(&mut self, cx: &mut WindowContext) -> LayoutId { - unsafe { self.0.get_mut() }.layout(cx) + self.0.layout(cx) } pub fn paint(&mut self, cx: &mut WindowContext) { - unsafe { self.0.get_mut() }.paint(cx) + self.0.paint(cx) } /// Initializes this element and performs layout within the given available space to determine its size. @@ -433,7 +433,7 @@ impl AnyElement { available_space: Size, cx: &mut WindowContext, ) -> Size { - unsafe { self.0.get_mut() }.measure(available_space, cx) + self.0.measure(available_space, cx) } /// Initializes this element and performs layout in the available space, then paints it at the given origin. @@ -443,11 +443,11 @@ impl AnyElement { available_space: Size, cx: &mut WindowContext, ) { - unsafe { self.0.get_mut() }.draw(origin, available_space, cx) + self.0.draw(origin, available_space, cx) } pub fn inner_id(&self) -> Option { - unsafe { self.0.get() }.element_id() + self.0.element_id() } } diff --git a/crates/gpui2/src/window.rs b/crates/gpui2/src/window.rs index 74b67503d5..76fa36c68b 100644 --- a/crates/gpui2/src/window.rs +++ b/crates/gpui2/src/window.rs @@ -99,7 +99,7 @@ struct FocusEvent { slotmap::new_key_type! { pub struct FocusId; } thread_local! { - pub static FRAME_ARENA: RefCell = RefCell::new(Arena::default()); + pub static FRAME_ARENA: RefCell = RefCell::new(Arena::new(16 * 1024 * 1024)); } impl FocusId { @@ -829,14 +829,15 @@ impl<'a> WindowContext<'a> { mut handler: impl FnMut(&Event, DispatchPhase, &mut WindowContext) + 'static, ) { let order = self.window.next_frame.z_index_stack.clone(); - let handler = FRAME_ARENA.with_borrow_mut(|arena| { - arena.alloc( - move |event: &dyn Any, phase: DispatchPhase, cx: &mut WindowContext<'_>| { - handler(event.downcast_ref().unwrap(), phase, cx) - }, - ) - }); - let handler = unsafe { handler.map(|handler| handler as _) }; + let handler = FRAME_ARENA + .with_borrow_mut(|arena| { + arena.alloc( + move |event: &dyn Any, phase: DispatchPhase, cx: &mut WindowContext<'_>| { + handler(event.downcast_ref().unwrap(), phase, cx) + }, + ) + }) + .map(|handler| handler as _); self.window .next_frame .mouse_listeners @@ -855,14 +856,15 @@ impl<'a> WindowContext<'a> { &mut self, listener: impl Fn(&Event, DispatchPhase, &mut WindowContext) + 'static, ) { - let listener = FRAME_ARENA.with_borrow_mut(|arena| { - arena.alloc(move |event: &dyn Any, phase, cx: &mut WindowContext<'_>| { - if let Some(event) = event.downcast_ref::() { - listener(event, phase, cx) - } + let listener = FRAME_ARENA + .with_borrow_mut(|arena| { + arena.alloc(move |event: &dyn Any, phase, cx: &mut WindowContext<'_>| { + if let Some(event) = event.downcast_ref::() { + listener(event, phase, cx) + } + }) }) - }); - let listener = unsafe { listener.map(|handler| handler as _) }; + .map(|handler| handler as _); self.window.next_frame.dispatch_tree.on_key_event(listener); } @@ -877,8 +879,9 @@ impl<'a> WindowContext<'a> { action_type: TypeId, listener: impl Fn(&dyn Any, DispatchPhase, &mut WindowContext) + 'static, ) { - let listener = FRAME_ARENA.with_borrow_mut(|arena| arena.alloc(listener)); - let listener = unsafe { listener.map(|handler| handler as _) }; + let listener = FRAME_ARENA + .with_borrow_mut(|arena| arena.alloc(listener)) + .map(|handler| handler as _); self.window .next_frame .dispatch_tree @@ -1284,14 +1287,15 @@ impl<'a> WindowContext<'a> { cx.with_key_dispatch(Some(KeyContext::default()), None, |_, cx| { for (action_type, action_listeners) in &cx.app.global_action_listeners { for action_listener in action_listeners.iter().cloned() { - let listener = FRAME_ARENA.with_borrow_mut(|arena| { - arena.alloc( - move |action: &dyn Any, phase, cx: &mut WindowContext<'_>| { - action_listener(action, phase, cx) - }, - ) - }); - let listener = unsafe { listener.map(|listener| listener as _) }; + let listener = FRAME_ARENA + .with_borrow_mut(|arena| { + arena.alloc( + move |action: &dyn Any, phase, cx: &mut WindowContext<'_>| { + action_listener(action, phase, cx) + }, + ) + }) + .map(|listener| listener as _); cx.window .next_frame .dispatch_tree @@ -1478,7 +1482,6 @@ impl<'a> WindowContext<'a> { // Capture phase, events bubble from back to front. Handlers for this phase are used for // special purposes, such as detecting events outside of a given Bounds. for (_, handler) in &mut handlers { - let handler = unsafe { handler.get_mut() }; handler(event, DispatchPhase::Capture, self); if !self.app.propagate_event { break; @@ -1488,7 +1491,6 @@ impl<'a> WindowContext<'a> { // Bubble phase, where most normal handlers do their work. if self.app.propagate_event { for (_, handler) in handlers.iter_mut().rev() { - let handler = unsafe { handler.get_mut() }; handler(event, DispatchPhase::Bubble, self); if !self.app.propagate_event { break; @@ -1538,8 +1540,7 @@ impl<'a> WindowContext<'a> { context_stack.push(context); } - for mut key_listener in node.key_listeners.clone() { - let key_listener = unsafe { key_listener.get_mut() }; + for key_listener in node.key_listeners.clone() { key_listener(event, DispatchPhase::Capture, self); if !self.propagate_event { return; @@ -1551,8 +1552,7 @@ impl<'a> WindowContext<'a> { for node_id in dispatch_path.iter().rev() { // Handle low level key events let node = self.window.rendered_frame.dispatch_tree.node(*node_id); - for mut key_listener in node.key_listeners.clone() { - let key_listener = unsafe { key_listener.get_mut() }; + for key_listener in node.key_listeners.clone() { key_listener(event, DispatchPhase::Bubble, self); if !self.propagate_event { return; @@ -1604,12 +1604,11 @@ impl<'a> WindowContext<'a> { let node = self.window.rendered_frame.dispatch_tree.node(*node_id); for DispatchActionListener { action_type, - mut listener, + listener, } in node.action_listeners.clone() { let any_action = action.as_any(); if action_type == any_action.type_id() { - let listener = unsafe { listener.get_mut() }; listener(any_action, DispatchPhase::Capture, self); if !self.propagate_event { return; @@ -1622,13 +1621,12 @@ impl<'a> WindowContext<'a> { let node = self.window.rendered_frame.dispatch_tree.node(*node_id); for DispatchActionListener { action_type, - mut listener, + listener, } in node.action_listeners.clone() { let any_action = action.as_any(); if action_type == any_action.type_id() { self.propagate_event = false; // Actions stop propagation by default during the bubble phase - let listener = unsafe { listener.get_mut() }; listener(any_action, DispatchPhase::Bubble, self); if !self.propagate_event { return; From 5a4e2e6b90900fe685ce69ff230da5163f370a76 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 18 Dec 2023 10:41:20 +0100 Subject: [PATCH 4/4] Use a closure to allocate structs onto the `Arena` This is a trick borrowed from Bumpalo that helps LLVM understand it should instantiate the object directly on the heap, as opposed to doing so on the stack and then moving it. --- crates/gpui2/src/arena.rs | 36 ++++++++++++++++-------- crates/gpui2/src/element.rs | 9 +++--- crates/gpui2/src/gpui2.rs | 1 + crates/gpui2/src/window.rs | 56 +++++++++++++++++-------------------- 4 files changed, 55 insertions(+), 47 deletions(-) diff --git a/crates/gpui2/src/arena.rs b/crates/gpui2/src/arena.rs index a362c720a2..c99f09d29f 100644 --- a/crates/gpui2/src/arena.rs +++ b/crates/gpui2/src/arena.rs @@ -12,6 +12,7 @@ struct ArenaElement { } impl Drop for ArenaElement { + #[inline(always)] fn drop(&mut self) { unsafe { (self.drop)(self.value); @@ -48,15 +49,23 @@ impl Arena { } #[inline(always)] - pub fn alloc(&mut self, value: T) -> ArenaRef { + pub fn alloc(&mut self, f: impl FnOnce() -> T) -> ArenaRef { + #[inline(always)] + unsafe fn inner_writer(ptr: *mut T, f: F) + where + F: FnOnce() -> T, + { + ptr::write(ptr, f()); + } + unsafe fn drop(ptr: NonNull) { std::ptr::drop_in_place(ptr.cast::().as_ptr()); } unsafe { - let layout = alloc::Layout::for_value(&value).pad_to_align(); + let layout = alloc::Layout::new::().pad_to_align(); let ptr = NonNull::new_unchecked(self.start.as_ptr().add(self.offset).cast::()); - ptr::write(ptr.as_ptr(), value); + inner_writer(ptr.as_ptr(), f); self.elements.push(ArenaElement { value: ptr.cast(), @@ -92,6 +101,7 @@ impl Clone for ArenaRef { } impl ArenaRef { + #[inline(always)] pub fn map(mut self, f: impl FnOnce(&mut T) -> &mut U) -> ArenaRef { ArenaRef { ptr: unsafe { NonNull::new_unchecked(f(&mut *self)) }, @@ -110,6 +120,7 @@ impl ArenaRef { impl Deref for ArenaRef { type Target = T; + #[inline(always)] fn deref(&self) -> &Self::Target { self.validate(); unsafe { self.ptr.as_ref() } @@ -117,6 +128,7 @@ impl Deref for ArenaRef { } impl DerefMut for ArenaRef { + #[inline(always)] fn deref_mut(&mut self) -> &mut Self::Target { self.validate(); unsafe { self.ptr.as_mut() } @@ -132,20 +144,20 @@ mod tests { #[test] fn test_arena() { let mut arena = Arena::new(1024); - let a = arena.alloc(1u64); - let b = arena.alloc(2u32); - let c = arena.alloc(3u16); - let d = arena.alloc(4u8); + let a = arena.alloc(|| 1u64); + let b = arena.alloc(|| 2u32); + let c = arena.alloc(|| 3u16); + let d = arena.alloc(|| 4u8); assert_eq!(*a, 1); assert_eq!(*b, 2); assert_eq!(*c, 3); assert_eq!(*d, 4); arena.clear(); - let a = arena.alloc(5u64); - let b = arena.alloc(6u32); - let c = arena.alloc(7u16); - let d = arena.alloc(8u8); + let a = arena.alloc(|| 5u64); + let b = arena.alloc(|| 6u32); + let c = arena.alloc(|| 7u16); + let d = arena.alloc(|| 8u8); assert_eq!(*a, 5); assert_eq!(*b, 6); assert_eq!(*c, 7); @@ -159,7 +171,7 @@ mod tests { self.0.set(true); } } - arena.alloc(DropGuard(dropped.clone())); + arena.alloc(|| DropGuard(dropped.clone())); arena.clear(); assert!(dropped.get()); } diff --git a/crates/gpui2/src/element.rs b/crates/gpui2/src/element.rs index d54d1c245d..6f739d8e33 100644 --- a/crates/gpui2/src/element.rs +++ b/crates/gpui2/src/element.rs @@ -1,6 +1,6 @@ use crate::{ - arena::ArenaRef, AvailableSpace, BorrowWindow, Bounds, ElementId, LayoutId, Pixels, Point, - Size, ViewContext, WindowContext, FRAME_ARENA, + frame_alloc, ArenaRef, AvailableSpace, BorrowWindow, Bounds, ElementId, LayoutId, Pixels, + Point, Size, ViewContext, WindowContext, }; use derive_more::{Deref, DerefMut}; pub(crate) use smallvec::SmallVec; @@ -413,9 +413,8 @@ impl AnyElement { E: 'static + Element, E::State: Any, { - let element = - FRAME_ARENA.with_borrow_mut(|arena| arena.alloc(Some(DrawableElement::new(element)))); - let element = element.map(|element| element as &mut dyn ElementObject); + let element = frame_alloc(|| Some(DrawableElement::new(element))) + .map(|element| element as &mut dyn ElementObject); AnyElement(element) } diff --git a/crates/gpui2/src/gpui2.rs b/crates/gpui2/src/gpui2.rs index 322ca20608..6af274ef5d 100644 --- a/crates/gpui2/src/gpui2.rs +++ b/crates/gpui2/src/gpui2.rs @@ -39,6 +39,7 @@ mod private { pub use action::*; pub use anyhow::Result; pub use app::*; +pub(crate) use arena::*; pub use assets::*; pub use color::*; pub use ctor::ctor; diff --git a/crates/gpui2/src/window.rs b/crates/gpui2/src/window.rs index 56c5466b63..337f61272f 100644 --- a/crates/gpui2/src/window.rs +++ b/crates/gpui2/src/window.rs @@ -102,6 +102,11 @@ thread_local! { pub static FRAME_ARENA: RefCell = RefCell::new(Arena::new(16 * 1024 * 1024)); } +#[inline(always)] +pub(crate) fn frame_alloc(f: impl FnOnce() -> T) -> ArenaRef { + FRAME_ARENA.with_borrow_mut(|arena| arena.alloc(f)) +} + impl FocusId { /// Obtains whether the element associated with this handle is currently focused. pub fn is_focused(&self, cx: &WindowContext) -> bool { @@ -829,15 +834,12 @@ impl<'a> WindowContext<'a> { mut handler: impl FnMut(&Event, DispatchPhase, &mut WindowContext) + 'static, ) { let order = self.window.next_frame.z_index_stack.clone(); - let handler = FRAME_ARENA - .with_borrow_mut(|arena| { - arena.alloc( - move |event: &dyn Any, phase: DispatchPhase, cx: &mut WindowContext<'_>| { - handler(event.downcast_ref().unwrap(), phase, cx) - }, - ) - }) - .map(|handler| handler as _); + let handler = frame_alloc(|| { + move |event: &dyn Any, phase: DispatchPhase, cx: &mut WindowContext<'_>| { + handler(event.downcast_ref().unwrap(), phase, cx) + } + }) + .map(|handler| handler as _); self.window .next_frame .mouse_listeners @@ -856,15 +858,14 @@ impl<'a> WindowContext<'a> { &mut self, listener: impl Fn(&Event, DispatchPhase, &mut WindowContext) + 'static, ) { - let listener = FRAME_ARENA - .with_borrow_mut(|arena| { - arena.alloc(move |event: &dyn Any, phase, cx: &mut WindowContext<'_>| { - if let Some(event) = event.downcast_ref::() { - listener(event, phase, cx) - } - }) - }) - .map(|handler| handler as _); + let listener = frame_alloc(|| { + move |event: &dyn Any, phase, cx: &mut WindowContext<'_>| { + if let Some(event) = event.downcast_ref::() { + listener(event, phase, cx) + } + } + }) + .map(|handler| handler as _); self.window.next_frame.dispatch_tree.on_key_event(listener); } @@ -879,9 +880,7 @@ impl<'a> WindowContext<'a> { action_type: TypeId, listener: impl Fn(&dyn Any, DispatchPhase, &mut WindowContext) + 'static, ) { - let listener = FRAME_ARENA - .with_borrow_mut(|arena| arena.alloc(listener)) - .map(|handler| handler as _); + let listener = frame_alloc(|| listener).map(|handler| handler as _); self.window .next_frame .dispatch_tree @@ -1286,15 +1285,12 @@ impl<'a> WindowContext<'a> { cx.with_key_dispatch(Some(KeyContext::default()), None, |_, cx| { for (action_type, action_listeners) in &cx.app.global_action_listeners { for action_listener in action_listeners.iter().cloned() { - let listener = FRAME_ARENA - .with_borrow_mut(|arena| { - arena.alloc( - move |action: &dyn Any, phase, cx: &mut WindowContext<'_>| { - action_listener(action, phase, cx) - }, - ) - }) - .map(|listener| listener as _); + let listener = frame_alloc(|| { + move |action: &dyn Any, phase, cx: &mut WindowContext<'_>| { + action_listener(action, phase, cx) + } + }) + .map(|listener| listener as _); cx.window .next_frame .dispatch_tree