Skip to content

Commit

Permalink
Get Editor & Project to share breakpoints through RWLock
Browse files Browse the repository at this point in the history 
Previously editor would send breakpoints to project whenever
a new breakpoint was deleted or added. Now editor shares a
shared pointer to a breakpoint datastructure. This behavior
is more efficient because it doesn't have to repeatly send
breakpoints everytime one is updated.

Still have to handle cases when a breakpoint is added/removed during
a debugging phase. I also have to figure out how to share the breakpoints
pointer only once per project and editor.
  • Loading branch information
@Anthony-Eid
Anthony-Eid committed Jul 27, 2024
1 parent f0a5775 commit 87f5323
Show file tree
Hide file tree
Showing 8 changed files with 88 additions and 55 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/dap/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ async-std = "1.12.0"
dap-types = { git = "https://github.com/zed-industries/dap-types" }
futures.workspace = true
gpui.workspace = true
multi_buffer.workspace = true
log.workspace = true
parking_lot.workspace = true
postage.workspace = true
Expand Down
5 changes: 5 additions & 0 deletions crates/dap/src/client.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -555,3 +555,8 @@ impl DebugAdapterClient {
.await
}
}

#[derive(Clone, Debug, Hash, PartialEq, Eq)]
pub struct Breakpoint {
pub position: multi_buffer::Anchor,
}
1 change: 1 addition & 0 deletions crates/editor/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ clock.workspace = true
collections.workspace = true
convert_case = "0.6.0"
db.workspace = true
dap.workspace = true
emojis.workspace = true
file_icons.workspace = true
futures.workspace = true
Expand Down
29 changes: 21 additions & 8 deletions crates/editor/src/editor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ use language::{point_to_lsp, BufferRow, Runnable, RunnableRange};
use linked_editing_ranges::refresh_linked_ranges;
use task::{ResolvedTask, TaskTemplate, TaskVariables};

