editor: Improve performance of edit coalescing (#14567)
# Background In https://github.com/zed-industries/zed/issues/14408 we received a repro for "Replace all" being slow, even after the work I did https://github.com/zed-industries/zed/pull/13654. Admittedly #13654 was a pretty straightforward change. Under the profiler it turned out that we're spending *10 seconds* in `memmove` on main thread. Ugh. Not great. The direct ancestor of the memmove call was66f0c390a8/crates/editor/src/display_map/tab_map.rs (L108-L119)
What? # Accidental O(n^2) We have a bunch of `consolidate_*_edits` functions which take a list of Fold/Tab/Inlay/Wrap edits and merge consecutive edits if their ranges overlap/are next to one another. The loop usually goes as follows: ``` while ix < edits.len() { let (prev_edits, next_edits) = edits.split_at_mut(ix); let prev_edit = prev_edits.last_mut().unwrap(); let edit = &next_edits[0]; if PREV_EDIT_CAN_BE_MERGED_WITH_CURRENT_ONE { MERGE_EDITS(prev_edit, edit); edits.remove(ix); // !! } else { ix += 1; } } ``` The problem is the call to `.remove` - it has to shift all of the consecutive elements in the `edits` vector! Thus, when processing the edits from the original repro (where consolidation shrinks the edit list from 210k entries to 30k), we mostly spend time moving entries in memory around. Thus, the original repro isn't really an issue with replace_all; it's just that replace_all is one of the few tools available to the end user that can apply large # of edits in a single transaction. # Solution In this PR I address the issue by rewriting the loop in a way that does not throw items away via `.remove`. Instead, `Iterator::scan` is used, which lets us achieve the same logic without having the pitfalls of `.remove`s. Crucially, **this code does not allocate a new backing buffer for edits** (see [this article for rationale](https://blog.polybdenum.com/2024/01/17/identifying-the-collect-vec-memory-leak-footgun.html)); with `vec.into_iter().scan().filter_map().collect()` we still use the same underlying buffer as the one that's passed into `consolidate_*` functions. In development I verified that by checking whether the pointers to backing storage of a Vec are the same before and after the consolidation. # Results ### Before Nightly 0.145.0 [66f0c390a8
](66f0c390a8
) https://github.com/user-attachments/assets/8b0ad3bc-86d6-4f8a-850c-ebb86e8b3bfc (~13s end-to-end) ### After https://github.com/user-attachments/assets/366835db-1d84-4f95-8c74-b1506a9fabec (~2s end-to-end) The remaining lag is (I think) lies in `TextSummary` calculation and not the consolidation itself. Thus, for the purposes of scoping this PR, I'll tackle it separately. Release Notes: - Significantly improved performance of applying large quantities of concurrent edits (e.g. when running "Replace all").
This commit is contained in:
parent
751508b6a2
commit
d338e4a8a6
3 changed files with 100 additions and 58 deletions
|
@ -104,19 +104,29 @@ impl TabMap {
|
|||
}
|
||||
|
||||
// Combine any edits that overlap due to the expansion.
|
||||
let mut ix = 1;
|
||||
while ix < fold_edits.len() {
|
||||
let (prev_edits, next_edits) = fold_edits.split_at_mut(ix);
|
||||
let prev_edit = prev_edits.last_mut().unwrap();
|
||||
let edit = &next_edits[0];
|
||||
if prev_edit.old.end >= edit.old.start {
|
||||
prev_edit.old.end = edit.old.end;
|
||||
prev_edit.new.end = edit.new.end;
|
||||
fold_edits.remove(ix);
|
||||
} else {
|
||||
ix += 1;
|
||||
}
|
||||
}
|
||||
let mut fold_edits = fold_edits.into_iter();
|
||||
let fold_edits = if let Some(mut first_edit) = fold_edits.next() {
|
||||
let mut v: Vec<_> = fold_edits
|
||||
.scan(&mut first_edit, |state, edit| {
|
||||
if state.old.end >= edit.old.start {
|
||||
state.old.end = edit.old.end;
|
||||
state.new.end = edit.new.end;
|
||||
Some(None) // Skip this edit, it's merged
|
||||
} else {
|
||||
let new_state = edit.clone();
|
||||
let result = Some(Some(state.clone())); // Yield the previous edit
|
||||
**state = new_state;
|
||||
result
|
||||
}
|
||||
})
|
||||
.flatten()
|
||||
.collect();
|
||||
v.push(first_edit);
|
||||
|
||||
v
|
||||
} else {
|
||||
vec![]
|
||||
};
|
||||
|
||||
for fold_edit in fold_edits {
|
||||
let old_start = fold_edit.old.start.to_point(&old_snapshot.fold_snapshot);
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue