Auto merge of rust-lang#13547 - Veykril:line-index, r=Veykril

internal: Optimize `apply_document_changes` a bit

cc rust-lang/rust-analyzer#13538

Author: bors

crates/ide-db/src/line_index.rs

@@ -58,8 +58,11 @@ impl LineIndex {
         let mut utf16_lines = NoHashHashMap::default();
         let mut utf16_chars = Vec::new();
 
-        let mut newlines = vec![0.into()];
-        let mut curr_row @ mut curr_col = 0.into();
+        let mut newlines = Vec::with_capacity(16);
+        newlines.push(TextSize::from(0));
+
+        let mut curr_row = 0.into();
+        let mut curr_col = 0.into();
         let mut line = 0;
         for c in text.chars() {
             let c_len = TextSize::of(c);

crates/rust-analyzer/src/line_index.rs

@@ -27,10 +27,6 @@ pub(crate) enum LineEndings {
 impl LineEndings {
     /// Replaces `\r\n` with `\n` in-place in `src`.
     pub(crate) fn normalize(src: String) -> (String, LineEndings) {
-        if !src.as_bytes().contains(&b'\r') {
-            return (src, LineEndings::Unix);
-        }
-
         // We replace `\r\n` with `\n` in-place, which doesn't break utf-8 encoding.
         // While we *can* call `as_mut_vec` and do surgery on the live string
         // directly, let's rather steal the contents of `src`. This makes the code
@@ -39,10 +35,19 @@ impl LineEndings {
         let mut buf = src.into_bytes();
         let mut gap_len = 0;
         let mut tail = buf.as_mut_slice();
+        let mut crlf_seen = false;
+
+        let find_crlf = |src: &[u8]| src.windows(2).position(|it| it == b"\r\n");
+
         loop {
             let idx = match find_crlf(&tail[gap_len..]) {
-                None => tail.len(),
-                Some(idx) => idx + gap_len,
+                None if crlf_seen => tail.len(),
+                // SAFETY: buf is unchanged and therefor still contains utf8 data
+                None => return (unsafe { String::from_utf8_unchecked(buf) }, LineEndings::Unix),
+                Some(idx) => {
+                    crlf_seen = true;
+                    idx + gap_len
+                }
             };
             tail.copy_within(gap_len..idx, 0);
             tail = &mut tail[idx - gap_len..];
@@ -54,15 +59,48 @@ impl LineEndings {
 
         // Account for removed `\r`.
         // After `set_len`, `buf` is guaranteed to contain utf-8 again.
-        let new_len = buf.len() - gap_len;
         let src = unsafe {
+            let new_len = buf.len() - gap_len;
             buf.set_len(new_len);
             String::from_utf8_unchecked(buf)
         };
-        return (src, LineEndings::Dos);
+        (src, LineEndings::Dos)
+    }
+}
 
-        fn find_crlf(src: &[u8]) -> Option<usize> {
-            src.windows(2).position(|it| it == b"\r\n")
-        }
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn unix() {
+        let src = "a\nb\nc\n\n\n\n";
+        let (res, endings) = LineEndings::normalize(src.into());
+        assert_eq!(endings, LineEndings::Unix);
+        assert_eq!(res, src);
+    }
+
+    #[test]
+    fn dos() {
+        let src = "\r\na\r\n\r\nb\r\nc\r\n\r\n\r\n\r\n";
+        let (res, endings) = LineEndings::normalize(src.into());
+        assert_eq!(endings, LineEndings::Dos);
+        assert_eq!(res, "\na\n\nb\nc\n\n\n\n");
+    }
+
+    #[test]
+    fn mixed() {
+        let src = "a\r\nb\r\nc\r\n\n\r\n\n";
+        let (res, endings) = LineEndings::normalize(src.into());
+        assert_eq!(endings, LineEndings::Dos);
+        assert_eq!(res, "a\nb\nc\n\n\n\n");
+    }
+
+    #[test]
+    fn none() {
+        let src = "abc";
+        let (res, endings) = LineEndings::normalize(src.into());
+        assert_eq!(endings, LineEndings::Unix);
+        assert_eq!(res, src);
     }
 }

crates/rust-analyzer/src/lsp_utils.rs

@@ -1,5 +1,5 @@
 //! Utilities for LSP-related boilerplate code.
-use std::{ops::Range, sync::Arc};
+use std::{mem, ops::Range, sync::Arc};
 
 use lsp_server::Notification;
 
@@ -133,11 +133,37 @@ impl GlobalState {
 }
 
 pub(crate) fn apply_document_changes(
-    old_text: &mut String,
-    content_changes: Vec<lsp_types::TextDocumentContentChangeEvent>,
-) {
+    file_contents: impl FnOnce() -> String,
+    mut content_changes: Vec<lsp_types::TextDocumentContentChangeEvent>,
+) -> String {
+    // Skip to the last full document change, as it invalidates all previous changes anyways.
+    let mut start = content_changes
+        .iter()
+        .rev()
+        .position(|change| change.range.is_none())
+        .map(|idx| content_changes.len() - idx - 1)
+        .unwrap_or(0);
+
+    let mut text: String = match content_changes.get_mut(start) {
+        // peek at the first content change as an optimization
+        Some(lsp_types::TextDocumentContentChangeEvent { range: None, text, .. }) => {
+            let text = mem::take(text);
+            start += 1;
+
+            // The only change is a full document update
+            if start == content_changes.len() {
+                return text;
+            }
+            text
+        }
+        Some(_) => file_contents(),
+        // we received no content changes
+        None => return file_contents(),
+    };
+
     let mut line_index = LineIndex {
-        index: Arc::new(ide::LineIndex::new(old_text)),
+        // the index will be overwritten in the bottom loop's first iteration
+        index: Arc::new(ide::LineIndex::new(&text)),
         // We don't care about line endings or offset encoding here.
         endings: LineEndings::Unix,
         encoding: PositionEncoding::Utf16,
@@ -148,38 +174,20 @@ pub(crate) fn apply_document_changes(
     // Some clients (e.g. Code) sort the ranges in reverse. As an optimization, we
     // remember the last valid line in the index and only rebuild it if needed.
     // The VFS will normalize the end of lines to `\n`.
-    enum IndexValid {
-        All,
-        UpToLineExclusive(u32),
-    }
-
-    impl IndexValid {
-        fn covers(&self, line: u32) -> bool {
-            match *self {
-                IndexValid::UpToLineExclusive(to) => to > line,
-                _ => true,
-            }
-        }
-    }
-
-    let mut index_valid = IndexValid::All;
+    let mut index_valid = !0u32;
     for change in content_changes {
-        match change.range {
-            Some(range) => {
-                if !index_valid.covers(range.end.line) {
-                    line_index.index = Arc::new(ide::LineIndex::new(old_text));
-                }
-                index_valid = IndexValid::UpToLineExclusive(range.start.line);
-                if let Ok(range) = from_proto::text_range(&line_index, range) {
-                    old_text.replace_range(Range::<usize>::from(range), &change.text);
-                }
+        // The None case can't happen as we have handled it above already
+        if let Some(range) = change.range {
+            if index_valid <= range.end.line {
+                *Arc::make_mut(&mut line_index.index) = ide::LineIndex::new(&text);
             }
-            None => {
-                *old_text = change.text;
-                index_valid = IndexValid::UpToLineExclusive(0);
+            index_valid = range.start.line;
+            if let Ok(range) = from_proto::text_range(&line_index, range) {
+                text.replace_range(Range::<usize>::from(range), &change.text);
             }
         }
     }
+    text
 }
 
 /// Checks that the edits inside the completion and the additional edits do not overlap.
@@ -242,51 +250,50 @@ mod tests {
             };
         }
 
-        let mut text = String::new();
-        apply_document_changes(&mut text, vec![]);
+        let text = apply_document_changes(|| String::new(), vec![]);
         assert_eq!(text, "");
-        apply_document_changes(
-            &mut text,
+        let text = apply_document_changes(
+            || text,
             vec![TextDocumentContentChangeEvent {
                 range: None,
                 range_length: None,
                 text: String::from("the"),
             }],
         );
         assert_eq!(text, "the");
-        apply_document_changes(&mut text, c![0, 3; 0, 3 => " quick"]);
+        let text = apply_document_changes(|| text, c![0, 3; 0, 3 => " quick"]);
         assert_eq!(text, "the quick");
-        apply_document_changes(&mut text, c![0, 0; 0, 4 => "", 0, 5; 0, 5 => " foxes"]);
+        let text = apply_document_changes(|| text, c![0, 0; 0, 4 => "", 0, 5; 0, 5 => " foxes"]);
         assert_eq!(text, "quick foxes");
-        apply_document_changes(&mut text, c![0, 11; 0, 11 => "\ndream"]);
+        let text = apply_document_changes(|| text, c![0, 11; 0, 11 => "\ndream"]);
         assert_eq!(text, "quick foxes\ndream");
-        apply_document_changes(&mut text, c![1, 0; 1, 0 => "have "]);
+        let text = apply_document_changes(|| text, c![1, 0; 1, 0 => "have "]);
         assert_eq!(text, "quick foxes\nhave dream");
-        apply_document_changes(
-            &mut text,
+        let text = apply_document_changes(
+            || text,
             c![0, 0; 0, 0 => "the ", 1, 4; 1, 4 => " quiet", 1, 16; 1, 16 => "s\n"],
         );
         assert_eq!(text, "the quick foxes\nhave quiet dreams\n");
-        apply_document_changes(&mut text, c![0, 15; 0, 15 => "\n", 2, 17; 2, 17 => "\n"]);
+        let text = apply_document_changes(|| text, c![0, 15; 0, 15 => "\n", 2, 17; 2, 17 => "\n"]);
         assert_eq!(text, "the quick foxes\n\nhave quiet dreams\n\n");
-        apply_document_changes(
-            &mut text,
+        let text = apply_document_changes(
+            || text,
             c![1, 0; 1, 0 => "DREAM", 2, 0; 2, 0 => "they ", 3, 0; 3, 0 => "DON'T THEY?"],
         );
         assert_eq!(text, "the quick foxes\nDREAM\nthey have quiet dreams\nDON'T THEY?\n");
-        apply_document_changes(&mut text, c![0, 10; 1, 5 => "", 2, 0; 2, 12 => ""]);
+        let text = apply_document_changes(|| text, c![0, 10; 1, 5 => "", 2, 0; 2, 12 => ""]);
         assert_eq!(text, "the quick \nthey have quiet dreams\n");
 
-        text = String::from("❤️");
-        apply_document_changes(&mut text, c![0, 0; 0, 0 => "a"]);
+        let text = String::from("❤️");
+        let text = apply_document_changes(|| text, c![0, 0; 0, 0 => "a"]);
         assert_eq!(text, "a❤️");
 
-        text = String::from("a\nb");
-        apply_document_changes(&mut text, c![0, 1; 1, 0 => "\nțc", 0, 1; 1, 1 => "d"]);
+        let text = String::from("a\nb");
+        let text = apply_document_changes(|| text, c![0, 1; 1, 0 => "\nțc", 0, 1; 1, 1 => "d"]);
         assert_eq!(text, "adcb");
 
-        text = String::from("a\nb");
-        apply_document_changes(&mut text, c![0, 1; 1, 0 => "ț\nc", 0, 2; 0, 2 => "c"]);
+        let text = String::from("a\nb");
+        let text = apply_document_changes(|| text, c![0, 1; 1, 0 => "ț\nc", 0, 2; 0, 2 => "c"]);
         assert_eq!(text, "ațc\ncb");
     }
 

crates/rust-analyzer/src/main_loop.rs

@@ -759,8 +759,10 @@ impl GlobalState {
 
                     let vfs = &mut this.vfs.write().0;
                     let file_id = vfs.file_id(&path).unwrap();
-                    let mut text = String::from_utf8(vfs.file_contents(file_id).to_vec()).unwrap();
-                    apply_document_changes(&mut text, params.content_changes);
+                    let text = apply_document_changes(
+                        || std::str::from_utf8(vfs.file_contents(file_id)).unwrap().into(),
+                        params.content_changes,
+                    );
 
                     vfs.set_file_contents(path, Some(text.into_bytes()));
                 }