Repository navigation
Map canonical LSP paths into symlinked external directories - #64895
Conversation
|
We require contributors to sign our Contributor License Agreement, and we don't have @taburet on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'. |
zed-community-bot
Bot
added
the
first contribution
|
@cla-bot check |
cla-bot
Bot
added
the
cla-signed
|
The cla-bot has been summoned, and re-checked this pull request! |
This is a bit worrying, as initial worktree scans can take seconds. On the other hand, I do think this PR is worth pursuing.. Ugh. |
| if worktree.is_some() || scans.is_empty() { | ||
| return Ok(worktree); | ||
| } | ||
| join_all(scans).await; |
There was a problem hiding this comment.
This waits for all worktrees to complete scanning, when we could eagerly start inspecting worktrees before all of them are done scanning.
| /// given by its canonical form, to its path within this worktree. | ||
| pub fn relative_path_for_external_abs_path(&self, abs_path: &Path) -> Option<RelPathBuf> { | ||
| abs_path.ancestors().find_map(|canonical| { | ||
| let relative = self.external_canonical_to_relative.get(canonical)?; |
There was a problem hiding this comment.
I realize this code was moved over, but I really don't love how we're iterating over all ancestors. external_canonical_to_relative is a BTreeMap, which means we can use it's range api to reduce lookups significantly.
There was a problem hiding this comment.
yeah, this also looked weird to me but I thought there is some reason behind it, will try to rework it
There was a problem hiding this comment.
I have tried to improve it but all my attempts perform about the same or worse
the main problem is that external_canonical_to_relative keys nest and are sorted lexicographically, you might end up at some nested key of a sibling directory, so you have to find the common prefix and retry, finding the prefix is not free
if you have some specific solution in mind, could you please outline it?
There was a problem hiding this comment.
Something akin to https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=bdee388586d86cc03859fd55d9602ad5 would be cool.
Click for bench results
Running path_prefix_bench.rs (target/release/deps/path_prefix_bench-869b12d7c98d96d2)
Gnuplot not found, using plotters backend
path prefix lookup/all ancestors/deep miss in every worktree
time: [495.58 µs 497.71 µs 500.33 µs]
path prefix lookup/narrowing range/deep miss in every worktree
time: [61.528 µs 61.769 µs 62.103 µs]
Found 2 outliers among 20 measurements (10.00%)
1 (5.00%) high mild
1 (5.00%) high severe
path prefix lookup/all ancestors/shallow miss in every worktree
time: [24.739 µs 24.877 µs 24.977 µs]
path prefix lookup/narrowing range/shallow miss in every worktree
time: [17.421 µs 17.559 µs 17.666 µs]
path prefix lookup/all ancestors/deep hit in last worktree
time: [492.45 µs 495.80 µs 498.60 µs]
path prefix lookup/narrowing range/deep hit in last worktree
time: [58.001 µs 58.261 µs 58.571 µs]
path prefix lookup/all ancestors/exact hit in last worktree
time: [23.701 µs 23.830 µs 23.976 µs]
path prefix lookup/narrowing range/exact hit in last worktree
time: [16.691 µs 16.780 µs 16.925 µs]
Found 1 outliers among 20 measurements (5.00%)
1 (5.00%) high severe
path prefix lookup/all ancestors/deep hit in first worktree
time: [15.888 µs 15.935 µs 15.989 µs]
Found 1 outliers among 20 measurements (5.00%)
1 (5.00%) high severe
path prefix lookup/narrowing range/deep hit in first worktree
time: [417.45 ns 421.79 ns 426.43 ns]
Found 2 outliers among 20 measurements (10.00%)
1 (5.00%) high mild
1 (5.00%) high severe
path prefix lookup/all ancestors/exact hit in first worktree
time: [349.38 ns 350.72 ns 352.22 ns]
path prefix lookup/narrowing range/exact hit in first worktree
time: [389.90 ns 391.23 ns 393.03 ns]
Found 2 outliers among 20 measurements (10.00%)
1 (5.00%) high mild
1 (5.00%) high severe
path prefix lookup/all ancestors/deep miss with crowded range
time: [559.45 µs 561.26 µs 563.94 µs]
Found 1 outliers among 20 measurements (5.00%)
1 (5.00%) high mild
path prefix lookup/narrowing range/deep miss with crowded range
time: [83.632 µs 84.790 µs 86.004 µs]
Found 1 outliers among 20 measurements (5.00%)
1 (5.00%) high severe
ChristopherBiscardi
added
the
area:language server
9ad05ef to
cc77487
Compare
Summary
Some language servers report files inside a symlinked directory by their canonical path. For example, with
/project/pkg -> /shared/pkg, the server returns/shared/pkg/other.rsinstead of/project/pkg/other.rs. Zed found no worktree for that path, so it opened the file in a new worktree.Now, if the path matches no worktree, Zed checks it against the symlinked directories that each worktree has scanned. It maps the canonical path back to its path in the worktree (
pkg/other.rs). Those mappings exist only after scanning, so if no match is found, the lookup waits for scanning worktrees to finish and tries once more.The mapping code already existed inside
BackgroundScanner. I moved it toLocalSnapshot::relative_path_for_external_abs_pathso both places use it.Testing
scan_symlinkssetting and forscan_symlinks: always.To reproduce manually: create a project with a symlink to a directory outside it, open a file that uses code from that directory, and run "go to definition" on a symbol defined there.
Self-review
UI changes follow the checklist.No UI changes.Release Notes: