a38ec6d · helix · git.iko.soy

git.iko.soy/helix

on master help sign in

avoid excessive memory consumption in picker (#8127)

a38ec6dPascal Kuthe3 years ago

helix-term/src/compositor.rs+6

compare with master

unifiedsplit

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

16}

18use crate::job::Jobs;

19+use crate::ui::picker;

19use helix_view::Editor;

20use helix_view::Editor;

21pub use helix_view::input::Event;

22pub use helix_view::input::Event;

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

100

101

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

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

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

103 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+ }

103 let size = self.size();

109 let size = self.size();

104 // trigger required_size on init

110 // trigger required_size on init

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

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

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

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

6mod markdown;

7pub mod menu;

8pub mod overlay;

9−mod picker;

9+pub mod picker;

10pub mod popup;

11mod prompt;

12mod spinner;

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

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

46 Document, DocumentId, Editor,

47};

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

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

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

51pub const MIN_AREA_WIDTH_FOR_PREVIEW: u16 = 72;

52pub const MIN_AREA_WIDTH_FOR_PREVIEW: u16 = 72;

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

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

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

803 };

804 };

804

805

805− let close_fn =

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+ };

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

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

807− // remove the layer

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

809− })));

827+ };

810

828

811 // So that idle timeout retriggers

829 // So that idle timeout retriggers

812 ctx.editor.reset_idle_timer();

830 ctx.editor.reset_idle_timer();

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

830 key!(End) => {

848 key!(End) => {

831 self.to_end();

849 self.to_end();

832 }

850 }

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

834− return close_fn;

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

835− }

836 alt!(Enter) => {

852 alt!(Enter) => {

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

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

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

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

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

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

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

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

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

844 }

860 }

845− return close_fn;

861+ return close_fn(self);

846 }

862 }

847 ctrl!('s') => {

863 ctrl!('s') => {

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

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

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

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

850 }

866 }

851− return close_fn;

867+ return close_fn(self);

852 }

868 }

853 ctrl!('v') => {

869 ctrl!('v') => {

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

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

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

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

856 }

872 }

857− return close_fn;

873+ return close_fn(self);

858 }

874 }

859 ctrl!('t') => {

875 ctrl!('t') => {

860 self.toggle_preview();

876 self.toggle_preview();

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

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

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

883 Some((width, height))

899 Some((width, height))

884 }

900 }

901+

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

903+ Some(ID)

904+ }

885}

905}

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

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

887 fn drop(&mut self) {

907 fn drop(&mut self) {

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

906}

926}

907

927

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

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

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

910−

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

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

912 Self {

930 Self {

913 file_picker,

931 file_picker,

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

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

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

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

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

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

959 // 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) {

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

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

944 None => return,

962 None => return,

945 };

963 };

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

960 }

978 }

961

979

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

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

963− Some(Self::ID)

981+ Some(ID)

964 }

982 }

965}

983}