From 3ceb39c82c487f795c056ebc66af5c9445a1571f Mon Sep 17 00:00:00 2001 From: Fernando Herrera <fernando.j.herrera@gmail.com> Date: Wed, 6 Apr 2022 13:25:02 +0100 Subject: [PATCH] use arc to avoid cloning entire engine for menus (#5104) * use arc to avoid cloning entire engine for menus * remove complete import path * remove stack clone * reference in completer --- crates/nu-cli/src/completions/base.rs | 2 +- .../src/completions/command_completions.rs | 5 ++-- crates/nu-cli/src/completions/completer.rs | 5 ++-- .../src/completions/custom_completions.rs | 5 ++-- .../src/completions/file_completions.rs | 5 ++-- .../src/completions/variable_completions.rs | 6 ++-- crates/nu-cli/src/menus/help_completions.rs | 5 ++-- crates/nu-cli/src/menus/menu_completions.rs | 19 +++++++++---- crates/nu-cli/src/reedline_config.rs | 28 ++++++++++--------- crates/nu-cli/src/repl.rs | 5 ++-- 10 files changed, 50 insertions(+), 35 deletions(-) diff --git a/crates/nu-cli/src/completions/base.rs b/crates/nu-cli/src/completions/base.rs index 01a49f3d44..ee56d693bf 100644 --- a/crates/nu-cli/src/completions/base.rs +++ b/crates/nu-cli/src/completions/base.rs @@ -44,7 +44,7 @@ pub trait Completer { .collect() } - // Sort is results using the completion options + // Sort results using the completion options fn sort( &self, items: Vec<Suggestion>, diff --git a/crates/nu-cli/src/completions/command_completions.rs b/crates/nu-cli/src/completions/command_completions.rs index 83962ed148..503e3b566b 100644 --- a/crates/nu-cli/src/completions/command_completions.rs +++ b/crates/nu-cli/src/completions/command_completions.rs @@ -7,9 +7,10 @@ use nu_protocol::{ Span, }; use reedline::Suggestion; +use std::sync::Arc; pub struct CommandCompletion { - engine_state: EngineState, + engine_state: Arc<EngineState>, flattened: Vec<(Span, FlatShape)>, flat_idx: usize, flat_shape: FlatShape, @@ -17,7 +18,7 @@ pub struct CommandCompletion { impl CommandCompletion { pub fn new( - engine_state: EngineState, + engine_state: Arc<EngineState>, _: &StateWorkingSet, flattened: Vec<(Span, FlatShape)>, flat_idx: usize, diff --git a/crates/nu-cli/src/completions/completer.rs b/crates/nu-cli/src/completions/completer.rs index 68174c972f..c8272fe974 100644 --- a/crates/nu-cli/src/completions/completer.rs +++ b/crates/nu-cli/src/completions/completer.rs @@ -8,16 +8,17 @@ use nu_protocol::{ Span, Value, }; use reedline::{Completer as ReedlineCompleter, Suggestion}; +use std::sync::Arc; #[derive(Clone)] pub struct NuCompleter { - engine_state: EngineState, + engine_state: Arc<EngineState>, stack: Stack, config: Option<Value>, } impl NuCompleter { - pub fn new(engine_state: EngineState, stack: Stack, config: Option<Value>) -> Self { + pub fn new(engine_state: Arc<EngineState>, stack: Stack, config: Option<Value>) -> Self { Self { engine_state, stack, diff --git a/crates/nu-cli/src/completions/custom_completions.rs b/crates/nu-cli/src/completions/custom_completions.rs index 8ce43ba85b..d8790d3a55 100644 --- a/crates/nu-cli/src/completions/custom_completions.rs +++ b/crates/nu-cli/src/completions/custom_completions.rs @@ -6,9 +6,10 @@ use nu_protocol::{ PipelineData, Span, Type, Value, CONFIG_VARIABLE_ID, }; use reedline::Suggestion; +use std::sync::Arc; pub struct CustomCompletion { - engine_state: EngineState, + engine_state: Arc<EngineState>, stack: Stack, config: Option<Value>, decl_id: usize, @@ -17,7 +18,7 @@ pub struct CustomCompletion { impl CustomCompletion { pub fn new( - engine_state: EngineState, + engine_state: Arc<EngineState>, stack: Stack, config: Option<Value>, decl_id: usize, diff --git a/crates/nu-cli/src/completions/file_completions.rs b/crates/nu-cli/src/completions/file_completions.rs index 1f7fda70b4..255b6c4457 100644 --- a/crates/nu-cli/src/completions/file_completions.rs +++ b/crates/nu-cli/src/completions/file_completions.rs @@ -5,16 +5,17 @@ use nu_protocol::{ }; use reedline::Suggestion; use std::path::{is_separator, Path}; +use std::sync::Arc; const SEP: char = std::path::MAIN_SEPARATOR; #[derive(Clone)] pub struct FileCompletion { - engine_state: EngineState, + engine_state: Arc<EngineState>, } impl FileCompletion { - pub fn new(engine_state: EngineState) -> Self { + pub fn new(engine_state: Arc<EngineState>) -> Self { Self { engine_state } } } diff --git a/crates/nu-cli/src/completions/variable_completions.rs b/crates/nu-cli/src/completions/variable_completions.rs index 2a06392f37..71ef2dd071 100644 --- a/crates/nu-cli/src/completions/variable_completions.rs +++ b/crates/nu-cli/src/completions/variable_completions.rs @@ -3,16 +3,16 @@ use nu_protocol::{ engine::{EngineState, StateWorkingSet}, Span, }; - use reedline::Suggestion; +use std::sync::Arc; #[derive(Clone)] pub struct VariableCompletion { - engine_state: EngineState, + engine_state: Arc<EngineState>, } impl VariableCompletion { - pub fn new(engine_state: EngineState) -> Self { + pub fn new(engine_state: Arc<EngineState>) -> Self { Self { engine_state } } } diff --git a/crates/nu-cli/src/menus/help_completions.rs b/crates/nu-cli/src/menus/help_completions.rs index 0c7e021b76..08428e612c 100644 --- a/crates/nu-cli/src/menus/help_completions.rs +++ b/crates/nu-cli/src/menus/help_completions.rs @@ -1,11 +1,12 @@ use nu_engine::documentation::get_flags_section; use nu_protocol::{engine::EngineState, levenshtein_distance}; use reedline::{Completer, Suggestion}; +use std::sync::Arc; -pub struct NuHelpCompleter(EngineState); +pub struct NuHelpCompleter(Arc<EngineState>); impl NuHelpCompleter { - pub fn new(engine_state: EngineState) -> Self { + pub fn new(engine_state: Arc<EngineState>) -> Self { Self(engine_state) } diff --git a/crates/nu-cli/src/menus/menu_completions.rs b/crates/nu-cli/src/menus/menu_completions.rs index 1631fad688..96de1823d6 100644 --- a/crates/nu-cli/src/menus/menu_completions.rs +++ b/crates/nu-cli/src/menus/menu_completions.rs @@ -4,6 +4,7 @@ use nu_protocol::{ IntoPipelineData, Span, Value, }; use reedline::{menu_functions::parse_selection_char, Completer, Suggestion}; +use std::sync::Arc; const SELECTION_CHAR: char = '!'; @@ -11,7 +12,7 @@ pub struct NuMenuCompleter { block_id: usize, span: Span, stack: Stack, - engine_state: EngineState, + engine_state: Arc<EngineState>, only_buffer_difference: bool, } @@ -20,7 +21,7 @@ impl NuMenuCompleter { block_id: usize, span: Span, stack: Stack, - engine_state: EngineState, + engine_state: Arc<EngineState>, only_buffer_difference: bool, ) -> Self { Self { @@ -38,7 +39,6 @@ impl Completer for NuMenuCompleter { let parsed = parse_selection_char(line, SELECTION_CHAR); let block = self.engine_state.get_block(self.block_id); - let mut stack = self.stack.clone(); if let Some(buffer) = block.signature.get_positional(0) { if let Some(buffer_id) = &buffer.var_id { @@ -46,7 +46,7 @@ impl Completer for NuMenuCompleter { val: parsed.remainder.to_string(), span: self.span, }; - stack.add_var(*buffer_id, line_buffer); + self.stack.add_var(*buffer_id, line_buffer); } } @@ -56,12 +56,19 @@ impl Completer for NuMenuCompleter { val: pos as i64, span: self.span, }; - stack.add_var(*position_id, line_buffer); + self.stack.add_var(*position_id, line_buffer); } } let input = Value::nothing(self.span).into_pipeline_data(); - let res = eval_block(&self.engine_state, &mut stack, block, input, false, false); + let res = eval_block( + &self.engine_state, + &mut self.stack, + block, + input, + false, + false, + ); if let Ok(values) = res { let values = values.into_value(self.span); diff --git a/crates/nu-cli/src/reedline_config.rs b/crates/nu-cli/src/reedline_config.rs index 0c1610d0f4..7723341efc 100644 --- a/crates/nu-cli/src/reedline_config.rs +++ b/crates/nu-cli/src/reedline_config.rs @@ -14,6 +14,7 @@ use reedline::{ default_emacs_keybindings, default_vi_insert_keybindings, default_vi_normal_keybindings, ColumnarMenu, EditCommand, Keybindings, ListMenu, Reedline, ReedlineEvent, ReedlineMenu, }; +use std::sync::Arc; const DEFAULT_COMPLETION_MENU: &str = r#" { @@ -72,14 +73,14 @@ const DEFAULT_HELP_MENU: &str = r#" // Adds all menus to line editor pub(crate) fn add_menus( mut line_editor: Reedline, - engine_state: &EngineState, + engine_state: Arc<EngineState>, stack: &Stack, config: &Config, ) -> Result<Reedline, ShellError> { line_editor = line_editor.clear_menus(); for menu in &config.menus { - line_editor = add_menu(line_editor, menu, engine_state, stack, config)? + line_editor = add_menu(line_editor, menu, engine_state.clone(), stack, config)? } // Checking if the default menus have been added from the config file @@ -96,7 +97,7 @@ pub(crate) fn add_menus( .any(|menu| menu.name.into_string("", config) == name) { let (block, _) = { - let mut working_set = StateWorkingSet::new(engine_state); + let mut working_set = StateWorkingSet::new(&engine_state); let (output, _) = parse( &mut working_set, Some(name), // format!("entry #{}", entry_num) @@ -110,11 +111,12 @@ pub(crate) fn add_menus( let mut temp_stack = Stack::new(); let input = Value::nothing(Span::test_data()).into_pipeline_data(); - let res = eval_block(engine_state, &mut temp_stack, &block, input, false, false)?; + let res = eval_block(&engine_state, &mut temp_stack, &block, input, false, false)?; if let PipelineData::Value(value, None) = res { for menu in create_menus(&value, config)? { - line_editor = add_menu(line_editor, &menu, engine_state, stack, config)?; + line_editor = + add_menu(line_editor, &menu, engine_state.clone(), stack, config)?; } } } @@ -126,7 +128,7 @@ pub(crate) fn add_menus( fn add_menu( line_editor: Reedline, menu: &ParsedMenu, - engine_state: &EngineState, + engine_state: Arc<EngineState>, stack: &Stack, config: &Config, ) -> Result<Reedline, ShellError> { @@ -176,7 +178,7 @@ macro_rules! add_style { pub(crate) fn add_columnar_menu( line_editor: Reedline, menu: &ParsedMenu, - engine_state: &EngineState, + engine_state: Arc<EngineState>, stack: &Stack, config: &Config, ) -> Result<Reedline, ShellError> { @@ -258,7 +260,7 @@ pub(crate) fn add_columnar_menu( *val, *span, stack.captures_to_stack(captures), - engine_state.clone(), + engine_state, only_buffer_difference, ); Ok(line_editor.with_menu(ReedlineMenu::WithCompleter { @@ -278,7 +280,7 @@ pub(crate) fn add_columnar_menu( pub(crate) fn add_list_menu( line_editor: Reedline, menu: &ParsedMenu, - engine_state: &EngineState, + engine_state: Arc<EngineState>, stack: &Stack, config: &Config, ) -> Result<Reedline, ShellError> { @@ -344,7 +346,7 @@ pub(crate) fn add_list_menu( *val, *span, stack.captures_to_stack(captures), - engine_state.clone(), + engine_state, only_buffer_difference, ); Ok(line_editor.with_menu(ReedlineMenu::WithCompleter { @@ -364,7 +366,7 @@ pub(crate) fn add_list_menu( pub(crate) fn add_description_menu( line_editor: Reedline, menu: &ParsedMenu, - engine_state: &EngineState, + engine_state: Arc<EngineState>, stack: &Stack, config: &Config, ) -> Result<Reedline, ShellError> { @@ -451,7 +453,7 @@ pub(crate) fn add_description_menu( match &menu.source { Value::Nothing { .. } => { - let completer = Box::new(NuHelpCompleter::new(engine_state.clone())); + let completer = Box::new(NuHelpCompleter::new(engine_state)); Ok(line_editor.with_menu(ReedlineMenu::WithCompleter { menu: Box::new(description_menu), completer, @@ -466,7 +468,7 @@ pub(crate) fn add_description_menu( *val, *span, stack.captures_to_stack(captures), - engine_state.clone(), + engine_state, only_buffer_difference, ); Ok(line_editor.with_menu(ReedlineMenu::WithCompleter { diff --git a/crates/nu-cli/src/repl.rs b/crates/nu-cli/src/repl.rs index a209d298ec..c68e41c313 100644 --- a/crates/nu-cli/src/repl.rs +++ b/crates/nu-cli/src/repl.rs @@ -169,6 +169,7 @@ pub fn evaluate_repl( ctrlc.store(false, Ordering::SeqCst); } + let engine_reference = std::sync::Arc::new(engine_state.clone()); line_editor = line_editor .with_highlighter(Box::new(NuHighlighter { engine_state: engine_state.clone(), @@ -182,7 +183,7 @@ pub fn evaluate_repl( DefaultHinter::default().with_style(color_hm["hints"]), )) .with_completer(Box::new(NuCompleter::new( - engine_state.clone(), + engine_reference.clone(), stack.clone(), stack.vars.get(&CONFIG_VARIABLE_ID).cloned(), ))) @@ -194,7 +195,7 @@ pub fn evaluate_repl( info!("update reedline {}:{}:{}", file!(), line!(), column!()); } - line_editor = match add_menus(line_editor, engine_state, stack, &config) { + line_editor = match add_menus(line_editor, engine_reference, stack, &config) { Ok(line_editor) => line_editor, Err(e) => { let working_set = StateWorkingSet::new(engine_state);