use dap::client::Breakpoint;
use hover_links::{HoverLink, HoveredLinkState, InlayHighlight};
pub use lsp::CompletionContext;
use lsp::{
Expand Down Expand Up @@ -450,11 +451,6 @@ struct ResolvedTasks {
position: Anchor,
}

#[derive(Clone, Debug, Hash, PartialEq, Eq)]
struct Breakpoint {
position: Anchor,
}

#[derive(Copy, Clone, Debug)]
struct MultiBufferOffset(usize);
#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
Expand Down Expand Up @@ -574,7 +570,7 @@ pub struct Editor {
expect_bounds_change: Option<Bounds<Pixels>>,
tasks: BTreeMap<(BufferId, BufferRow), RunnableTasks>,
tasks_update_task: Option<Task<()>>,
breakpoints: BTreeMap<BufferId, HashSet<Breakpoint>>,
breakpoints: Arc<RwLock<BTreeMap<BufferId, HashSet<Breakpoint>>>>,
previous_search_ranges: Option<Arc<[Range<Anchor>]>>,
file_header_size: u8,
breadcrumb_header: Option<String>,
Expand Down Expand Up @@ -1788,6 +1784,15 @@ impl Editor {
None
};

let breakpoints: Arc<RwLock<BTreeMap<BufferId, HashSet<Breakpoint>>>> = Default::default();
// TODO: Figure out why the code below doesn't work
// if let Some(project) = project.as_ref() {
// dbg!("Setting breakpoints from editor to project");
// project.update(cx, |project, _cx| {
// project.breakpoints = breakpoints.clone();
// })
// }

let mut this = Self {
focus_handle,
show_cursor_when_unfocused: false,
Expand Down Expand Up @@ -1890,7 +1895,7 @@ impl Editor {
blame_subscription: None,
file_header_size,
tasks: Default::default(),
breakpoints: Default::default(),
breakpoints,
_subscriptions: vec![
cx.observe(&buffer, Self::on_buffer_changed),
cx.subscribe(&buffer, Self::on_buffer_event),
Expand Down Expand Up @@ -5994,6 +5999,12 @@ impl Editor {
let Some(project) = &self.project else {
return;
};

// TODO: Figure out how to only clone breakpoints pointer once per editor
project.update(cx, |project, _cx| {
project.breakpoints = self.breakpoints.clone();
});

let Some(buffer) = self.buffer.read(cx).as_singleton() else {
return;
};
Expand All @@ -6004,7 +6015,9 @@ impl Editor {
position: breakpoint_position,
};

let breakpoint_set = self.breakpoints.entry(buffer_id).or_default();
let mut write_guard = self.breakpoints.write();

let breakpoint_set = write_guard.entry(buffer_id).or_default();

if !breakpoint_set.remove(&breakpoint) {
breakpoint_set.insert(breakpoint);
Expand Down
1 change: 1 addition & 0 deletions crates/editor/src/element.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1568,6 +1568,7 @@ impl EditorElement {
self.editor.update(cx, |editor, cx| {
editor
.breakpoints
.read()
.iter()
.flat_map(|(_buffer_id, breakpoint_set)| breakpoint_set.iter())
.filter_map(|breakpoint| {
Expand Down
102 changes: 56 additions & 46 deletions crates/project/src/project.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use client::{
use clock::ReplicaId;
use collections::{btree_map, BTreeMap, BTreeSet, HashMap, HashSet, VecDeque};
use dap::{
client::{DebugAdapterClient, DebugAdapterClientId},
client::{Breakpoint, DebugAdapterClient, DebugAdapterClientId},
transport::Events,
SourceBreakpoint,
};
Expand Down Expand Up @@ -119,7 +119,7 @@ use task::{
HideStrategy, RevealStrategy, Shell, TaskContext, TaskTemplate, TaskVariables, VariableName,
};
use terminals::Terminals;
use text::{Anchor, BufferId, LineEnding};
use text::{Anchor, BufferId, LineEnding, Point};
use unicase::UniCase;
use util::{
debug_panic, defer, maybe, merge_json_value_into, parse_env_output, post_inc,
Expand Down Expand Up @@ -182,7 +182,7 @@ pub struct Project {
language_servers: HashMap<LanguageServerId, LanguageServerState>,
language_server_ids: HashMap<(WorktreeId, LanguageServerName), LanguageServerId>,
debug_adapters: HashMap<DebugAdapterClientId, DebugAdapterClientState>,
breakpoints: HashMap<BufferId, Vec<Breakpoint>>,
pub breakpoints: Arc<RwLock<BTreeMap<BufferId, HashSet<Breakpoint>>>>,
language_server_statuses: BTreeMap<LanguageServerId, LanguageServerStatus>,
last_formatting_failure: Option<String>,
last_workspace_edits_by_language_server: HashMap<LanguageServerId, ProjectTransaction>,
Expand Down Expand Up @@ -314,10 +314,6 @@ impl PartialEq for LanguageServerPromptRequest {
}
}

struct Breakpoint {
row: BufferRow,
}

#[derive(Clone, Debug, PartialEq)]
pub enum Event {
LanguageServerAdded(LanguageServerId),
Expand Down Expand Up @@ -1157,11 +1153,19 @@ impl Project {
let task = project.update(&mut cx, |project, cx| {
let mut tasks = Vec::new();

for (buffer_id, breakpoints) in project.breakpoints.iter() {
let res = maybe!({
for (buffer_id, breakpoints) in project.breakpoints.read().iter() {
let buffer = maybe!({
let buffer = project.buffer_for_id(*buffer_id, cx)?;
Some(buffer.read(cx))
});

if buffer.is_none() {
continue;
}
let buffer = buffer.as_ref().unwrap();

let project_path = buffer.read(cx).project_path(cx)?;
let res = maybe!({
let project_path = buffer.project_path(cx)?;
let worktree = project.worktree_for_id(project_path.worktree_id, cx)?;
let path = worktree.read(cx).absolutize(&project_path.path).ok()?;

Expand All @@ -1176,7 +1180,12 @@ impl Project {
breakpoints
.iter()
.map(|b| SourceBreakpoint {
line: b.row as u64,
line: buffer
.summary_for_anchor::<Point>(
&b.position.text_anchor,
)
.row
as u64,
condition: None,
hit_condition: None,
log_message: None,
Expand Down Expand Up @@ -1204,6 +1213,7 @@ impl Project {
debug_task: task::ResolvedTask,
cx: &mut ModelContext<Self>,
) {
dbg!(&self.breakpoints);
let id = DebugAdapterClientId(1);
let debug_template = debug_task.original_task();
let cwd = debug_template
Expand Down Expand Up @@ -1268,41 +1278,41 @@ impl Project {
row: BufferRow,
cx: &mut ModelContext<Self>,
) {
let breakpoints_for_buffer = self
.breakpoints
.entry(buffer.read(cx).remote_id())
.or_insert(Vec::new());

if let Some(ix) = breakpoints_for_buffer
.iter()
.position(|breakpoint| breakpoint.row == row)
{
breakpoints_for_buffer.remove(ix);
} else {
breakpoints_for_buffer.push(Breakpoint { row });
}

let clients = self
.debug_adapters
.iter()
.filter_map(|(_, state)| match state {
DebugAdapterClientState::Starting(_) => None,
DebugAdapterClientState::Running(client) => Some(client.clone()),
})
.collect::<Vec<_>>();

let mut tasks = Vec::new();
for client in clients {
tasks.push(self.send_breakpoints(client, cx));
}

cx.background_executor()
.spawn(async move {
try_join_all(tasks).await?;

anyhow::Ok(())
})
.detach_and_log_err(cx)
// let breakpoints_for_buffer = self
// .breakpoints
// .entry(buffer.read(cx).remote_id())
// .or_insert(Vec::new());

// if let Some(ix) = breakpoints_for_buffer
// .iter()
// .position(|breakpoint| breakpoint.row == row)
// {
// breakpoints_for_buffer.remove(ix);
// } else {
// breakpoints_for_buffer.push(Breakpoint { row });
// }

// let clients = self
// .debug_adapters
// .iter()
// .filter_map(|(_, state)| match state {
// DebugAdapterClientState::Starting(_) => None,
// DebugAdapterClientState::Running(client) => Some(client.clone()),
// })
// .collect::<Vec<_>>();

// let mut tasks = Vec::new();
// for client in clients {
// tasks.push(self.send_breakpoints(client, cx));
// }

// cx.background_executor()
// .spawn(async move {
// try_join_all(tasks).await?;

// anyhow::Ok(())
// })
// .detach_and_log_err(cx)
}

fn shutdown_language_servers(
Expand Down
2 changes: 1 addition & 1 deletion crates/text/src/text.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2155,7 +2155,7 @@ impl BufferSnapshot {
})
}

fn summary_for_anchor<D>(&self, anchor: &Anchor) -> D
pub fn summary_for_anchor<D>(&self, anchor: &Anchor) -> D
where
D: TextDimension,
{
Expand Down

0 comments on commit 87f5323

Please sign in to comment.