From 52057c5619f7f3bfa1d334315fbb3d0f24b33166 Mon Sep 17 00:00:00 2001 From: Mikayla Date: Fri, 15 Sep 2023 13:18:50 -0700 Subject: [PATCH] Simplify path representation in collab panel Optimize set representation in collab --- Cargo.lock | 1 + crates/channel/src/channel_store.rs | 71 ++++-- .../src/channel_store/channel_index.rs | 4 +- crates/collab/Cargo.toml | 1 + crates/collab/src/db/queries/channels.rs | 45 +++- crates/collab_ui/src/collab_panel.rs | 212 ++++++------------ 6 files changed, 161 insertions(+), 173 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 829ed18bbbf8334364902ce382354ab615922594..327ca26937d78970a4a09c74f0a9854da51214ff 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1505,6 +1505,7 @@ dependencies = [ "serde_json", "settings", "sha-1 0.9.8", + "smallvec", "sqlx", "text", "theme", diff --git a/crates/channel/src/channel_store.rs b/crates/channel/src/channel_store.rs index 208247aca557f1ffa3a766488780192c8ca83121..702679fdda3aa13055ad7ccef19fea96372b14f5 100644 --- a/crates/channel/src/channel_store.rs +++ b/crates/channel/src/channel_store.rs @@ -3,12 +3,12 @@ mod channel_index; use crate::{channel_buffer::ChannelBuffer, channel_chat::ChannelChat}; use anyhow::{anyhow, Result}; use client::{Client, Subscription, User, UserId, UserStore}; -use collections::{hash_map, HashMap, HashSet}; +use collections::{hash_map::{self, DefaultHasher}, HashMap, HashSet}; use futures::{channel::mpsc, future::Shared, Future, FutureExt, StreamExt}; use gpui::{AppContext, AsyncAppContext, Entity, ModelContext, ModelHandle, Task, WeakModelHandle}; use rpc::{proto, TypedEnvelope}; use serde_derive::{Deserialize, Serialize}; -use std::{mem, ops::Deref, sync::Arc, time::Duration}; +use std::{mem, ops::Deref, sync::Arc, time::Duration, borrow::Cow, hash::{Hash, Hasher}}; use util::ResultExt; use self::channel_index::ChannelIndex; @@ -43,26 +43,6 @@ pub struct Channel { #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Serialize, Deserialize)] pub struct ChannelPath(Arc<[ChannelId]>); -impl Deref for ChannelPath { - type Target = [ChannelId]; - - fn deref(&self) -> &Self::Target { - &self.0 - } -} - -impl ChannelPath { - pub fn parent_id(&self) -> Option { - self.0.len().checked_sub(2).map(|i| self.0[i]) - } -} - -impl Default for ChannelPath { - fn default() -> Self { - ChannelPath(Arc::from([])) - } -} - pub struct ChannelMembership { pub user: Arc, pub kind: proto::channel_member::Kind, @@ -860,3 +840,50 @@ impl ChannelStore { })) } } + +impl Deref for ChannelPath { + type Target = [ChannelId]; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl ChannelPath { + pub fn new(path: Arc<[ChannelId]>) -> Self { + debug_assert!(path.len() >= 1); + Self(path) + } + + pub fn parent_id(&self) -> Option { + self.0.len().checked_sub(2).map(|i| self.0[i]) + } + + pub fn channel_id(&self) -> ChannelId { + self.0[self.0.len() - 1] + } + + pub fn unique_id(&self) -> u64 { + let mut hasher = DefaultHasher::new(); + self.0.deref().hash(&mut hasher); + hasher.finish() + } +} + +impl From for Cow<'static, ChannelPath> { + fn from(value: ChannelPath) -> Self { + Cow::Owned(value) + } +} + +impl<'a> From<&'a ChannelPath> for Cow<'a, ChannelPath> { + fn from(value: &'a ChannelPath) -> Self { + Cow::Borrowed(value) + } +} + +impl Default for ChannelPath { + fn default() -> Self { + ChannelPath(Arc::from([])) + } +} diff --git a/crates/channel/src/channel_store/channel_index.rs b/crates/channel/src/channel_store/channel_index.rs index dd911ab363e3cccca75014b0edea8e2ffa10b0e6..f7d9e873ae5eb99e533d85e05951689ae588a740 100644 --- a/crates/channel/src/channel_store/channel_index.rs +++ b/crates/channel/src/channel_store/channel_index.rs @@ -108,7 +108,7 @@ impl<'a> ChannelPathsEditGuard<'a> { if path.ends_with(&[parent_id]) { let mut new_path = path.to_vec(); new_path.push(channel_id); - self.paths.insert(ix + 1, ChannelPath(new_path.into())); + self.paths.insert(ix + 1, ChannelPath::new(new_path.into())); ix += 2; } else if path.get(0) == Some(&channel_id) { // Clear out any paths that have this chahnnel as their root @@ -120,7 +120,7 @@ impl<'a> ChannelPathsEditGuard<'a> { } fn insert_root(&mut self, channel_id: ChannelId) { - self.paths.push(ChannelPath(Arc::from([channel_id]))); + self.paths.push(ChannelPath::new(Arc::from([channel_id]))); } } diff --git a/crates/collab/Cargo.toml b/crates/collab/Cargo.toml index c580e911bcf67fa55952380d00fc992762f14ce8..57db1cd761e71257b65e13aaaf95f01df0bc85d2 100644 --- a/crates/collab/Cargo.toml +++ b/crates/collab/Cargo.toml @@ -41,6 +41,7 @@ prost.workspace = true rand.workspace = true reqwest = { version = "0.11", features = ["json"], optional = true } scrypt = "0.7" +smallvec.workspace = true # Remove fork dependency when a version with https://github.com/SeaQL/sea-orm/pull/1283 is released. sea-orm = { git = "https://github.com/zed-industries/sea-orm", rev = "18f4c691085712ad014a51792af75a9044bacee6", features = ["sqlx-postgres", "postgres-array", "runtime-tokio-rustls"] } sea-query = "0.27" diff --git a/crates/collab/src/db/queries/channels.rs b/crates/collab/src/db/queries/channels.rs index de464c17562e7f8ab5a586a4ed404f20fe0d571a..dab392c583d82f82b95ac709d07ba73e43c77e1a 100644 --- a/crates/collab/src/db/queries/channels.rs +++ b/crates/collab/src/db/queries/channels.rs @@ -1,6 +1,8 @@ +use smallvec::SmallVec; + use super::*; -type ChannelDescendants = HashMap>; +type ChannelDescendants = HashMap>; impl Database { #[cfg(test)] @@ -150,7 +152,7 @@ impl Database { .exec(&*tx) .await?; - // Delete any other paths that incldue this channel + // Delete any other paths that include this channel let sql = r#" DELETE FROM channel_paths WHERE @@ -351,7 +353,7 @@ impl Database { let parents = parents_by_child_id.get(&row.id).unwrap(); if parents.len() > 0 { let mut added_channel = false; - for parent in parents { + for parent in parents.iter() { // Trim out any dangling parent pointers. // That the user doesn't have access to if trim_dangling_parents { @@ -843,7 +845,7 @@ impl Database { ); tx.execute(channel_paths_stmt).await?; for (from_id, to_ids) in from_descendants.iter().filter(|(id, _)| id != &&channel) { - for to_id in to_ids { + for to_id in to_ids.iter() { let channel_paths_stmt = Statement::from_sql_and_values( self.pool.get_database_backend(), sql, @@ -979,3 +981,38 @@ impl Database { enum QueryUserIds { UserId, } + +struct SmallSet(SmallVec<[T; 1]>); + +impl Deref for SmallSet { + type Target = [T]; + + fn deref(&self) -> &Self::Target { + self.0.deref() + } +} + +impl Default for SmallSet { + fn default() -> Self { + Self(SmallVec::new()) + } +} + +impl SmallSet { + fn insert(&mut self, value: T) -> bool + where + T: Ord, + { + match self.binary_search(&value) { + Ok(_) => false, + Err(ix) => { + self.0.insert(ix, value); + true + }, + } + } + + fn clear(&mut self) { + self.0.clear(); + } +} diff --git a/crates/collab_ui/src/collab_panel.rs b/crates/collab_ui/src/collab_panel.rs index 2ed2a1a230cd9d1e3bcbc2a453feacc9bab36e9f..2fe6148fe503489939b6fd49db8614ba29a14a8c 100644 --- a/crates/collab_ui/src/collab_panel.rs +++ b/crates/collab_ui/src/collab_panel.rs @@ -50,33 +50,33 @@ use workspace::{ }; #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] -struct RemoveChannel { - channel_id: ChannelId, +struct ToggleCollapse { + location: ChannelPath, } #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] -struct ToggleCollapse { - location: ChannelLocation<'static>, +struct NewChannel { + location: ChannelPath, } #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] -struct NewChannel { - location: ChannelLocation<'static>, +struct RenameChannel { + location: ChannelPath, } #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] -struct InviteMembers { +struct RemoveChannel { channel_id: ChannelId, } #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] -struct ManageMembers { +struct InviteMembers { channel_id: ChannelId, } #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] -struct RenameChannel { - location: ChannelLocation<'static>, +struct ManageMembers { + channel_id: ChannelId, } #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] @@ -148,30 +148,6 @@ impl_actions!( const COLLABORATION_PANEL_KEY: &'static str = "CollaborationPanel"; -#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] -pub struct ChannelLocation<'a> { - channel: ChannelId, - path: Cow<'a, ChannelPath>, -} - -impl From<(ChannelId, ChannelPath)> for ChannelLocation<'static> { - fn from(value: (ChannelId, ChannelPath)) -> Self { - ChannelLocation { - channel: value.0, - path: Cow::Owned(value.1), - } - } -} - -impl<'a> From<(ChannelId, &'a ChannelPath)> for ChannelLocation<'a> { - fn from(value: (ChannelId, &'a ChannelPath)) -> Self { - ChannelLocation { - channel: value.0, - path: Cow::Borrowed(value.1), - } - } -} - pub fn init(cx: &mut AppContext) { settings::register::(cx); contact_finder::init(cx); @@ -190,7 +166,7 @@ pub fn init(cx: &mut AppContext) { cx.add_action(CollabPanel::manage_members); cx.add_action(CollabPanel::rename_selected_channel); cx.add_action(CollabPanel::rename_channel); - cx.add_action(CollabPanel::toggle_channel_collapsed); + cx.add_action(CollabPanel::toggle_channel_collapsed_action); cx.add_action(CollabPanel::collapse_selected_channel); cx.add_action(CollabPanel::expand_selected_channel); cx.add_action(CollabPanel::open_channel_notes); @@ -248,11 +224,11 @@ pub fn init(cx: &mut AppContext) { #[derive(Debug)] pub enum ChannelEditingState { Create { - location: Option>, + location: Option, pending_name: Option, }, Rename { - location: ChannelLocation<'static>, + location: ChannelPath, pending_name: Option, }, } @@ -286,7 +262,7 @@ pub struct CollabPanel { list_state: ListState, subscriptions: Vec, collapsed_sections: Vec
, - collapsed_channels: Vec>, + collapsed_channels: Vec, workspace: WeakViewHandle, context_menu_on_selected: bool, } @@ -294,7 +270,7 @@ pub struct CollabPanel { #[derive(Serialize, Deserialize)] struct SerializedCollabPanel { width: Option, - collapsed_channels: Option>>, + collapsed_channels: Option>, } #[derive(Debug)] @@ -826,15 +802,13 @@ impl CollabPanel { let (channel, path) = channel_store.channel_at_index(mat.candidate_id).unwrap(); let depth = path.len() - 1; - let location: ChannelLocation<'_> = (channel.id, path).into(); - - if collapse_depth.is_none() && self.is_channel_collapsed(&location) { + if collapse_depth.is_none() && self.is_channel_collapsed(path) { collapse_depth = Some(depth); } else if let Some(collapsed_depth) = collapse_depth { if depth > collapsed_depth { continue; } - if self.is_channel_collapsed(&location) { + if self.is_channel_collapsed(path) { collapse_depth = Some(depth); } else { collapse_depth = None; @@ -843,9 +817,9 @@ impl CollabPanel { match &self.channel_editing_state { Some(ChannelEditingState::Create { - location: parent_id, + location: parent_path, .. - }) if *parent_id == Some(location) => { + }) if parent_path.as_ref() == Some(path) => { self.entries.push(ListEntry::Channel { channel: channel.clone(), depth, @@ -854,10 +828,10 @@ impl CollabPanel { self.entries .push(ListEntry::ChannelEditor { depth: depth + 1 }); } - Some(ChannelEditingState::Rename { location, .. }) - if location.channel == channel.id - && location.path == Cow::Borrowed(path) => - { + Some(ChannelEditingState::Rename { + location: parent_path, + .. + }) if parent_path == path => { self.entries.push(ListEntry::ChannelEditor { depth }); } _ => { @@ -1674,13 +1648,7 @@ impl CollabPanel { let channel_id = channel.id; let has_children = self.channel_store.read(cx).has_children(channel_id); - let disclosed = { - let location = ChannelLocation { - channel: channel_id, - path: Cow::Borrowed(&path), - }; - has_children.then(|| !self.collapsed_channels.binary_search(&location).is_ok()) - }; + let disclosed = has_children.then(|| !self.collapsed_channels.binary_search(&path).is_ok()); let is_active = iife!({ let call_channel = ActiveCall::global(cx) @@ -1696,7 +1664,7 @@ impl CollabPanel { enum ChannelCall {} - MouseEventHandler::new::(id(&path) as usize, cx, |state, cx| { + MouseEventHandler::new::(path.unique_id() as usize, cx, |state, cx| { let row_hovered = state.hovered(); Flex::::row() @@ -1767,10 +1735,10 @@ impl CollabPanel { .disclosable( disclosed, Box::new(ToggleCollapse { - location: (channel_id, path.clone()).into(), + location: path.clone(), }), ) - .with_id(id(&path) as usize) + .with_id(path.unique_id() as usize) .with_style(theme.disclosure.clone()) .element() .constrained() @@ -1786,11 +1754,7 @@ impl CollabPanel { this.join_channel_chat(channel_id, cx); }) .on_click(MouseButton::Right, move |e, this, cx| { - this.deploy_channel_context_menu( - Some(e.position), - &(channel_id, path.clone()).into(), - cx, - ); + this.deploy_channel_context_menu(Some(e.position), &path, cx); }) .with_cursor_style(CursorStyle::PointingHand) .into_any() @@ -2037,7 +2001,7 @@ impl CollabPanel { fn deploy_channel_context_menu( &mut self, position: Option, - location: &ChannelLocation<'static>, + path: &ChannelPath, cx: &mut ViewContext, ) { self.context_menu_on_selected = position.is_none(); @@ -2063,20 +2027,16 @@ impl CollabPanel { if let Some(channel_name) = channel_name { items.push(ContextMenuItem::action( format!("Move '#{}' here", channel_name), - MoveChannel { - to: location.channel, - }, + MoveChannel { to: path.channel_id() }, )); items.push(ContextMenuItem::action( format!("Link '#{}' here", channel_name), - LinkChannel { - to: location.channel, - }, + LinkChannel { to: path.channel_id() }, )); items.push(ContextMenuItem::Separator) } - let expand_action_name = if self.is_channel_collapsed(&location) { + let expand_action_name = if self.is_channel_collapsed(&path) { "Expand Subchannels" } else { "Collapse Subchannels" @@ -2086,32 +2046,27 @@ impl CollabPanel { ContextMenuItem::action( expand_action_name, ToggleCollapse { - location: location.clone(), - }, - ), - ContextMenuItem::action( - "Open Notes", - OpenChannelBuffer { - channel_id: location.channel, + location: path.clone(), }, ), + ContextMenuItem::action("Open Notes", OpenChannelBuffer { channel_id: path.channel_id() }), ]); - if self.channel_store.read(cx).is_user_admin(location.channel) { - let parent_id = location.path.parent_id(); + if self.channel_store.read(cx).is_user_admin(path.channel_id()) { + let parent_id = path.parent_id(); items.extend([ ContextMenuItem::Separator, ContextMenuItem::action( "New Subchannel", NewChannel { - location: location.clone(), + location: path.clone(), }, ), ContextMenuItem::action( "Rename", RenameChannel { - location: location.clone(), + location: path.clone(), }, ), ContextMenuItem::Separator, @@ -2121,7 +2076,7 @@ impl CollabPanel { items.push(ContextMenuItem::action( "Unlink from parent", UnlinkChannel { - channel_id: location.channel, + channel_id: path.channel_id(), parent_id, }, )); @@ -2130,7 +2085,7 @@ impl CollabPanel { items.extend([ContextMenuItem::action( "Move this channel", StartMoveChannel { - channel_id: location.channel, + channel_id: path.channel_id(), parent_id, }, )]); @@ -2140,20 +2095,20 @@ impl CollabPanel { ContextMenuItem::action( "Invite Members", InviteMembers { - channel_id: location.channel, + channel_id: path.channel_id(), }, ), ContextMenuItem::action( "Manage Members", ManageMembers { - channel_id: location.channel, + channel_id: path.channel_id(), }, ), ContextMenuItem::Separator, ContextMenuItem::action( "Delete", RemoveChannel { - channel_id: location.channel, + channel_id: path.channel_id(), }, ), ]); @@ -2296,7 +2251,7 @@ impl CollabPanel { .update(cx, |channel_store, cx| { channel_store.create_channel( &channel_name, - location.as_ref().map(|location| location.channel), + location.as_ref().map(|location| location.channel_id()), cx, ) }) @@ -2315,7 +2270,7 @@ impl CollabPanel { self.channel_store .update(cx, |channel_store, cx| { - channel_store.rename(location.channel, &channel_name, cx) + channel_store.rename(location.channel_id(), &channel_name, cx) }) .detach(); cx.notify(); @@ -2342,58 +2297,48 @@ impl CollabPanel { _: &CollapseSelectedChannel, cx: &mut ViewContext, ) { - let Some((channel_id, path)) = self + let Some((_, path)) = self .selected_channel() .map(|(channel, parent)| (channel.id, parent)) else { return; }; - let path = path.to_owned(); - - if self.is_channel_collapsed(&(channel_id, path.clone()).into()) { + if self.is_channel_collapsed(&path) { return; } - self.toggle_channel_collapsed( - &ToggleCollapse { - location: (channel_id, path).into(), - }, - cx, - ) + self.toggle_channel_collapsed(&path.clone(), cx); + } fn expand_selected_channel(&mut self, _: &ExpandSelectedChannel, cx: &mut ViewContext) { - let Some((channel_id, path)) = self + let Some((_, path)) = self .selected_channel() .map(|(channel, parent)| (channel.id, parent)) else { return; }; - let path = path.to_owned(); - - if !self.is_channel_collapsed(&(channel_id, path.clone()).into()) { + if !self.is_channel_collapsed(&path) { return; } - self.toggle_channel_collapsed( - &ToggleCollapse { - location: (channel_id, path).into(), - }, - cx, - ) + self.toggle_channel_collapsed(path.to_owned(), cx) } - fn toggle_channel_collapsed(&mut self, action: &ToggleCollapse, cx: &mut ViewContext) { - let location = action.location.clone(); + fn toggle_channel_collapsed_action(&mut self, action: &ToggleCollapse, cx: &mut ViewContext) { + self.toggle_channel_collapsed(&action.location, cx); + } - match self.collapsed_channels.binary_search(&location) { + fn toggle_channel_collapsed<'a>(&mut self, path: impl Into>, cx: &mut ViewContext) { + let path = path.into(); + match self.collapsed_channels.binary_search(&path) { Ok(ix) => { self.collapsed_channels.remove(ix); } Err(ix) => { - self.collapsed_channels.insert(ix, location); + self.collapsed_channels.insert(ix, path.into_owned()); } }; self.serialize(cx); @@ -2402,8 +2347,8 @@ impl CollabPanel { cx.focus_self(); } - fn is_channel_collapsed(&self, location: &ChannelLocation) -> bool { - self.collapsed_channels.binary_search(location).is_ok() + fn is_channel_collapsed(&self, path: &ChannelPath) -> bool { + self.collapsed_channels.binary_search(path).is_ok() } fn leave_call(cx: &mut ViewContext) { @@ -2472,10 +2417,10 @@ impl CollabPanel { } fn rename_selected_channel(&mut self, _: &menu::SecondaryConfirm, cx: &mut ViewContext) { - if let Some((channel, parent)) = self.selected_channel() { + if let Some((_, parent)) = self.selected_channel() { self.rename_channel( &RenameChannel { - location: (channel.id, parent.to_owned()).into(), + location: parent.to_owned(), }, cx, ); @@ -2484,11 +2429,11 @@ impl CollabPanel { fn rename_channel(&mut self, action: &RenameChannel, cx: &mut ViewContext) { let channel_store = self.channel_store.read(cx); - if !channel_store.is_user_admin(action.location.channel) { + if !channel_store.is_user_admin(action.location.channel_id()) { return; } if let Some(channel) = channel_store - .channel_for_id(action.location.channel) + .channel_for_id(action.location.channel_id()) .cloned() { self.channel_editing_state = Some(ChannelEditingState::Rename { @@ -2512,11 +2457,11 @@ impl CollabPanel { } fn show_inline_context_menu(&mut self, _: &menu::ShowContextMenu, cx: &mut ViewContext) { - let Some((channel, path)) = self.selected_channel() else { + let Some((_, path)) = self.selected_channel() else { return; }; - self.deploy_channel_context_menu(None, &(channel.id, path.to_owned()).into(), cx); + self.deploy_channel_context_menu(None, &path.to_owned(), cx); } fn selected_channel(&self) -> Option<(&Arc, &ChannelPath)> { @@ -2983,26 +2928,3 @@ fn render_icon_button(style: &IconButton, svg_path: &'static str) -> impl Elemen .contained() .with_style(style.container) } - -/// Hash a channel path to a u64, for use as a mouse id -/// Based on the Fowler–Noll–Vo hash: -/// https://en.wikipedia.org/wiki/Fowler%E2%80%93Noll%E2%80%93Vo_hash_function -fn id(path: &[ChannelId]) -> u64 { - // I probably should have done this, but I didn't - // let hasher = DefaultHasher::new(); - // let path = path.hash(&mut hasher); - // let x = hasher.finish(); - - const OFFSET: u64 = 14695981039346656037; - const PRIME: u64 = 1099511628211; - - let mut hash = OFFSET; - for id in path.iter() { - for id in id.to_ne_bytes() { - hash = hash ^ (id as u64); - hash = (hash as u128 * PRIME as u128) as u64; - } - } - - hash -}