Perf regression: r-a can't keep up with editing

[nightkr]

Moving #6362 (comment) to a new issue, since it seems to be an unrelated regression.

As of somewhere between 43253c5...cb78c40 Rust-Analyzer became nearly unusable on my laptop (Ryzen 3750H, emacs + lsp-mode, Linux). This manifests as auto-completion takes seconds to show up after I start typing, and when I'm "done" with an edit it slowly chugs through my changed characters one at a time while re-highlighting errors at about a second per character.

Weirdly enough my desktop doesn't seem to show that behaviour at all, and is basically perfectly fine (Ryzen 3900X, but otherwise similar).

It seems to happen for https://github.com/clux/kube-rs and to a lesser degree https://github.com/hyperium/hyper, but an empty dummy project created by cargo new works fine.

I'll try to do some more bisecting to see if I can find the cause...

[nightkr]

Huh, now cb78c40 seems to be on the fine side too. The bisecting continues, I guess.

[kjeremy]

Do you have https://rust-analyzer.github.io/manual.html#on-typing-assists turned on?

…

On Fri, Oct 30, 2020, 9:16 AM Teo Klestrup Röijezon < ***@***.***> wrote: Huh, now cb78c40 <cb78c40> seems to be on the fine side too. The bisecting continues, I guess. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#6413 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABBACRFOD3XEVLLDCYCQ7VTSNK4D7ANCNFSM4TE523OA> .

[nightkr]

@kjeremy Not that I'm aware of. My R-A LSP settings are the following:

  (setq
   lsp-rust-analyzer-cargo-load-out-dirs-from-check t
   lsp-rust-analyzer-proc-macro-enable t
   lsp-rust-analyzer-cargo-watch-command "clippy"
   rust-rustfmt-switches '("+nightly" "--edition" "2018"))

From what I can tell, editor.formatOnType isn't even registered as a setting that lsp-mode knows about.

lsp-java does apparently have a similar setting that is on by default (lsp-java-format-on-type-enabled is t), but that seems to map to the LSP setting "java.format.onType.enabled" (which I wouldn't expect R-A to care about.. :P).

[nightkr]

Actually, it looks like this format-on-type is built into lsp-mode, toggled by lsp-toggle-on-type-formatting and defaulting to t. Changing it doesn't seem to have any effect so far, but I'll give it some more testing..

[kjeremy]

The other thing would be to disable the onenter setting or whatever's it's called.

…

On Fri, Oct 30, 2020, 9:54 AM Teo Klestrup Röijezon < ***@***.***> wrote: Actually, it looks like this format-on-type is built into lsp-mode, toggled by lsp-toggle-on-type-formatting and defaulting to t. Changing it doesn't seem to have any effect so far, but I'll give it some more testing.. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6413 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABBACRH46SP4RSR76STPIXTSNLAQPANCNFSM4TE523OA> .

[flodiebold]

Which Emacs version are you using; are you using native JSON support? (See https://emacs-lsp.github.io/lsp-mode/page/performance/).

[nightkr]

Okay, bisect seems to have narrowed it down to cde7392

@kjeremy

The other thing would be to disable the onenter setting or whatever's it's called.

I guess you're talking about https://rust-analyzer.github.io/manual.html#on-enter? If so..

teo@interops-teo ~/.emacs.d (develop)> rg -uuu rust-analyzer.onEnter
teo@interops-teo ~/.emacs.d (develop) [1]> rg -uuu onEnter
Binary file .cache/savehist matches (found "\u{0}" byte around offset 80216)

Binary file workspace/.metadata/.plugins/org.eclipse.jdt.core/2824280114.index matches (found "\u{0}" byte around offset 0)

Binary file workspace/.metadata/.plugins/org.eclipse.jdt.core/2253745042.index matches (found "\u{0}" byte around offset 0)

Binary file phpactor/vendor/phpactor/code-builder/.git/objects/pack/pack-7fa51d87f69cdabf7ec8eaf03e2367e8549570a6.pack matches (found "\u{0}" byte around offset 4)

Doesn't look like lsp-mode uses that at all.

@flodiebold

Which Emacs version are you using; are you using native JSON support? (See https://emacs-lsp.github.io/lsp-mode/page/performance/).

The laptop is using Ubuntu 20.04's 26.3, without native JSON (according to lsp-doctor). The desktop is using Arch's 27.1, with native JSON. Considering the commit and the nature of the sluggishness (Emacs is still fairly responsive..) it seems unlikely that this is a JSON problem, but I'll give 27 a shot just in case.

[nightkr]

Well, Emacs 27 did improve things massively after all (thanks, @flodiebold!), but there is still a noticeable difference between cde7392 and 518f6d7.

[flodiebold]

Yeah I think I've noticed that as well. @jonas-schievink are we priming caches and sending progress about that on every update? Probably Emacs is a bit less smooth about displaying that.

[jonas-schievink]

uuh yes we are :)

I was expecting this to blow up. On every key stroke, we create the progress indicator, update it hundreds of times, and then delete it again. I was surprised that this still performs well in VS Code.

I'm not totally sure how to fix this. Maybe with some debouncing in another thread?

[nightkr]

Yeah.. applying

diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs
index fb18f9014..e17f9c02c 100644
--- a/crates/rust-analyzer/src/main_loop.rs
+++ b/crates/rust-analyzer/src/main_loop.rs
@@ -218,7 +218,9 @@ impl GlobalState {
                         }
                     };
 
-                    self.report_progress("indexing", state, message, Some(fraction));
+                    if state != Progress::Report {
+                        self.report_progress("indexing", state, message, Some(fraction));
+                    }
                 }
             },
             Event::Vfs(mut task) => {

to cde7392 makes it perform as well as 518f6d7 (under Emacs 26.3). Obviously that isn't a true solution in itself, but I guess it indicates that debouncing should help.

[flodiebold]

Could we only do the progress reporting when it's taking a bit longer? I.e. just record start time and check that. I admit I'm not really up to date on how the cache priming works.

[jonas-schievink]

Yeah, if we do it in another thread. I guess we want to parallelize it anyways, so that should work out.

[flodiebold]

Why only if we do it in another thread? 🤔

[jonas-schievink]

I guess we don't strictly need another thread, but the CrateDefMap computation even for just libcore can block for around 1 second, and we wouldn't give any feedback for that if we only used the single thread to check how long we've spent on it. I'd like to give immediate user feedback for that.

[jonas-schievink]

I guess we don't strictly need another thread, but the CrateDefMap computation even for just libcore can block for around 1 second, and we wouldn't give any feedback for that if we only used the single thread to check how long we've spent on it. I'd like to give immediate user feedback for that.

I think we can get away with coalescing updates in the main loop, like we do for VFS events. The "prime caches" operation already runs in the task pool.

[jonas-schievink]

I've opened #6441, which should hopefully make performance return to normal.

Please file any other performance problems as separate issues.