Autocomplete performance benchmarks #6633
Labels
A-completion
autocompletion
A-perf
performance issues
S-unactionable
Issue requires feedback, design decisions or is blocked on other work
Comments
|
Proof of concept sounds nice, and I don't think PRs are ever unwelcomed here. I don't know much about the topic, but here's a CLI bench argument logic: that looks very related. |
|
@georgewfraser yeah, as far as micro benches go, the
But, in practice, I think monitoring things as you go would do the trick. I suggest just setting export RA_PROFILE ='*>150'in In this particular case (trying to complete stuff in 5333ms - handle_completion
5289ms - fuzzy_completion
365ms - render_resolution
298ms - item::Builder::build
296ms - rewrite
296ms - rewrite_children
296ms - rewrite_self (184 calls)
0ms - with_children (1 calls)
1ms - diff (1 calls)
0ms - insert_use (1 calls)
0ms - into_text_edit (1 calls)
0ms - mod_path_to_ast (1 calls)
0ms - rewrite_root (1 calls)Looks like rendering a completion item takes a long time, and it really shouldn't. cc @Veykril, you might be curious to look into this. |
😄 |
|
Let's not forget this one here as well 😄 https://github.com/rust-analyzer/rust-analyzer/blob/0993f9067cfc14cded484906283d1df8e8741e8e/crates/syntax/src/algo.rs#L185 I can take a stab at replacing the diffing algo in the (hopefully) near future I think, given the current one is really just not great and partially hacked in such a way that imports don't cause the cursor to jump(which I believe is a big factor to the bad runtime for it due to the lookahead we are doing in there currently). I would imagine that we probably want to go with the same algo/approach that roslyn uses which is according to the comment here TreeComparer.cs#L11-L13: At least from the bit I've seen this algorithm looks promising. |
|
Oh wait I just noticed the |
|
I'm not sure what to test this on, but I'd add a |
I was typing |
|
I have a branch with the working resolve completions: https://github.com/SomeoneToIgnore/rust-analyzer/tree/resolve-completions Adding the following tracenote that with this, This isn't the final version of the feature, but the fuzzy completion and resolve completion code will most probably stay the same for the first iteration, so we might want to try and optimise this case. Yet 5333ms is something dreadful, I've never seen my readings spiking over 300ms. |
Same applies for me, never saw it go above 300ms either. So SyntaxRewriter seems to always traverse all the descendants of the root node until it hits either a replacement or a token, so even if we were to only replace a single node in the tree we would still visit all the descendants of all its siblings which sounds very wasteful. This also means we kind of always do a deep clone of the tree(minus the replacements/deletions) due to how |
|
I've just tried to re-reproduce on the latest master, it reproduced easily:
|
This was referenced Dec 3, 2020
bors bot
added a commit
that referenced
this issue
Dec 8, 2020
6706: Move import text edit calculation into a completion resolve request r=matklad a=SomeoneToIgnore Part of #6612 (presumably fixing it) Part of #6366 (does not cover all possible resolve capabilities we can do) Closes #6594 Further improves imports on completion performance by deferring the computations for import inserts. To use the new mode, you have to have the experimental completions enabled and use the LSP 3.16-compliant client that reports `additionalTextEdits` in its `CompletionItemCapabilityResolveSupport` field in the client capabilities. rust-analyzer VSCode extension does this already hence picks up the changes completely. Performance implications are descrbed in: #6633 (comment) Co-authored-by: Kirill Bulatov <mail4score@gmail.com>
Found it! still slow after this change, but already much faster. |
lnicola
added
S-unactionable
bors bot
added a commit
that referenced
this issue
Apr 2, 2021
8302: Allow multiple modules from the same crate in fixtures r=SomeoneToIgnore a=SomeoneToIgnore A tiny step towards #6633 As discussed in [Zulip](https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Fwg-rls-2.2E0/topic/Completion.20benchmarks), we would need to have a generated code for the completions benchmark. To better represent a real project, we'd better allow to specify multiple modules in a crate within a single fixture. Co-authored-by: Kirill Bulatov <mail4score@gmail.com>
|
We now have an actually usable bench here: it works fine for debugging and and investigation. We don’t use it to capture regressions. I wish we had infra for perf regression tests, but I don’t know an easy way to do it reliably. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I recently experienced a pretty severe performance regression on autocomplete #6612
I'm interested in implementing some performance benchmarks so we can detect these sorts of regressions. Would such a PR be welcome? Would you like to offer any guidance on how this should be implemented, or should I just put together a proof of concept and we can discuss from there?
The text was updated successfully, but these errors were encountered: