From ef1ec88523329810ec867110dd60803421133f97 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 9 Mar 2022 10:48:52 +0100 Subject: [PATCH] Remove delegate support from GPUI We added this because we thought it would save some allocations when sending operations given that we could move them to the delegate upon notifying it, but the reality is that we serialize operations and that only requires a reference. --- crates/gpui/src/app.rs | 124 ++-------------------------------- crates/language/src/tests.rs | 6 +- crates/project/src/project.rs | 4 +- 3 files changed, 10 insertions(+), 124 deletions(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 6727fc5f08f8caeba74e1e344c587494f6fd8053..e91963bfa6341fac51089a53de34e1121346cec9 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -740,7 +740,6 @@ type ActionCallback = type GlobalActionCallback = dyn FnMut(&dyn AnyAction, &mut MutableAppContext); type SubscriptionCallback = Box bool>; -type DelegationCallback = Box, &mut MutableAppContext) -> bool>; type ObservationCallback = Box bool>; type ReleaseObservationCallback = Box; @@ -758,7 +757,6 @@ pub struct MutableAppContext { next_subscription_id: usize, frame_count: usize, subscriptions: Arc>>>, - delegations: Arc>>, observations: Arc>>>, release_observations: Arc>>>, presenters_and_platform_windows: @@ -806,7 +804,6 @@ impl MutableAppContext { next_subscription_id: 0, frame_count: 0, subscriptions: Default::default(), - delegations: Default::default(), observations: Default::default(), release_observations: Default::default(), presenters_and_platform_windows: HashMap::new(), @@ -1152,35 +1149,6 @@ impl MutableAppContext { } } - fn become_delegate_internal(&mut self, handle: &H, mut callback: F) -> Subscription - where - E: Entity, - H: Handle, - F: 'static + FnMut(H, E::Event, &mut Self) -> bool, - { - let id = post_inc(&mut self.next_subscription_id); - let emitter = handle.downgrade(); - self.delegations.lock().insert( - handle.id(), - ( - id, - Box::new(move |payload, cx| { - if let Some(emitter) = H::upgrade_from(&emitter, cx.as_ref()) { - let payload = *payload.downcast().expect("downcast is type safe"); - callback(emitter, payload, cx) - } else { - false - } - }), - ), - ); - Subscription::Delegation { - id, - entity_id: handle.id(), - delegations: Some(Arc::downgrade(&self.delegations)), - } - } - pub fn observe_release(&mut self, handle: &H, mut callback: F) -> Subscription where E: Entity, @@ -1730,17 +1698,6 @@ impl MutableAppContext { } } } - - let delegate = self.delegations.lock().remove(&entity_id); - if let Some((id, mut callback)) = delegate { - let alive = callback(payload, self); - if alive { - self.delegations - .lock() - .entry(entity_id) - .or_insert_with(|| (id, callback)); - } - } } fn notify_model_observers(&mut self, observed_id: usize) { @@ -2387,26 +2344,6 @@ impl<'a, T: Entity> ModelContext<'a, T> { }) } - pub fn become_delegate(&mut self, handle: &H, mut callback: F) -> Subscription - where - E: Entity, - H: Handle, - F: 'static + FnMut(&mut T, H, E::Event, &mut ModelContext), - { - let delegate = self.weak_handle(); - self.app - .become_delegate_internal(handle, move |emitter, event, cx| { - if let Some(delegate) = delegate.upgrade(cx) { - delegate.update(cx, |subscriber, cx| { - callback(subscriber, emitter, event, cx); - }); - true - } else { - false - } - }) - } - pub fn observe_release( &mut self, handle: &ModelHandle, @@ -2672,26 +2609,6 @@ impl<'a, T: View> ViewContext<'a, T> { }) } - pub fn become_delegate(&mut self, handle: &H, mut callback: F) -> Subscription - where - E: Entity, - H: Handle, - F: 'static + FnMut(&mut T, H, E::Event, &mut ViewContext), - { - let delegate = self.weak_handle(); - self.app - .become_delegate_internal(handle, move |emitter, event, cx| { - if let Some(delegate) = delegate.upgrade(cx) { - delegate.update(cx, |subscriber, cx| { - callback(subscriber, emitter, event, cx); - }); - true - } else { - false - } - }) - } - pub fn observe_release(&mut self, handle: &H, mut callback: F) -> Subscription where E: Entity, @@ -3845,11 +3762,6 @@ pub enum Subscription { entity_id: usize, subscriptions: Option>>>>, }, - Delegation { - id: usize, - entity_id: usize, - delegations: Option>>>, - }, Observation { id: usize, entity_id: usize, @@ -3869,9 +3781,6 @@ impl Subscription { Subscription::Subscription { subscriptions, .. } => { subscriptions.take(); } - Subscription::Delegation { delegations, .. } => { - delegations.take(); - } Subscription::Observation { observations, .. } => { observations.take(); } @@ -3918,19 +3827,6 @@ impl Drop for Subscription { } } } - Subscription::Delegation { - id, - entity_id, - delegations, - } => { - if let Some(delegations) = delegations.as_ref().and_then(Weak::upgrade) { - if let Entry::Occupied(entry) = delegations.lock().entry(*entity_id) { - if *id == entry.get().0 { - let _ = entry.remove(); - } - } - } - } } } } @@ -4197,11 +4093,6 @@ mod tests { let handle_1 = cx.add_model(|_| Model::default()); let handle_2 = cx.add_model(|_| Model::default()); handle_1.update(cx, |_, cx| { - cx.become_delegate(&handle_2, |model, _, event, _| { - model.events.push(event * 3); - }) - .detach(); - cx.subscribe(&handle_2, move |model: &mut Model, emitter, event, cx| { model.events.push(*event); @@ -4214,10 +4105,10 @@ mod tests { }); handle_2.update(cx, |_, c| c.emit(7)); - assert_eq!(handle_1.read(cx).events, vec![7, 21]); + assert_eq!(handle_1.read(cx).events, vec![7]); handle_2.update(cx, |_, c| c.emit(5)); - assert_eq!(handle_1.read(cx).events, vec![7, 21, 5, 10, 15]); + assert_eq!(handle_1.read(cx).events, vec![7, 5, 10]); } #[crate::test(self)] @@ -4475,11 +4366,6 @@ mod tests { let handle_3 = cx.add_model(|_| Model); handle_1.update(cx, |_, cx| { - cx.become_delegate(&handle_2, |me, _, event, _| { - me.events.push(event * 3); - }) - .detach(); - cx.subscribe(&handle_2, move |me, emitter, event, cx| { me.events.push(*event); @@ -4497,13 +4383,13 @@ mod tests { }); handle_2.update(cx, |_, c| c.emit(7)); - assert_eq!(handle_1.read(cx).events, vec![7, 21]); + assert_eq!(handle_1.read(cx).events, vec![7]); handle_2.update(cx, |_, c| c.emit(5)); - assert_eq!(handle_1.read(cx).events, vec![7, 21, 5, 10, 15]); + assert_eq!(handle_1.read(cx).events, vec![7, 5, 10]); handle_3.update(cx, |_, c| c.emit(9)); - assert_eq!(handle_1.read(cx).events, vec![7, 21, 5, 10, 15, 9]); + assert_eq!(handle_1.read(cx).events, vec![7, 5, 10, 9]); } #[crate::test(self)] diff --git a/crates/language/src/tests.rs b/crates/language/src/tests.rs index 972821f77b867cc3b26263ee8380ca7ab34fe0d1..582410a9be9968f79f50fb514bea35fb5694be5b 100644 --- a/crates/language/src/tests.rs +++ b/crates/language/src/tests.rs @@ -80,7 +80,7 @@ fn test_edit_events(cx: &mut gpui::MutableAppContext) { let buffer1_ops = buffer1_ops.clone(); |buffer, cx| { let buffer_1_events = buffer_1_events.clone(); - cx.become_delegate(&buffer1, move |_, _, event, _| match event { + cx.subscribe(&buffer1, move |_, _, event, _| match event.clone() { Event::Operation(op) => buffer1_ops.borrow_mut().push(op), event @ _ => buffer_1_events.borrow_mut().push(event), }) @@ -610,7 +610,7 @@ fn test_random_collaboration(cx: &mut MutableAppContext, mut rng: StdRng) { let mut buffer = Buffer::new(i as ReplicaId, base_text.as_str(), cx); buffer.set_group_interval(Duration::from_millis(rng.gen_range(0..=200))); let network = network.clone(); - cx.become_delegate(&cx.handle(), move |buffer, _, event, _| { + cx.subscribe(&cx.handle(), move |buffer, _, event, _| { if let Event::Operation(op) = event { network .borrow_mut() @@ -706,7 +706,7 @@ fn test_random_collaboration(cx: &mut MutableAppContext, mut rng: StdRng) { Buffer::from_proto(new_replica_id, old_buffer, None, cx).unwrap(); new_buffer.set_group_interval(Duration::from_millis(rng.gen_range(0..=200))); let network = network.clone(); - cx.become_delegate(&cx.handle(), move |buffer, _, event, _| { + cx.subscribe(&cx.handle(), move |buffer, _, event, _| { if let Event::Operation(op) = event { network.borrow_mut().broadcast( buffer.replica_id(), diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index fab9212b16f6ef4f54840349a2bb988cd013969a..e8ed9fbbb8196e9dbfabfc9aa5bc709b1917c40c 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -942,7 +942,7 @@ impl Project { remote_id ))?, } - cx.become_delegate(buffer, |this, buffer, event, cx| { + cx.subscribe(buffer, |this, buffer, event, cx| { this.on_buffer_event(buffer, event, cx); }) .detach(); @@ -1028,7 +1028,7 @@ impl Project { fn on_buffer_event( &mut self, buffer: ModelHandle, - event: BufferEvent, + event: &BufferEvent, cx: &mut ModelContext, ) -> Option<()> { match event {