Make SanitizedPath wrap Path instead of Arc<Path> to avoid allocation (#37106)

Release Notes:

- N/A
This commit is contained in:
Michael Sloan
2025-08-28 13:32:30 -06:00
committed by GitHub
parent 69933d5b81
commit 47aaaa8bcf
14 changed files with 156 additions and 85 deletions
+31 -25
View File
@@ -158,7 +158,7 @@ pub struct RemoteWorktree {
#[derive(Clone)]
pub struct Snapshot {
id: WorktreeId,
abs_path: SanitizedPath,
abs_path: Arc<SanitizedPath>,
root_name: String,
root_char_bag: CharBag,
entries_by_path: SumTree<Entry>,
@@ -457,7 +457,7 @@ enum ScanState {
scanning: bool,
},
RootUpdated {
new_path: Option<SanitizedPath>,
new_path: Option<Arc<SanitizedPath>>,
},
}
@@ -763,8 +763,8 @@ impl Worktree {
pub fn abs_path(&self) -> Arc<Path> {
match self {
Worktree::Local(worktree) => worktree.abs_path.clone().into(),
Worktree::Remote(worktree) => worktree.abs_path.clone().into(),
Worktree::Local(worktree) => SanitizedPath::cast_arc(worktree.abs_path.clone()),
Worktree::Remote(worktree) => SanitizedPath::cast_arc(worktree.abs_path.clone()),
}
}
@@ -1813,7 +1813,7 @@ impl LocalWorktree {
// Otherwise, the FS watcher would do it on the `RootUpdated` event,
// but with a noticeable delay, so we handle it proactively.
local.update_abs_path_and_refresh(
Some(SanitizedPath::from(abs_path.clone())),
Some(SanitizedPath::new_arc(&abs_path)),
cx,
);
Task::ready(Ok(this.root_entry().cloned()))
@@ -2090,7 +2090,7 @@ impl LocalWorktree {
fn update_abs_path_and_refresh(
&mut self,
new_path: Option<SanitizedPath>,
new_path: Option<Arc<SanitizedPath>>,
cx: &Context<Worktree>,
) {
if let Some(new_path) = new_path {
@@ -2340,7 +2340,7 @@ impl Snapshot {
pub fn new(id: u64, root_name: String, abs_path: Arc<Path>) -> Self {
Snapshot {
id: WorktreeId::from_usize(id as usize),
abs_path: abs_path.into(),
abs_path: SanitizedPath::from_arc(abs_path),
root_char_bag: root_name.chars().map(|c| c.to_ascii_lowercase()).collect(),
root_name,
always_included_entries: Default::default(),
@@ -2368,7 +2368,7 @@ impl Snapshot {
//
// This is definitely a bug, but it's not clear if we should handle it here or not.
pub fn abs_path(&self) -> &Arc<Path> {
self.abs_path.as_path()
SanitizedPath::cast_arc_ref(&self.abs_path)
}
fn build_initial_update(&self, project_id: u64, worktree_id: u64) -> proto::UpdateWorktree {
@@ -2464,7 +2464,7 @@ impl Snapshot {
Some(removed_entry.path)
}
fn update_abs_path(&mut self, abs_path: SanitizedPath, root_name: String) {
fn update_abs_path(&mut self, abs_path: Arc<SanitizedPath>, root_name: String) {
self.abs_path = abs_path;
if root_name != self.root_name {
self.root_char_bag = root_name.chars().map(|c| c.to_ascii_lowercase()).collect();
@@ -2483,7 +2483,7 @@ impl Snapshot {
update.removed_entries.len()
);
self.update_abs_path(
SanitizedPath::from(PathBuf::from_proto(update.abs_path)),
SanitizedPath::new_arc(&PathBuf::from_proto(update.abs_path)),
update.root_name,
);
@@ -3849,7 +3849,11 @@ impl BackgroundScanner {
root_entry.is_ignored = true;
state.insert_entry(root_entry.clone(), self.fs.as_ref(), self.watcher.as_ref());
}
state.enqueue_scan_dir(root_abs_path.into(), &root_entry, &scan_job_tx);
state.enqueue_scan_dir(
SanitizedPath::cast_arc(root_abs_path),
&root_entry,
&scan_job_tx,
);
}
};
@@ -3930,8 +3934,9 @@ impl BackgroundScanner {
self.forcibly_load_paths(&request.relative_paths).await;
let root_path = self.state.lock().snapshot.abs_path.clone();
let root_canonical_path = match self.fs.canonicalize(root_path.as_path()).await {
Ok(path) => SanitizedPath::from(path),
let root_canonical_path = self.fs.canonicalize(root_path.as_path()).await;
let root_canonical_path = match &root_canonical_path {
Ok(path) => SanitizedPath::new(path),
Err(err) => {
log::error!("failed to canonicalize root path {root_path:?}: {err}");
return true;
@@ -3959,8 +3964,8 @@ impl BackgroundScanner {
}
self.reload_entries_for_paths(
root_path,
root_canonical_path,
&root_path,
&root_canonical_path,
&request.relative_paths,
abs_paths,
None,
@@ -3972,8 +3977,9 @@ impl BackgroundScanner {
async fn process_events(&self, mut abs_paths: Vec<PathBuf>) {
let root_path = self.state.lock().snapshot.abs_path.clone();
let root_canonical_path = match self.fs.canonicalize(root_path.as_path()).await {
Ok(path) => SanitizedPath::from(path),
let root_canonical_path = self.fs.canonicalize(root_path.as_path()).await;
let root_canonical_path = match &root_canonical_path {
Ok(path) => SanitizedPath::new(path),
Err(err) => {
let new_path = self
.state
@@ -3982,7 +3988,7 @@ impl BackgroundScanner {
.root_file_handle
.clone()
.and_then(|handle| handle.current_path(&self.fs).log_err())
.map(SanitizedPath::from)
.map(|path| SanitizedPath::new_arc(&path))
.filter(|new_path| *new_path != root_path);
if let Some(new_path) = new_path.as_ref() {
@@ -4011,7 +4017,7 @@ impl BackgroundScanner {
abs_paths.sort_unstable();
abs_paths.dedup_by(|a, b| a.starts_with(b));
abs_paths.retain(|abs_path| {
let abs_path = SanitizedPath::from(abs_path);
let abs_path = &SanitizedPath::new(abs_path);
let snapshot = &self.state.lock().snapshot;
{
@@ -4054,7 +4060,7 @@ impl BackgroundScanner {
return false;
};
if abs_path.0.file_name() == Some(*GITIGNORE) {
if abs_path.file_name() == Some(*GITIGNORE) {
for (_, repo) in snapshot.git_repositories.iter().filter(|(_, repo)| repo.directory_contains(&relative_path)) {
if !dot_git_abs_paths.iter().any(|dot_git_abs_path| dot_git_abs_path == repo.common_dir_abs_path.as_ref()) {
dot_git_abs_paths.push(repo.common_dir_abs_path.to_path_buf());
@@ -4093,8 +4099,8 @@ impl BackgroundScanner {
let (scan_job_tx, scan_job_rx) = channel::unbounded();
log::debug!("received fs events {:?}", relative_paths);
self.reload_entries_for_paths(
root_path,
root_canonical_path,
&root_path,
&root_canonical_path,
&relative_paths,
abs_paths,
Some(scan_job_tx.clone()),
@@ -4441,8 +4447,8 @@ impl BackgroundScanner {
/// All list arguments should be sorted before calling this function
async fn reload_entries_for_paths(
&self,
root_abs_path: SanitizedPath,
root_canonical_path: SanitizedPath,
root_abs_path: &SanitizedPath,
root_canonical_path: &SanitizedPath,
relative_paths: &[Arc<Path>],
abs_paths: Vec<PathBuf>,
scan_queue_tx: Option<Sender<ScanJob>>,
@@ -4470,7 +4476,7 @@ impl BackgroundScanner {
}
}
anyhow::Ok(Some((metadata, SanitizedPath::from(canonical_path))))
anyhow::Ok(Some((metadata, SanitizedPath::new_arc(&canonical_path))))
} else {
Ok(None)
}