oak-team/oak-gpui
3
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Throttle the sending of UpdateFollowers messages (#8918)

Browse Source 
## Problem

We're trying to figure out why we sometimes see high latency when
collaborating, even though the collab server logs indicate that messages
are not taking long to process.

We think that high volumes of certain types of messages, including
`UpdateFollowers` may cause a lot of messages to queue up, causing
delays before collab sees certain messages.

## Fix

This PR reduces the number of `UpdateFollowers` messages that clients
send to collab when scrolling around or moving the cursor, using a
time-based throttle.

The downside of this change is that scrolling will not be as smooth when
following someone. The advantage is that it will be much easier to keep
up with the stream of updates, since they will be sent much less
frequently.

## Release Notes:

- Fixed slowness that could occur when collaborating due to excessive
messages being sent to support following.

---------

Co-authored-by: Nathan <nathan@zed.dev>
Co-authored-by: Conrad <conrad@zed.dev>
Co-authored-by: Antonio Scandurra <me@as-cii.com>
Co-authored-by: Thorsten <thorsten@zed.dev>
Co-authored-by: Thorsten Ball <mrnugget@gmail.com>
This commit is contained in:
Max Brunsfeld
2024-03-06 14:58:41 +01:00
committed by GitHub
co-authored by Nathan Conrad Antonio Scandurra Thorsten Thorsten Ball
parent c8383e3b18
commit 6036830049
7 changed files with 109 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/workspace/src/item.rs
+64 -34
View File
@@ -11,6 +11,7 @@ use client::{
proto::{self, PeerId},
Client,
};
use futures::{channel::mpsc, StreamExt};
use gpui::{
AnyElement, AnyView, AppContext, Entity, EntityId, EventEmitter, FocusHandle, FocusableView,
HighlightStyle, Model, Pixels, Point, SharedString, Task, View, ViewContext, WeakView,
@@ -27,14 +28,13 @@ use std::{
ops::Range,
path::PathBuf,
rc::Rc,
sync::{
atomic::{AtomicBool, Ordering},
Arc,
},
sync::Arc,
time::Duration,
};
use theme::Theme;
pub const LEADER_UPDATE_THROTTLE: Duration = Duration::from_millis(200);
#[derive(Deserialize)]
pub struct ItemSettings {
pub git_status: bool,
@@ -415,7 +415,7 @@ impl<T: Item> ItemHandle for View<T> {
followed_item.is_project_item(cx),
proto::update_followers::Variant::CreateView(proto::View {
id: followed_item
.remote_id(&workspace.app_state.client, cx)
.remote_id(&workspace.client(), cx)
.map(|id| id.to_proto()),
variant: Some(message),
leader_id: workspace.leader_for_pane(&pane),
@@ -431,8 +431,46 @@ impl<T: Item> ItemHandle for View<T> {
.is_none()
{
let mut pending_autosave = DelayedDebouncedEditAction::new();
let (pending_update_tx, mut pending_update_rx) = mpsc::unbounded();
let pending_update = Rc::new(RefCell::new(None));
let pending_update_scheduled = Arc::new(AtomicBool::new(false));
let mut send_follower_updates = None;
if let Some(item) = self.to_followable_item_handle(cx) {
let is_project_item = item.is_project_item(cx);
let item = item.downgrade();
send_follower_updates = Some(cx.spawn({
let pending_update = pending_update.clone();
|workspace, mut cx| async move {
while let Some(mut leader_id) = pending_update_rx.next().await {
while let Ok(Some(id)) = pending_update_rx.try_next() {
leader_id = id;
}
workspace.update(&mut cx, |workspace, cx| {
let item = item.upgrade().expect(
"item to be alive, otherwise task would have been dropped",
);
workspace.update_followers(
is_project_item,
proto::update_followers::Variant::UpdateView(
proto::UpdateView {
id: item
.remote_id(workspace.client(), cx)
.map(|id| id.to_proto()),
variant: pending_update.borrow_mut().take(),
leader_id,
},
),
cx,
);
})?;
cx.background_executor().timer(LEADER_UPDATE_THROTTLE).await;
}
anyhow::Ok(())
}
}));
}
let mut event_subscription =
Some(cx.subscribe(self, move |workspace, item, event, cx| {
@@ -448,9 +486,7 @@ impl<T: Item> ItemHandle for View<T> {
};
if let Some(item) = item.to_followable_item_handle(cx) {
let is_project_item = item.is_project_item(cx);
let leader_id = workspace.leader_for_pane(&pane);
let follow_event = item.to_follow_event(event);
if leader_id.is_some()
&& matches!(follow_event, Some(FollowEvent::Unfollow))
@@ -458,35 +494,13 @@ impl<T: Item> ItemHandle for View<T> {
workspace.unfollow(&pane, cx);
}
if item.focus_handle(cx).contains_focused(cx)
&& item.add_event_to_update_proto(
if item.focus_handle(cx).contains_focused(cx) {
item.add_event_to_update_proto(
event,
&mut pending_update.borrow_mut(),
cx,
)
&& !pending_update_scheduled.load(Ordering::SeqCst)
{
pending_update_scheduled.store(true, Ordering::SeqCst);
cx.defer({
let pending_update = pending_update.clone();
let pending_update_scheduled = pending_update_scheduled.clone();
move |this, cx| {
pending_update_scheduled.store(false, Ordering::SeqCst);
this.update_followers(
is_project_item,
proto::update_followers::Variant::UpdateView(
proto::UpdateView {
id: item
.remote_id(&this.app_state.client, cx)
.map(|id| id.to_proto()),
variant: pending_update.borrow_mut().take(),
leader_id,
},
),
cx,
);
}
});
);
pending_update_tx.unbounded_send(leader_id).ok();
}
}
@@ -535,6 +549,7 @@ impl<T: Item> ItemHandle for View<T> {
cx.observe_release(self, move |workspace, _, _| {
workspace.panes_by_item.remove(&item_id);
event_subscription.take();
send_follower_updates.take();
})
.detach();
}
@@ -715,6 +730,7 @@ pub trait FollowableItem: Item {
pub trait FollowableItemHandle: ItemHandle {
fn remote_id(&self, client: &Arc<Client>, cx: &WindowContext) -> Option<ViewId>;
fn downgrade(&self) -> Box<dyn WeakFollowableItemHandle>;
fn set_leader_peer_id(&self, leader_peer_id: Option<PeerId>, cx: &mut WindowContext);
fn to_state_proto(&self, cx: &WindowContext) -> Option<proto::view::Variant>;
fn add_event_to_update_proto(
@@ -743,6 +759,10 @@ impl<T: FollowableItem> FollowableItemHandle for View<T> {
})
}
fn downgrade(&self) -> Box<dyn WeakFollowableItemHandle> {
Box::new(self.downgrade())
}
fn set_leader_peer_id(&self, leader_peer_id: Option<PeerId>, cx: &mut WindowContext) {
self.update(cx, |this, cx| this.set_leader_peer_id(leader_peer_id, cx))
}
@@ -782,6 +802,16 @@ impl<T: FollowableItem> FollowableItemHandle for View<T> {
}
}
pub trait WeakFollowableItemHandle: Send + Sync {
fn upgrade(&self) -> Option<Box<dyn FollowableItemHandle>>;
}
impl<T: FollowableItem> WeakFollowableItemHandle for WeakView<T> {
fn upgrade(&self) -> Option<Box<dyn FollowableItemHandle>> {
Some(Box::new(self.upgrade()?))
}
}
#[cfg(any(test, feature = "test-support"))]
pub mod test {
use super::{Item, ItemEvent};
crates/workspace/src/workspace.rs
+1 -1
View File
@@ -1108,7 +1108,7 @@ impl Workspace {
)
}
pub fn client(&self) -> &Client {
pub fn client(&self) -> &Arc<Client> {
&self.app_state.client
}