Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

rust-analyzer: diagnostics can take a long time to be updated #23566

Open
1 task done
cartercanedy opened this issue Jan 23, 2025 · 5 comments · May be fixed by #23639
Open
1 task done

rust-analyzer: diagnostics can take a long time to be updated #23566

cartercanedy opened this issue Jan 23, 2025 · 5 comments · May be fixed by #23639
Labels
bug [core label] language server An umbrella label for all language servers language An umbrella label for all programming languages syntax behaviors performance Feedback for performance issues, speed, memory usage, etc rust Rust programming language support

Comments

@cartercanedy
Copy link

Check for existing issues

  • Completed

Describe the bug / provide steps to reproduce it

VSCode proactively queries RA for diagnostics in the active text document via a textDocument/diagnostic rpc rather than waiting for a textDocument/publishDiagnostic notification after a full workspace check, which can significantly reduce the latency between code edits and useful diagnostics in crates deep in the dependency graph.

I've attached the trace logs from vscode to show that the latency in the textDocument/diagnostic rpc request/response loop after a textDocument/didChange notification from the client, which shows a <1s response, where Zed can have a diagnostic latency of >10s on my hardware

vscode-ra-log.txt

Zed Version and System Specs

Zed: v0.171.0 (Zed Dev 8b8ca795f459f5b911b6d7dfcdb89cdca01f1781)
OS: Windows 10.0.19045
Memory: 63.8 GiB
Architecture: x86_64
GPU: NVIDIA GeForce RTX 3080 Ti Laptop GPU || NVIDIA || 538.92

If applicable, add screenshots or screencasts of the incorrect state / behavior

No response

If applicable, attach your Zed.log file to this issue.

No response
@JosephTLyons JosephTLyons added performance Feedback for performance issues, speed, memory usage, etc rust Rust programming language support language An umbrella label for all programming languages syntax behaviors language server An umbrella label for all language servers breaking-change and removed triage labels Jan 23, 2025
@cartercanedy
Copy link
Author

@JosephTLyons I don't think that implementing the rpc routines would be an end-user breaking change, right? It's should just improve the perceived responsiveness

@osiewicz
Copy link
Contributor

osiewicz commented Jan 23, 2025 •
edited
Loading

To be fair some of this (I think) is on RA side; when I've profiled my lovely 8k-diag use case (https://gist.github.com/osiewicz/89113b481504c719dc330ed02c51b4af) I've noticed that while building it (and emitting all the errors) takes ~3s whereas it takes RA 18s to emit all of them. I think what you're suggesting is sound and a good way to go about this tbh

@JosephTLyons
Copy link
Collaborator

@JosephTLyons I don't think that implementing the rpc routines would be an end-user breaking change, right? It's should just improve the perceived responsiveness

Sorry - the "breaking-change" label was unintentional.

@SomeoneToIgnore
Copy link
Contributor

See #19230

I think, some form of a breaking change may happen if no "related files" will be reported when pulling for diagnostics: now, r-a pushes all workspace-related diagnostics, including cargo check ones for all files possible.
With the pull approach, it's not really feasible to pull for each file in the workspace, so we might loose the ability to show project diagnostics properly.

@cartercanedy
Copy link
Author

cartercanedy commented Jan 24, 2025 •
edited
Loading

@SomeoneToIgnore from what I can tell, vscode still invokes textDocument/didChange to trigger workspace checks, but I believe that the textDocument/diagnostic rpc is made independent of the full workspace analysis.

I don't think that this should replace what's already implemented for triggering workspace diagnostic updates, but implementing this for the active document seems to be a way to shorten the feedback loop for the most relevant diagnostics, and any textDocument/publishDiagnostics notifications can be handled as normal, which would emulate the VSCode diagnostic querying strategy

@github-actions github-actions bot mentioned this issue Jan 24, 2025
Open
@cartercanedy cartercanedy linked a pull request Jan 25, 2025 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug [core label] language server An umbrella label for all language servers language An umbrella label for all programming languages syntax behaviors performance Feedback for performance issues, speed, memory usage, etc rust Rust programming language support
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

lsp: Add limited implementation of pull-based diagnostics cartercanedy/zed
4 participants
@SomeoneToIgnore @JosephTLyons @osiewicz @cartercanedy