From b570c7f6a6a6b9c5c6638d5bc86943342f7d5b5a Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Fri, 22 Nov 2024 14:11:01 -0600 Subject: [PATCH] Reduce allocations when deduping completions in place This is still suboptimal because we are allocating a vector of indices to be removed (but allocation-free in the normal case of no duplicates) but significantly better than the previous version of the code that duplicated the strings (which are larger and spread out all over the heap). The ideal code (similar to what we had in the C++ version, iirc) would look like this, but it's not allowed because the borrow checker hates you: ``` fn unique_in_place_illegal(comps: &mut Vec) { let mut seen = HashSet::with_capacity(comps.len()); let mut idx = 0; while idx < comps.len() { if !seen.insert(&comps[idx].completion) { comps.remove(idx); continue; } idx += 1; } } ``` --- src/complete.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/complete.rs b/src/complete.rs index 84a770f86..752a57b06 100644 --- a/src/complete.rs +++ b/src/complete.rs @@ -512,14 +512,17 @@ fn compare_completions_by_tilde(a: &Completion, b: &Completion) -> Ordering { /// Unique the list of completions, without perturbing their order. fn unique_completions_retaining_order(comps: &mut Vec) { let mut seen = HashSet::with_capacity(comps.len()); + let removals = comps.iter().enumerate().fold(vec![], |mut v, (i, c)| { + if !seen.insert(&c.completion) { + v.push(i); + } + v + }); - let pred = |c: &Completion| { - // Keep (return true) if insertion succeeds. - // todo!("don't clone here"); - seen.insert(c.completion.to_owned()) - }; - - comps.retain(pred); + // Remove in reverse order so the indexes remain valid + for idx in removals.iter().rev() { + comps.remove(*idx); + } } /// Sorts and removes any duplicate completions in the completion list, then puts them in priority