search

rust-lang / rust-analyzer

A Rust compiler front-end for IDEs
https://rust-analyzer.github.io/
Apache License 2.0
14.08k stars 1.57k forks source link

`DidChangeTextDocument` notification is slow to process and may block #13538

Open Veykril opened 1 year ago

Veykril commented 1 year ago

We handle client to server notifications synchronously, which means we should make sure that these notifications are processed fast as otherwise the server will block on them. If a user opens a very large rust file (100k+ lines) they will start encountering overly long loop and the general editor experience becomes very laggy. We can't make the processing for this notification async unfortunately so we should try and optimize the things we do there instead.

Relevant code snippets:

https://github.com/rust-lang/rust-analyzer/blob/bbcb77ea6fc08df598b20267afd6a44018b21f5b/crates/rust-analyzer/src/main_loop.rs#L746-L768 https://github.com/rust-lang/rust-analyzer/blob/bbcb77ea6fc08df598b20267afd6a44018b21f5b/crates/rust-analyzer/src/lsp_utils.rs#L135-L183 https://github.com/rust-lang/rust-analyzer/blob/bbcb77ea6fc08df598b20267afd6a44018b21f5b/crates/ide-db/src/line_index.rs#L57-L94

The main part here is the LineIndex calculation as we have to traverse the entire document text character by character, so we should look into possibly optimizing the creation of it.

Veykril commented 8 months ago

cc @roife , I think you were looking into speeding this up? or was that something else

roife commented 8 months ago

I have tried placing the LineEndings (normalize) calculation within vfs-notify (which runs in a separate thread), but I encountered two problems:

  1. We need to carry LineEndings information in vfs::Change, which would make the code more complex.
  2. Both 'didOpen' and 'didChange' events require recalculating LineIndex, and they are sync_mut, so calculation of LineEndings still needs to be done synchronously in these events, not in apply_document_changes, but moved earlier to handle_did_open_text_document and handle_did_change_text_document. Therefore, it seems ineffective for scenarios involving frequent edits.

I have considered using SIMD to accelerate the search process for \r in normalize (similar to what was done in LineIndex). Perhaps this could be effective?

lnicola commented 8 months ago

I have considered using SIMD to accelerate the search process for \r in normalize (similar to what was done in LineIndex). Perhaps this could be effective?

We can probably use the memchr crate, it's already well-optimized.

roife commented 8 months ago

We can probably use the memchr crate

I just tried using memchr to calculate LineEndings on a large file (composed of several files from rust-analyzer itself).

It is worth noting that I use a brute-force algorithm in memchr, so it can be further optimized for increased speed through careful refinement.

Veykril commented 8 months ago

That looks very promising, optimizing seems enough to me. Offloading that to another thread is too complicated (and likely does not yield much in terms of benefits)

lnicola commented 8 months ago

Is that on ARM? Because I don't see a NEON implementation in memchr.

roife commented 8 months ago

Is that on ARM? Because I don't see a NEON implementation in memchr.

Yes, I'm working on Apple Silicon.

I don't see a NEON implementation in memchr.

It added support for neon just months ago🙈: https://github.com/BurntSushi/memchr/pull/129

lnicola commented 8 months ago

TIL #[cfg]-gated files don't show up in the sidebar on https://docs.rs/memchr/latest/src/memchr/lib.rs.html#1-221 :sweat_smile:.