From 09c497f744d1696a796cd55e1388e7895b8f20b8 Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Tue, 16 Jul 2024 18:29:45 +0200 Subject: [PATCH] editor: Ensure allocation reuse (#14577) In #14567 I claimed that the underlying allocation is reused. And it was. At the time I've submitted a PR I was using `.filter_map(|x| x)`, which got flagged by clippy as something that could be simplified to `.flatten()` - that however broke the allocation reuse promise. Thus, this PR goes back to using `filter_map` and additionally in debug builds it performs checks for allocation reuse. With .flatten in place, a bunch of unit test fail on that branch, so the checks do work. Release Notes: - N/A --- crates/editor/src/display_map/fold_map.rs | 13 +++++++++---- crates/editor/src/display_map/tab_map.rs | 7 +++++-- crates/editor/src/display_map/wrap_map.rs | 7 +++++-- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/crates/editor/src/display_map/fold_map.rs b/crates/editor/src/display_map/fold_map.rs index 1484ec0257..328aef9b45 100644 --- a/crates/editor/src/display_map/fold_map.rs +++ b/crates/editor/src/display_map/fold_map.rs @@ -817,8 +817,11 @@ fn consolidate_inlay_edits(mut edits: Vec) -> Vec { .then_with(|| b.old.end.cmp(&a.old.end)) }); + let _old_alloc_ptr = edits.as_ptr(); let mut inlay_edits = edits.into_iter(); let inlay_edits = if let Some(mut first_edit) = inlay_edits.next() { + // This code relies on reusing allocations from the Vec<_> - at the time of writing .flatten() prevents them. + #[allow(clippy::filter_map_identity)] let mut v: Vec<_> = inlay_edits .scan(&mut first_edit, |prev_edit, edit| { if prev_edit.old.end >= edit.old.start { @@ -831,10 +834,10 @@ fn consolidate_inlay_edits(mut edits: Vec) -> Vec { Some(Some(prev)) // Yield the previous edit } }) - .flatten() + .filter_map(|x| x) .collect(); v.push(first_edit.clone()); - + debug_assert_eq!(_old_alloc_ptr, v.as_ptr(), "Inlay edits were reallocated"); v } else { vec![] @@ -850,9 +853,11 @@ fn consolidate_fold_edits(mut edits: Vec) -> Vec { .cmp(&b.old.start) .then_with(|| b.old.end.cmp(&a.old.end)) }); - + let _old_alloc_ptr = edits.as_ptr(); let mut fold_edits = edits.into_iter(); let fold_edits = if let Some(mut first_edit) = fold_edits.next() { + // This code relies on reusing allocations from the Vec<_> - at the time of writing .flatten() prevents them. + #[allow(clippy::filter_map_identity)] let mut v: Vec<_> = fold_edits .scan(&mut first_edit, |prev_edit, edit| { if prev_edit.old.end >= edit.old.start { @@ -865,7 +870,7 @@ fn consolidate_fold_edits(mut edits: Vec) -> Vec { Some(Some(prev)) // Yield the previous edit } }) - .flatten() + .filter_map(|x| x) .collect(); v.push(first_edit.clone()); v diff --git a/crates/editor/src/display_map/tab_map.rs b/crates/editor/src/display_map/tab_map.rs index cdb4e6da13..f497fb1758 100644 --- a/crates/editor/src/display_map/tab_map.rs +++ b/crates/editor/src/display_map/tab_map.rs @@ -103,9 +103,12 @@ impl TabMap { } } + let _old_alloc_ptr = fold_edits.as_ptr(); // Combine any edits that overlap due to the expansion. let mut fold_edits = fold_edits.into_iter(); let fold_edits = if let Some(mut first_edit) = fold_edits.next() { + // This code relies on reusing allocations from the Vec<_> - at the time of writing .flatten() prevents them. + #[allow(clippy::filter_map_identity)] let mut v: Vec<_> = fold_edits .scan(&mut first_edit, |state, edit| { if state.old.end >= edit.old.start { @@ -119,10 +122,10 @@ impl TabMap { result } }) - .flatten() + .filter_map(|x| x) .collect(); v.push(first_edit); - + debug_assert_eq!(v.as_ptr(), _old_alloc_ptr, "Fold edits were reallocated"); v } else { vec![] diff --git a/crates/editor/src/display_map/wrap_map.rs b/crates/editor/src/display_map/wrap_map.rs index 03727f22c9..15f497336d 100644 --- a/crates/editor/src/display_map/wrap_map.rs +++ b/crates/editor/src/display_map/wrap_map.rs @@ -1009,8 +1009,11 @@ impl<'a> sum_tree::Dimension<'a, TransformSummary> for WrapPoint { } fn consolidate_wrap_edits(edits: Vec) -> Vec { + let _old_alloc_ptr = edits.as_ptr(); let mut wrap_edits = edits.into_iter(); let wrap_edits = if let Some(mut first_edit) = wrap_edits.next() { + // This code relies on reusing allocations from the Vec<_> - at the time of writing .flatten() prevents them. + #[allow(clippy::filter_map_identity)] let mut v: Vec<_> = wrap_edits .scan(&mut first_edit, |prev_edit, edit| { if prev_edit.old.end >= edit.old.start { @@ -1022,10 +1025,10 @@ fn consolidate_wrap_edits(edits: Vec) -> Vec { Some(Some(prev)) // Yield the previous edit } }) - .flatten() + .filter_map(|x| x) .collect(); v.push(first_edit.clone()); - + debug_assert_eq!(v.as_ptr(), _old_alloc_ptr, "Wrap edits were reallocated"); v } else { vec![]