a38ec6d · helix · git.iko.soy

git.iko.soy/helix

avoid excessive memory consumption in picker (#8127)

* avoid excessive memory consumption from file picker * fix typos Co-authored-by: Chris <75008413+cd-a@users.noreply.github.com> --------- Co-authored-by: Chris <75008413+cd-a@users.noreply.github.com>

commita38ec6d6ca9e5dbbd2e313f3173f2e967ed71fc1 authorPascal Kuthe <pascal.kuthe@semimod.de> dateFri 1 Sep 2023, 02:13 (3 years ago) parent7cf775d

3 files changed+40−16

compare with master

unifiedsplit

helix-term/src/compositor.rs+6 helix-term/src/ui/mod.rs+1−1 helix-term/src/ui/picker.rs+33−15

helix-term/src/compositor.rs+6

@@ -16,6 +16,7 @@ pub enum EventResult {

1616}

1717

1818use crate::job::Jobs;

19+use crate::ui::picker;

1920use helix_view::Editor;

2021

2122pub use helix_view::input::Event;

@@ -100,6 +101,11 @@ impl Compositor {

100101

101102 /// Add a layer to be rendered in front of all existing layers.

102103 pub fn push(&mut self, mut layer: Box<dyn Component>) {

104+ // immediately clear last_picker field to avoid excessive memory

105+ // consumption for picker with many items

106+ if layer.id() == Some(picker::ID) {

107+ self.last_picker = None;

108+ }

103109 let size = self.size();

104110 // trigger required_size on init

105111 layer.required_size((size.width, size.height));

helix-term/src/ui/mod.rs+1−1

@@ -6,7 +6,7 @@ pub mod lsp;

66mod markdown;

77pub mod menu;

88pub mod overlay;

9−mod picker;

9+pub mod picker;

1010pub mod popup;

1111mod prompt;

1212mod spinner;

helix-term/src/ui/picker.rs+33−15

@@ -46,6 +46,7 @@ use helix_view::{

4646 Document, DocumentId, Editor,

4747};

4848

49+pub const ID: &str = "picker";

4950use super::{menu::Item, overlay::Overlay};

5051

5152pub const MIN_AREA_WIDTH_FOR_PREVIEW: u16 = 72;

@@ -802,11 +803,28 @@ impl<T: Item + 'static + Send + Sync> Component for Picker<T> {

802803 _ => return EventResult::Ignored(None),

803804 };

804805

805− let close_fn =

806− EventResult::Consumed(Some(Box::new(|compositor: &mut Compositor, _ctx| {

807− // remove the layer

808− compositor.last_picker = compositor.pop();

809− })));

806+ let close_fn = |picker: &mut Self| {

807+ // if the picker is very large don't store it as last_picker to avoid

808+ // excessive memory consumption

809+ let callback: compositor::Callback = if picker.matcher.snapshot().item_count() > 100_000

810+ {

811+ Box::new(|compositor: &mut Compositor, _ctx| {

812+ // remove the layer

813+ compositor.pop();

814+ })

815+ } else {

816+ // stop streaming in new items in the background, really we should

817+ // be restarting the stream somehow once the picker gets

818+ // reopened instead (like for an FS crawl) that would also remove the

819+ // need for the special case above but that is pretty tricky

820+ picker.shutdown.store(true, atomic::Ordering::Relaxed);

821+ Box::new(|compositor: &mut Compositor, _ctx| {

822+ // remove the layer

823+ compositor.last_picker = compositor.pop();

824+ })

825+ };

826+ EventResult::Consumed(Some(callback))

827+ };

810828

811829 // So that idle timeout retriggers

812830 ctx.editor.reset_idle_timer();

@@ -830,9 +848,7 @@ impl<T: Item + 'static + Send + Sync> Component for Picker<T> {

830848 key!(End) => {

831849 self.to_end();

832850 }

833− key!(Esc) | ctrl!('c') => {

834− return close_fn;

835− }

851+ key!(Esc) | ctrl!('c') => return close_fn(self),

836852 alt!(Enter) => {

837853 if let Some(option) = self.selection() {

838854 (self.callback_fn)(ctx, option, Action::Load);

@@ -842,19 +858,19 @@ impl<T: Item + 'static + Send + Sync> Component for Picker<T> {

842858 if let Some(option) = self.selection() {

843859 (self.callback_fn)(ctx, option, Action::Replace);

844860 }

845− return close_fn;

861+ return close_fn(self);

846862 }

847863 ctrl!('s') => {

848864 if let Some(option) = self.selection() {

849865 (self.callback_fn)(ctx, option, Action::HorizontalSplit);

850866 }

851− return close_fn;

867+ return close_fn(self);

852868 }

853869 ctrl!('v') => {

854870 if let Some(option) = self.selection() {

855871 (self.callback_fn)(ctx, option, Action::VerticalSplit);

856872 }

857− return close_fn;

873+ return close_fn(self);

858874 }

859875 ctrl!('t') => {

860876 self.toggle_preview();

@@ -882,6 +898,10 @@ impl<T: Item + 'static + Send + Sync> Component for Picker<T> {

882898 self.completion_height = height.saturating_sub(4);

883899 Some((width, height))

884900 }

901+

902+ fn id(&self) -> Option<&'static str> {

903+ Some(ID)

904+ }

885905}

886906impl<T: Item> Drop for Picker<T> {

887907 fn drop(&mut self) {

@@ -906,8 +926,6 @@ pub struct DynamicPicker<T: ui::menu::Item + Send + Sync> {

906926}

907927

908928impl<T: ui::menu::Item + Send + Sync> DynamicPicker<T> {

909− pub const ID: &'static str = "dynamic-picker";

910−

911929 pub fn new(file_picker: Picker<T>, query_callback: DynQueryCallback<T>) -> Self {

912930 Self {

913931 file_picker,

@@ -939,7 +957,7 @@ impl<T: Item + Send + Sync + 'static> Component for DynamicPicker<T> {

939957 let callback = Callback::EditorCompositor(Box::new(move |editor, compositor| {

940958 // Wrapping of pickers in overlay is done outside the picker code,

941959 // so this is fragile and will break if wrapped in some other widget.

942− let picker = match compositor.find_id::<Overlay<DynamicPicker<T>>>(Self::ID) {

960+ let picker = match compositor.find_id::<Overlay<DynamicPicker<T>>>(ID) {

943961 Some(overlay) => &mut overlay.content.file_picker,

944962 None => return,

945963 };

@@ -960,6 +978,6 @@ impl<T: Item + Send + Sync + 'static> Component for DynamicPicker<T> {

960978 }

961979

962980 fn id(&self) -> Option<&'static str> {

963− Some(Self::ID)

981+ Some(ID)

964982 }

965983}