-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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.
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
internal: Resolve inlay hint data lazily #15522
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does reduce the amount of data we sent over the lsp now which is great! But this does not close the linked issue as we still generate all the tooltips on the initial request on the server (while we don't have to). I'm fine with merging this without that being resolved though for the time being.
☔ The latest upstream changes (presumably #15548) made this pull request unmergeable. Please resolve the merge conflicts. |
Skip every propery set in inlay hint client resolve capabilities, reducing overall json footprint.
479bbd8
to
caf0185
Compare
28cd9de
to
7b3dba5
Compare
Omit sending inlay hint resolve data if inlay has no properties that client resolve capabilities support.
Alright, let's see how this goes |
Thank you for all your time taken for this series' review and discussions. |
Thank you for tackling (parts of) that issue, inlay hints are (I think, never really checked) way too slow right now and certainly too expensive |
☀️ Test successful - checks-actions |
I think location links for param inlay hints don't work after this PR (they do work for type inlay hints). Maybe another vscode bug? |
Definitely not VSCode to blame this time, I have created a PR to fix this: #16089 |
…lution, r=Veykril Query for nearest parent block around the hint to resolve This way, parameter hints will be found for resolution and #15522 (comment) will be fixed Hopefully that also helps with whatever else (lifetimes', etc.) hints in #13962 > hints are resolved by querying for textDocument/inlayHint with hint's position turned into a [position-1, position+1] range (instead of the original, much wider document range). > > This might lead to issues in the future, with e.g. lifetime hints (currently there's nothing to resolve for them and it's fine) that belong to a certain position, but need to have textDocument/inlayHint query for much bigger range than their position+/-1
internal: Send less data during `textDocument/completion` if possible Similar to #15522, stops sending extra data during `textDocument/completion` if that data was set in the client completions resolve capabilities, and sends those only during `completionItem/resolve` requests. Currently, rust-analyzer sends back all fields (including potentially huge docs) for every completion item which might get large. Same as the other one, this PR aims to keep the changes minimal and does not remove extra computations for such fields — instead, it just filters them out before sending to the client. The PR omits primitive, boolean and integer, types such as `deprecated`, `preselect`, `insertTextFormat`, `insertTextMode`, etc. AND `additionalTextEdits` — this one looks very dangerous to compute for each completion item (as the spec says we ought to if there's no corresponding resolve capabilities provided) due to the diff computations and the fact that this code had been in the resolution for some time. It would be good to resolve this lazily too, please let me know if it's ok to do. When tested with Zed which only defines `documentation` and `additionalTextEdits` in its client completion resolve capabilities, rust-analyzer starts to send almost 3 times less characters: Request: ```json {"jsonrpc":"2.0","id":104,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:///Users/someonetoignore/work/rust-analyzer/crates/ide/src/inlay_hints.rs"},"position":{"line":90,"character":14},"context":{"triggerKind":1}}} ``` <img width="1338" alt="image" src="https://github.com/user-attachments/assets/104f19b5-7095-4fc1-b008-5d829623b2e2"> Before: 381944 characters [before.json](https://github.com/user-attachments/files/17092385/before.json) After: 140503 characters [after.json](https://github.com/user-attachments/files/17092386/after.json) After Zed's [patch](zed-industries/zed#18212) to enable all resolving possible: 84452 characters [after-after.json](https://github.com/user-attachments/files/17092755/after-after.json)
…ykril internal: Send less data during `textDocument/completion` if possible Similar to rust-lang/rust-analyzer#15522, stops sending extra data during `textDocument/completion` if that data was set in the client completions resolve capabilities, and sends those only during `completionItem/resolve` requests. Currently, rust-analyzer sends back all fields (including potentially huge docs) for every completion item which might get large. Same as the other one, this PR aims to keep the changes minimal and does not remove extra computations for such fields — instead, it just filters them out before sending to the client. The PR omits primitive, boolean and integer, types such as `deprecated`, `preselect`, `insertTextFormat`, `insertTextMode`, etc. AND `additionalTextEdits` — this one looks very dangerous to compute for each completion item (as the spec says we ought to if there's no corresponding resolve capabilities provided) due to the diff computations and the fact that this code had been in the resolution for some time. It would be good to resolve this lazily too, please let me know if it's ok to do. When tested with Zed which only defines `documentation` and `additionalTextEdits` in its client completion resolve capabilities, rust-analyzer starts to send almost 3 times less characters: Request: ```json {"jsonrpc":"2.0","id":104,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:///Users/someonetoignore/work/rust-analyzer/crates/ide/src/inlay_hints.rs"},"position":{"line":90,"character":14},"context":{"triggerKind":1}}} ``` <img width="1338" alt="image" src="https://github.com/user-attachments/assets/104f19b5-7095-4fc1-b008-5d829623b2e2"> Before: 381944 characters [before.json](https://github.com/user-attachments/files/17092385/before.json) After: 140503 characters [after.json](https://github.com/user-attachments/files/17092386/after.json) After Zed's [patch](zed-industries/zed#18212) to enable all resolving possible: 84452 characters [after-after.json](https://github.com/user-attachments/files/17092755/after-after.json)
internal: Send less data during `textDocument/completion` if possible Similar to rust-lang/rust-analyzer#15522, stops sending extra data during `textDocument/completion` if that data was set in the client completions resolve capabilities, and sends those only during `completionItem/resolve` requests. Currently, rust-analyzer sends back all fields (including potentially huge docs) for every completion item which might get large. Same as the other one, this PR aims to keep the changes minimal and does not remove extra computations for such fields — instead, it just filters them out before sending to the client. The PR omits primitive, boolean and integer, types such as `deprecated`, `preselect`, `insertTextFormat`, `insertTextMode`, etc. AND `additionalTextEdits` — this one looks very dangerous to compute for each completion item (as the spec says we ought to if there's no corresponding resolve capabilities provided) due to the diff computations and the fact that this code had been in the resolution for some time. It would be good to resolve this lazily too, please let me know if it's ok to do. When tested with Zed which only defines `documentation` and `additionalTextEdits` in its client completion resolve capabilities, rust-analyzer starts to send almost 3 times less characters: Request: ```json {"jsonrpc":"2.0","id":104,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:///Users/someonetoignore/work/rust-analyzer/crates/ide/src/inlay_hints.rs"},"position":{"line":90,"character":14},"context":{"triggerKind":1}}} ``` <img width="1338" alt="image" src="https://github.com/user-attachments/assets/104f19b5-7095-4fc1-b008-5d829623b2e2"> Before: 381944 characters [before.json](https://github.com/user-attachments/files/17092385/before.json) After: 140503 characters [after.json](https://github.com/user-attachments/files/17092386/after.json) After Zed's [patch](zed-industries/zed#18212) to enable all resolving possible: 84452 characters [after-after.json](https://github.com/user-attachments/files/17092755/after-after.json)
Part of #13962
Support https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#inlayHint_resolve better, by omitting all inlay hint fields specified in the client hint resolve capabilities.
Current list of all capabilities possible to resolve later:
and every one specified in the client capabilities is now resolved by r-a, being omitted in the initial response.
When editing
inlay_hints.rs
file around line457
with no resolve capabilities, I getresolved json, 10803 characters
for the visible editor range alone, pretty much repeated on every consequent edit.
With this patch and all inlay hint resolve capabilities enabled, for the same example I observe quite a footprint reduction:
unresolved json, 4142 characters
with all unresolved parts needing only for navigation, hover or applying the hint edit — dynamic parts that are made after mouse hover or similar events, that resolve the hint data.