From 5df04ee8672f53160c772f3720fab8d876d95001 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Tue, 24 Jan 2023 23:38:04 +0200 Subject: [PATCH 01/17] Quick mockup of sub-pipeline benchmarking --- crates/nu-command/src/system/benchmark.rs | 4 +- crates/nu-engine/src/eval.rs | 169 ++++++++++++++++++++++ crates/nu-engine/src/lib.rs | 2 +- 3 files changed, 172 insertions(+), 3 deletions(-) diff --git a/crates/nu-command/src/system/benchmark.rs b/crates/nu-command/src/system/benchmark.rs index d9ba5e2c75197..650d59b01477a 100644 --- a/crates/nu-command/src/system/benchmark.rs +++ b/crates/nu-command/src/system/benchmark.rs @@ -1,6 +1,6 @@ use std::time::Instant; -use nu_engine::{eval_block, CallExt}; +use nu_engine::{eval_block_benchmark, CallExt}; use nu_protocol::ast::Call; use nu_protocol::engine::{Closure, Command, EngineState, Stack}; use nu_protocol::{ @@ -65,7 +65,7 @@ impl Command for Benchmark { // Get the start time after all other computation has been done. let start_time = Instant::now(); - eval_block( + eval_block_benchmark( engine_state, &mut stack, block, diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index f862d4e544aca..3f6f615cc37d9 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -11,6 +11,7 @@ use nu_protocol::{ }; use nu_utils::stdout_write_all_and_flush; use std::collections::HashMap; +use std::time::Instant; pub fn eval_operator(op: &Expression) -> Result { match op { @@ -966,6 +967,174 @@ pub fn eval_block_with_early_return( } } +pub fn eval_block_benchmark( + engine_state: &EngineState, + stack: &mut Stack, + block: &Block, + mut input: PipelineData, + redirect_stdout: bool, + redirect_stderr: bool, +) -> Result { + // if Block contains recursion, make sure we don't recurse too deeply (to avoid stack overflow) + if let Some(recursive) = block.recursive { + // picked 50 arbitrarily, should work on all architectures + const RECURSION_LIMIT: u64 = 50; + if recursive { + if *stack.recursion_count >= RECURSION_LIMIT { + stack.recursion_count = Box::new(0); + return Err(ShellError::RecursionLimitReached { + recursion_limit: RECURSION_LIMIT, + span: block.span, + }); + } + *stack.recursion_count += 1; + } + } + let num_pipelines = block.len(); + for (pipeline_idx, pipeline) in block.pipelines.iter().enumerate() { + let mut i = 0; + + while i < pipeline.elements.len() { + let redirect_stderr = redirect_stderr + || ((i < pipeline.elements.len() - 1) + && (matches!( + pipeline.elements[i + 1], + PipelineElement::Redirection(_, Redirection::Stderr, _) + | PipelineElement::Redirection(_, Redirection::StdoutAndStderr, _) + | PipelineElement::SeparateRedirection { .. } + ))); + + // if eval internal command failed, it can just make early return with `Err(ShellError)`. + let start_time = Instant::now(); + let eval_result = eval_element_with_input( + engine_state, + stack, + &pipeline.elements[i], + input, + redirect_stdout + || (i != pipeline.elements.len() - 1) + && (matches!( + pipeline.elements[i + 1], + PipelineElement::Redirection(_, Redirection::Stdout, _) + | PipelineElement::Redirection(_, Redirection::StdoutAndStderr, _) + | PipelineElement::Expression(..) + | PipelineElement::SeparateRedirection { .. } + )), + redirect_stderr, + ); + let end_time = Instant::now(); + + let element_str = String::from_utf8_lossy( + engine_state.get_span_contents(&pipeline.elements[i].span()), + ); + println!( + "`{}` : {} us", + element_str, + (end_time - start_time).as_micros() + ); + + match (eval_result, redirect_stderr) { + (Ok((pipeline_data, _)), true) => { + input = pipeline_data; + + // external command may runs to failed + // make early return so remaining commands will not be executed. + // don't return `Err(ShellError)`, so nushell wouldn't show extra error message. + } + (Err(error), true) => input = PipelineData::Value(Value::Error { error }, None), + (output, false) => { + let output = output?; + input = output.0; + // external command may runs to failed + // make early return so remaining commands will not be executed. + // don't return `Err(ShellError)`, so nushell wouldn't show extra error message. + if output.1 { + return Ok(input); + } + } + } + + i += 1; + } + + if pipeline_idx < (num_pipelines) - 1 { + match input { + PipelineData::Value(Value::Nothing { .. }, ..) => {} + PipelineData::ExternalStream { + ref mut exit_code, .. + } => { + let exit_code = exit_code.take(); + + // Drain the input to the screen via tabular output + let config = engine_state.get_config(); + + match engine_state.find_decl("table".as_bytes(), &[]) { + Some(decl_id) => { + let table = engine_state.get_decl(decl_id).run( + engine_state, + stack, + &Call::new(Span::new(0, 0)), + input, + )?; + + print_or_return(table, config)?; + } + None => { + print_or_return(input, config)?; + } + }; + + if let Some(exit_code) = exit_code { + let mut v: Vec<_> = exit_code.collect(); + + if let Some(v) = v.pop() { + stack.add_env_var("LAST_EXIT_CODE".into(), v); + } + } + } + _ => { + // Drain the input to the screen via tabular output + let config = engine_state.get_config(); + + match engine_state.find_decl("table".as_bytes(), &[]) { + Some(decl_id) => { + let table = engine_state.get_decl(decl_id); + + if let Some(block_id) = table.get_block_id() { + let block = engine_state.get_block(block_id); + eval_block( + engine_state, + stack, + block, + input, + redirect_stdout, + redirect_stderr, + )?; + } else { + let table = table.run( + engine_state, + stack, + &Call::new(Span::new(0, 0)), + input, + )?; + + print_or_return(table, config)?; + } + } + None => { + print_or_return(input, config)?; + } + }; + } + } + + input = PipelineData::empty() + } + } + + Ok(input) +} + pub fn eval_block( engine_state: &EngineState, stack: &mut Stack, diff --git a/crates/nu-engine/src/lib.rs b/crates/nu-engine/src/lib.rs index 657dc776353a7..552b73e986260 100644 --- a/crates/nu-engine/src/lib.rs +++ b/crates/nu-engine/src/lib.rs @@ -12,7 +12,7 @@ pub use column::get_columns; pub use documentation::get_full_help; pub use env::*; pub use eval::{ - eval_block, eval_block_with_early_return, eval_call, eval_expression, + eval_block, eval_block_benchmark, eval_block_with_early_return, eval_call, eval_expression, eval_expression_with_input, eval_operator, eval_subexpression, eval_variable, redirect_env, }; pub use glob_from::glob_from; From 6921ab0239dd1f23ad13c17378875cf32a414702 Mon Sep 17 00:00:00 2001 From: Darren Schroeder <343840+fdncred@users.noreply.github.com> Date: Wed, 25 Jan 2023 08:23:30 -0600 Subject: [PATCH 02/17] added some formatting and highlighting --- crates/nu-engine/src/eval.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index 3f6f615cc37d9..460181e52d056 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -6,8 +6,8 @@ use nu_protocol::{ Operator, PathMember, PipelineElement, Redirection, }, engine::{EngineState, Stack}, - Config, IntoInterruptiblePipelineData, IntoPipelineData, PipelineData, Range, ShellError, Span, - Spanned, Unit, Value, VarId, ENV_VARIABLE_ID, + format_duration, Config, IntoInterruptiblePipelineData, IntoPipelineData, PipelineData, Range, + ShellError, Span, Spanned, Unit, Value, VarId, ENV_VARIABLE_ID, }; use nu_utils::stdout_write_all_and_flush; use std::collections::HashMap; @@ -1028,9 +1028,9 @@ pub fn eval_block_benchmark( engine_state.get_span_contents(&pipeline.elements[i].span()), ); println!( - "`{}` : {} us", + "\x1b[32m{}\x1b[0m : \x1b[33m{}\x1b[0m", element_str, - (end_time - start_time).as_micros() + format_duration((end_time - start_time).as_nanos() as i64) ); match (eval_result, redirect_stderr) { From c9aa68471e5320afe37de584e9719af94731fc95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Wed, 1 Feb 2023 22:51:52 +0200 Subject: [PATCH 03/17] Allow setting benchmark depth; Use one eval func --- crates/nu-command/src/system/benchmark.rs | 16 +- crates/nu-engine/src/eval.rs | 184 +++------------------- crates/nu-engine/src/lib.rs | 2 +- crates/nu-protocol/src/engine/stack.rs | 4 + 4 files changed, 38 insertions(+), 168 deletions(-) diff --git a/crates/nu-command/src/system/benchmark.rs b/crates/nu-command/src/system/benchmark.rs index 650d59b01477a..777e5a8040390 100644 --- a/crates/nu-command/src/system/benchmark.rs +++ b/crates/nu-command/src/system/benchmark.rs @@ -1,6 +1,6 @@ use std::time::Instant; -use nu_engine::{eval_block_benchmark, CallExt}; +use nu_engine::{eval_block, CallExt}; use nu_protocol::ast::Call; use nu_protocol::engine::{Closure, Command, EngineState, Stack}; use nu_protocol::{ @@ -27,6 +27,12 @@ impl Command for Benchmark { SyntaxShape::Closure(Some(vec![SyntaxShape::Any])), "the closure to run", ) + .named( + "max-depth", + SyntaxShape::Int, + "How many levels of blocks to step into (default 1)", + Some('d'), + ) .input_output_types(vec![ (Type::Any, Type::Duration), (Type::Nothing, Type::Duration), @@ -64,8 +70,14 @@ impl Command for Benchmark { } // Get the start time after all other computation has been done. + stack.debug_depth = + if let Some(depth) = call.get_flag::(engine_state, &mut stack, "max-depth")? { + depth + } else { + 1 + }; let start_time = Instant::now(); - eval_block_benchmark( + eval_block( engine_state, &mut stack, block, diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index 460181e52d056..a7eab943347cd 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -967,7 +967,7 @@ pub fn eval_block_with_early_return( } } -pub fn eval_block_benchmark( +pub fn eval_block( engine_state: &EngineState, stack: &mut Stack, block: &Block, @@ -990,7 +990,16 @@ pub fn eval_block_benchmark( *stack.recursion_count += 1; } } + + let should_debug = if stack.debug_depth > 0 { + stack.debug_depth -= 1; + true + } else { + false + }; + let num_pipelines = block.len(); + for (pipeline_idx, pipeline) in block.pipelines.iter().enumerate() { let mut i = 0; @@ -1024,171 +1033,16 @@ pub fn eval_block_benchmark( ); let end_time = Instant::now(); - let element_str = String::from_utf8_lossy( - engine_state.get_span_contents(&pipeline.elements[i].span()), - ); - println!( - "\x1b[32m{}\x1b[0m : \x1b[33m{}\x1b[0m", - element_str, - format_duration((end_time - start_time).as_nanos() as i64) - ); - - match (eval_result, redirect_stderr) { - (Ok((pipeline_data, _)), true) => { - input = pipeline_data; - - // external command may runs to failed - // make early return so remaining commands will not be executed. - // don't return `Err(ShellError)`, so nushell wouldn't show extra error message. - } - (Err(error), true) => input = PipelineData::Value(Value::Error { error }, None), - (output, false) => { - let output = output?; - input = output.0; - // external command may runs to failed - // make early return so remaining commands will not be executed. - // don't return `Err(ShellError)`, so nushell wouldn't show extra error message. - if output.1 { - return Ok(input); - } - } - } - - i += 1; - } - - if pipeline_idx < (num_pipelines) - 1 { - match input { - PipelineData::Value(Value::Nothing { .. }, ..) => {} - PipelineData::ExternalStream { - ref mut exit_code, .. - } => { - let exit_code = exit_code.take(); - - // Drain the input to the screen via tabular output - let config = engine_state.get_config(); - - match engine_state.find_decl("table".as_bytes(), &[]) { - Some(decl_id) => { - let table = engine_state.get_decl(decl_id).run( - engine_state, - stack, - &Call::new(Span::new(0, 0)), - input, - )?; - - print_or_return(table, config)?; - } - None => { - print_or_return(input, config)?; - } - }; - - if let Some(exit_code) = exit_code { - let mut v: Vec<_> = exit_code.collect(); - - if let Some(v) = v.pop() { - stack.add_env_var("LAST_EXIT_CODE".into(), v); - } - } - } - _ => { - // Drain the input to the screen via tabular output - let config = engine_state.get_config(); - - match engine_state.find_decl("table".as_bytes(), &[]) { - Some(decl_id) => { - let table = engine_state.get_decl(decl_id); - - if let Some(block_id) = table.get_block_id() { - let block = engine_state.get_block(block_id); - eval_block( - engine_state, - stack, - block, - input, - redirect_stdout, - redirect_stderr, - )?; - } else { - let table = table.run( - engine_state, - stack, - &Call::new(Span::new(0, 0)), - input, - )?; - - print_or_return(table, config)?; - } - } - None => { - print_or_return(input, config)?; - } - }; - } - } - - input = PipelineData::empty() - } - } - - Ok(input) -} - -pub fn eval_block( - engine_state: &EngineState, - stack: &mut Stack, - block: &Block, - mut input: PipelineData, - redirect_stdout: bool, - redirect_stderr: bool, -) -> Result { - // if Block contains recursion, make sure we don't recurse too deeply (to avoid stack overflow) - if let Some(recursive) = block.recursive { - // picked 50 arbitrarily, should work on all architectures - const RECURSION_LIMIT: u64 = 50; - if recursive { - if *stack.recursion_count >= RECURSION_LIMIT { - stack.recursion_count = Box::new(0); - return Err(ShellError::RecursionLimitReached { - recursion_limit: RECURSION_LIMIT, - span: block.span, - }); + if should_debug { + let element_str = String::from_utf8_lossy( + engine_state.get_span_contents(&pipeline.elements[i].span()), + ); + println!( + "\x1b[32m{}\x1b[0m : \x1b[33m{}\x1b[0m", + element_str, + format_duration((end_time - start_time).as_nanos() as i64) + ); } - *stack.recursion_count += 1; - } - } - let num_pipelines = block.len(); - for (pipeline_idx, pipeline) in block.pipelines.iter().enumerate() { - let mut i = 0; - - while i < pipeline.elements.len() { - let redirect_stderr = redirect_stderr - || ((i < pipeline.elements.len() - 1) - && (matches!( - pipeline.elements[i + 1], - PipelineElement::Redirection(_, Redirection::Stderr, _) - | PipelineElement::Redirection(_, Redirection::StdoutAndStderr, _) - | PipelineElement::SeparateRedirection { .. } - ))); - - // if eval internal command failed, it can just make early return with `Err(ShellError)`. - let eval_result = eval_element_with_input( - engine_state, - stack, - &pipeline.elements[i], - input, - redirect_stdout - || (i != pipeline.elements.len() - 1) - && (matches!( - pipeline.elements[i + 1], - PipelineElement::Redirection(_, Redirection::Stdout, _) - | PipelineElement::Redirection(_, Redirection::StdoutAndStderr, _) - | PipelineElement::Expression(..) - | PipelineElement::SeparateRedirection { .. } - )), - redirect_stderr, - ); match (eval_result, redirect_stderr) { (Ok((pipeline_data, _)), true) => { diff --git a/crates/nu-engine/src/lib.rs b/crates/nu-engine/src/lib.rs index 552b73e986260..657dc776353a7 100644 --- a/crates/nu-engine/src/lib.rs +++ b/crates/nu-engine/src/lib.rs @@ -12,7 +12,7 @@ pub use column::get_columns; pub use documentation::get_full_help; pub use env::*; pub use eval::{ - eval_block, eval_block_benchmark, eval_block_with_early_return, eval_call, eval_expression, + eval_block, eval_block_with_early_return, eval_call, eval_expression, eval_expression_with_input, eval_operator, eval_subexpression, eval_variable, redirect_env, }; pub use glob_from::glob_from; diff --git a/crates/nu-protocol/src/engine/stack.rs b/crates/nu-protocol/src/engine/stack.rs index b1b423e21fe59..1f59c3f9ad69e 100644 --- a/crates/nu-protocol/src/engine/stack.rs +++ b/crates/nu-protocol/src/engine/stack.rs @@ -35,6 +35,7 @@ pub struct Stack { /// List of active overlays pub active_overlays: Vec, pub recursion_count: Box, + pub debug_depth: i64, } impl Stack { @@ -45,6 +46,7 @@ impl Stack { env_hidden: HashMap::new(), active_overlays: vec![DEFAULT_OVERLAY_NAME.to_string()], recursion_count: Box::new(0), + debug_depth: 0, } } @@ -126,6 +128,7 @@ impl Stack { env_hidden: HashMap::new(), active_overlays: self.active_overlays.clone(), recursion_count: self.recursion_count.to_owned(), + debug_depth: self.debug_depth, } } @@ -151,6 +154,7 @@ impl Stack { env_hidden: HashMap::new(), active_overlays: self.active_overlays.clone(), recursion_count: self.recursion_count.to_owned(), + debug_depth: self.debug_depth, } } From a83ecb6f0a6483605d3826e0a8f7782e84b24c76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Mon, 6 Feb 2023 21:47:38 +0200 Subject: [PATCH 04/17] Move the profiler to a separate command --- crates/nu-command/src/default_context.rs | 1 + crates/nu-command/src/experimental/mod.rs | 2 + crates/nu-command/src/experimental/profile.rs | 123 ++++++++++++++++++ crates/nu-command/src/system/benchmark.rs | 12 -- 4 files changed, 126 insertions(+), 12 deletions(-) create mode 100644 crates/nu-command/src/experimental/profile.rs diff --git a/crates/nu-command/src/default_context.rs b/crates/nu-command/src/default_context.rs index 37e343ac08fd0..392243c4e0b41 100644 --- a/crates/nu-command/src/default_context.rs +++ b/crates/nu-command/src/default_context.rs @@ -475,6 +475,7 @@ pub fn create_default_context() -> EngineState { bind_command! { ViewSource, IsAdmin, + Profile, }; // Deprecated diff --git a/crates/nu-command/src/experimental/mod.rs b/crates/nu-command/src/experimental/mod.rs index b912a315a2b53..ee8cedabee13e 100644 --- a/crates/nu-command/src/experimental/mod.rs +++ b/crates/nu-command/src/experimental/mod.rs @@ -1,5 +1,7 @@ mod is_admin; +mod profile; mod view_source; pub use is_admin::IsAdmin; +pub use profile::Profile; pub use view_source::ViewSource; diff --git a/crates/nu-command/src/experimental/profile.rs b/crates/nu-command/src/experimental/profile.rs new file mode 100644 index 0000000000000..74581fa508bd5 --- /dev/null +++ b/crates/nu-command/src/experimental/profile.rs @@ -0,0 +1,123 @@ +use nu_engine::{eval_block, CallExt}; +use nu_protocol::ast::Call; +use nu_protocol::engine::{Closure, Command, EngineState, Stack}; +use nu_protocol::{ + Category, Example, IntoPipelineData, PipelineData, Signature, SyntaxShape, Type, +}; + +#[derive(Clone)] +pub struct Profile; + +impl Command for Profile { + fn name(&self) -> &str { + "profile" + } + + fn usage(&self) -> &str { + "Time the running time of a closure" + } + + fn signature(&self) -> nu_protocol::Signature { + Signature::build("profile") + .required( + "closure", + SyntaxShape::Closure(Some(vec![SyntaxShape::Any])), + "the closure to run", + ) + .named( + "max-depth", + SyntaxShape::Int, + "How many levels of blocks to step into (default: 1)", + Some('d'), + ) + .input_output_types(vec![ + (Type::Any, Type::Duration), + (Type::Nothing, Type::Duration), + ]) + .allow_variants_without_examples(true) + .category(Category::System) + } + + fn run( + &self, + engine_state: &EngineState, + stack: &mut Stack, + call: &Call, + input: PipelineData, + ) -> Result { + let capture_block: Closure = call.req(engine_state, stack, 0)?; + let block = engine_state.get_block(capture_block.block_id); + + let redirect_stdout = call.redirect_stdout; + let redirect_stderr = call.redirect_stderr; + + let mut stack = stack.captures_to_stack(&capture_block.captures); + + // In order to provide the pipeline as a positional, it must be converted into a value. + // But because pipelines do not have Clone, this one has to be cloned as a value + // and then converted back into a pipeline for eval_block(). + // So, the metadata must be saved here and restored at that point. + let input_metadata = input.metadata(); + let input_val = input.into_value(call.head); + + if let Some(var) = block.signature.get_positional(0) { + if let Some(var_id) = &var.var_id { + stack.add_var(*var_id, input_val.clone()); + } + } + + // Get the start time after all other computation has been done. + stack.debug_depth = + if let Some(depth) = call.get_flag::(engine_state, &mut stack, "max-depth")? { + depth + } else { + 1 + }; + + let output = eval_block( + engine_state, + &mut stack, + block, + input_val.into_pipeline_data_with_metadata(input_metadata), + redirect_stdout, + redirect_stderr, + )? + .into_value(call.head); + + Ok(output.into_pipeline_data()) + } + + fn examples(&self) -> Vec { + vec![] + } +} + +#[test] +// Due to difficulty in observing side-effects from benchmark closures, +// checks that the closures have run correctly must use the filesystem. +fn test_benchmark_closure() { + use nu_test_support::{nu, nu_repl_code, playground::Playground}; + Playground::setup("test_benchmark_closure", |dirs, _| { + let inp = [ + r#"[2 3 4] | profile { to nuon | save foo.txt }"#, + "open foo.txt", + ]; + let actual_repl = nu!(cwd: dirs.test(), nu_repl_code(&inp)); + assert_eq!(actual_repl.err, ""); + assert_eq!(actual_repl.out, "[2, 3, 4]"); + }); +} + +#[test] +fn test_benchmark_closure_2() { + use nu_test_support::{nu, nu_repl_code, playground::Playground}; + Playground::setup("test_benchmark_closure", |dirs, _| { + let inp = [ + r#"[2 3 4] | profile {|e| {result: $e} | to nuon | save foo.txt }"#, + "open foo.txt", + ]; + let actual_repl = nu!(cwd: dirs.test(), nu_repl_code(&inp)); + assert_eq!(actual_repl.err, ""); + assert_eq!(actual_repl.out, "{result: [2, 3, 4]}"); + }); +} diff --git a/crates/nu-command/src/system/benchmark.rs b/crates/nu-command/src/system/benchmark.rs index 777e5a8040390..d9ba5e2c75197 100644 --- a/crates/nu-command/src/system/benchmark.rs +++ b/crates/nu-command/src/system/benchmark.rs @@ -27,12 +27,6 @@ impl Command for Benchmark { SyntaxShape::Closure(Some(vec![SyntaxShape::Any])), "the closure to run", ) - .named( - "max-depth", - SyntaxShape::Int, - "How many levels of blocks to step into (default 1)", - Some('d'), - ) .input_output_types(vec![ (Type::Any, Type::Duration), (Type::Nothing, Type::Duration), @@ -70,12 +64,6 @@ impl Command for Benchmark { } // Get the start time after all other computation has been done. - stack.debug_depth = - if let Some(depth) = call.get_flag::(engine_state, &mut stack, "max-depth")? { - depth - } else { - 1 - }; let start_time = Instant::now(); eval_block( engine_state, From 1f83d82caabedc6c36997b8048e732dca22129d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Mon, 6 Feb 2023 22:58:43 +0200 Subject: [PATCH 05/17] Stast collecting profiling output --- crates/nu-command/src/experimental/profile.rs | 9 ++-- crates/nu-engine/src/eval.rs | 48 +++++++++++++++---- crates/nu-protocol/src/engine/stack.rs | 4 ++ 3 files changed, 50 insertions(+), 11 deletions(-) diff --git a/crates/nu-command/src/experimental/profile.rs b/crates/nu-command/src/experimental/profile.rs index 74581fa508bd5..82da6614b701d 100644 --- a/crates/nu-command/src/experimental/profile.rs +++ b/crates/nu-command/src/experimental/profile.rs @@ -2,7 +2,8 @@ use nu_engine::{eval_block, CallExt}; use nu_protocol::ast::Call; use nu_protocol::engine::{Closure, Command, EngineState, Stack}; use nu_protocol::{ - Category, Example, IntoPipelineData, PipelineData, Signature, SyntaxShape, Type, + Category, Example, IntoPipelineData, PipelineData, Signature, SyntaxShape, Type, ListStream, + IntoInterruptiblePipelineData, Value }; #[derive(Clone)] @@ -74,7 +75,7 @@ impl Command for Profile { 1 }; - let output = eval_block( + let _ = eval_block( engine_state, &mut stack, block, @@ -84,7 +85,9 @@ impl Command for Profile { )? .into_value(call.head); - Ok(output.into_pipeline_data()) + let debug_output = Value::list(stack.debug_output.drain(..).collect(), call.head); + + Ok(debug_output.into_pipeline_data()) } fn examples(&self) -> Vec { diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index a7eab943347cd..dad939ff84b40 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -6,7 +6,7 @@ use nu_protocol::{ Operator, PathMember, PipelineElement, Redirection, }, engine::{EngineState, Stack}, - format_duration, Config, IntoInterruptiblePipelineData, IntoPipelineData, PipelineData, Range, + Config, IntoInterruptiblePipelineData, IntoPipelineData, ListStream, PipelineData, Range, ShellError, Span, Spanned, Unit, Value, VarId, ENV_VARIABLE_ID, }; use nu_utils::stdout_write_all_and_flush; @@ -998,6 +998,8 @@ pub fn eval_block( false }; + // let mut debug_out = vec![]; + let num_pipelines = block.len(); for (pipeline_idx, pipeline) in block.pipelines.iter().enumerate() { @@ -1034,14 +1036,44 @@ pub fn eval_block( let end_time = Instant::now(); if should_debug { - let element_str = String::from_utf8_lossy( - engine_state.get_span_contents(&pipeline.elements[i].span()), - ); - println!( - "\x1b[32m{}\x1b[0m : \x1b[33m{}\x1b[0m", - element_str, - format_duration((end_time - start_time).as_nanos() as i64) + let span = pipeline.elements[i].span(); + let element_str = Value::string( + String::from_utf8_lossy( + engine_state.get_span_contents(&pipeline.elements[i].span()), + ), + span, ); + let value = match &eval_result { + Ok((PipelineData::Value(val, ..), ..)) => val.clone(), + Ok((PipelineData::ListStream(..), ..)) => Value::string("list stream", span), + Ok((PipelineData::ExternalStream { .. }, ..)) => { + Value::string("raw stream", span) + } + Ok((PipelineData::Empty, ..)) => Value::Nothing { span }, + Err(err) => Value::Error { error: err.clone() }, + }; + let ns = (end_time - start_time).as_nanos() as i64; + + stack.debug_output.push(Value::Record { + cols: vec![ + "level".to_string(), + "source".to_string(), + "value".to_string(), + "ns".to_string(), + ], + vals: vec![ + Value::int(stack.debug_depth + 1, span), + element_str, + value, + Value::Duration { val: ns, span }, + ], + span, + }); + // println!( + // "\x1b[32m{}\x1b[0m : \x1b[33m{}\x1b[0m", + // element_str, + // format_duration((end_time - start_time).as_nanos() as i64) + // ); } match (eval_result, redirect_stderr) { diff --git a/crates/nu-protocol/src/engine/stack.rs b/crates/nu-protocol/src/engine/stack.rs index 1f59c3f9ad69e..a7af48f5eef92 100644 --- a/crates/nu-protocol/src/engine/stack.rs +++ b/crates/nu-protocol/src/engine/stack.rs @@ -36,6 +36,7 @@ pub struct Stack { pub active_overlays: Vec, pub recursion_count: Box, pub debug_depth: i64, + pub debug_output: Vec, } impl Stack { @@ -47,6 +48,7 @@ impl Stack { active_overlays: vec![DEFAULT_OVERLAY_NAME.to_string()], recursion_count: Box::new(0), debug_depth: 0, + debug_output: vec![], } } @@ -129,6 +131,7 @@ impl Stack { active_overlays: self.active_overlays.clone(), recursion_count: self.recursion_count.to_owned(), debug_depth: self.debug_depth, + debug_output: self.debug_output.clone(), } } @@ -155,6 +158,7 @@ impl Stack { active_overlays: self.active_overlays.clone(), recursion_count: self.recursion_count.to_owned(), debug_depth: self.debug_depth, + debug_output: self.debug_output.clone(), } } From 4b28a18ca13311d0e3ea3705eff666146992d16c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Wed, 8 Feb 2023 00:30:56 +0200 Subject: [PATCH 06/17] Use pipeline metadata for storing profiling data --- .../nu-command/src/core_commands/metadata.rs | 12 +++ crates/nu-command/src/experimental/profile.rs | 50 ++++++++----- crates/nu-engine/src/eval.rs | 73 +++++++++++++------ crates/nu-protocol/src/engine/stack.rs | 4 - crates/nu-protocol/src/pipeline_data.rs | 10 +++ 5 files changed, 106 insertions(+), 43 deletions(-) diff --git a/crates/nu-command/src/core_commands/metadata.rs b/crates/nu-command/src/core_commands/metadata.rs index 3f60b15cd9753..fa8e0674ce24a 100644 --- a/crates/nu-command/src/core_commands/metadata.rs +++ b/crates/nu-command/src/core_commands/metadata.rs @@ -89,6 +89,12 @@ impl Command for Metadata { cols.push("source".into()); vals.push(Value::string("into html --list", head)) } + PipelineMetadata { + data_source: DataSource::Profiling(values), + } => { + cols.push("profiling".into()); + vals.push(Value::list(values.clone(), head)) + } } } @@ -154,6 +160,12 @@ fn build_metadata_record(arg: &Value, metadata: &Option, head: cols.push("source".into()); vals.push(Value::string("into html --list", head)) } + PipelineMetadata { + data_source: DataSource::Profiling(values), + } => { + cols.push("profiling".into()); + vals.push(Value::list(values.clone(), head)) + } } } diff --git a/crates/nu-command/src/experimental/profile.rs b/crates/nu-command/src/experimental/profile.rs index 82da6614b701d..88eb723b88b4f 100644 --- a/crates/nu-command/src/experimental/profile.rs +++ b/crates/nu-command/src/experimental/profile.rs @@ -2,8 +2,8 @@ use nu_engine::{eval_block, CallExt}; use nu_protocol::ast::Call; use nu_protocol::engine::{Closure, Command, EngineState, Stack}; use nu_protocol::{ - Category, Example, IntoPipelineData, PipelineData, Signature, SyntaxShape, Type, ListStream, - IntoInterruptiblePipelineData, Value + Category, DataSource, Example, IntoPipelineData, PipelineData, PipelineMetadata, Signature, + Spanned, SyntaxShape, Type, Value, }; #[derive(Clone)] @@ -46,19 +46,14 @@ impl Command for Profile { call: &Call, input: PipelineData, ) -> Result { - let capture_block: Closure = call.req(engine_state, stack, 0)?; - let block = engine_state.get_block(capture_block.block_id); + let capture_block: Spanned = call.req(engine_state, stack, 0)?; + let block = engine_state.get_block(capture_block.item.block_id); let redirect_stdout = call.redirect_stdout; let redirect_stderr = call.redirect_stderr; - let mut stack = stack.captures_to_stack(&capture_block.captures); + let mut stack = stack.captures_to_stack(&capture_block.item.captures); - // In order to provide the pipeline as a positional, it must be converted into a value. - // But because pipelines do not have Clone, this one has to be cloned as a value - // and then converted back into a pipeline for eval_block(). - // So, the metadata must be saved here and restored at that point. - let input_metadata = input.metadata(); let input_val = input.into_value(call.head); if let Some(var) = block.signature.get_positional(0) { @@ -67,7 +62,6 @@ impl Command for Profile { } } - // Get the start time after all other computation has been done. stack.debug_depth = if let Some(depth) = call.get_flag::(engine_state, &mut stack, "max-depth")? { depth @@ -75,19 +69,39 @@ impl Command for Profile { 1 }; - let _ = eval_block( + let profiling_metadata = PipelineMetadata { + data_source: DataSource::Profiling(vec![]), + }; + + // let result = eval_block( + // engine_state, + // &mut stack, + // block, + // input_val.into_pipeline_data_with_metadata(profiling_metadata), + // redirect_stdout, + // redirect_stderr, + // )? + // .metadata(); + // println!("GOT: {:?}", result); + + let result = if let Some(PipelineMetadata { + data_source: DataSource::Profiling(values), + }) = eval_block( engine_state, &mut stack, block, - input_val.into_pipeline_data_with_metadata(input_metadata), + input_val.into_pipeline_data_with_metadata(profiling_metadata), redirect_stdout, redirect_stderr, )? - .into_value(call.head); - - let debug_output = Value::list(stack.debug_output.drain(..).collect(), call.head); - - Ok(debug_output.into_pipeline_data()) + .metadata() + { + Value::list(values, call.head) + } else { + Value::nothing(call.head) + }; + + Ok(result.into_pipeline_data()) } fn examples(&self) -> Vec { diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index dad939ff84b40..223295e3d9e4f 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -6,8 +6,8 @@ use nu_protocol::{ Operator, PathMember, PipelineElement, Redirection, }, engine::{EngineState, Stack}, - Config, IntoInterruptiblePipelineData, IntoPipelineData, ListStream, PipelineData, Range, - ShellError, Span, Spanned, Unit, Value, VarId, ENV_VARIABLE_ID, + Config, DataSource, IntoInterruptiblePipelineData, IntoPipelineData, PipelineData, + PipelineMetadata, Range, ShellError, Span, Spanned, Unit, Value, VarId, ENV_VARIABLE_ID, }; use nu_utils::stdout_write_all_and_flush; use std::collections::HashMap; @@ -695,7 +695,9 @@ pub fn eval_expression_with_input( } elem => { - input = eval_expression(engine_state, stack, elem)?.into_pipeline_data(); + let input_metadata = input.metadata(); + input = eval_expression(engine_state, stack, elem)? + .into_pipeline_data_with_metadata(input_metadata); } }; @@ -998,10 +1000,10 @@ pub fn eval_block( false }; - // let mut debug_out = vec![]; - let num_pipelines = block.len(); + let mut input_metadata = input.metadata(); + for (pipeline_idx, pipeline) in block.pipelines.iter().enumerate() { let mut i = 0; @@ -1043,37 +1045,65 @@ pub fn eval_block( ), span, ); - let value = match &eval_result { - Ok((PipelineData::Value(val, ..), ..)) => val.clone(), - Ok((PipelineData::ListStream(..), ..)) => Value::string("list stream", span), - Ok((PipelineData::ExternalStream { .. }, ..)) => { - Value::string("raw stream", span) + let (value, element_metadata) = match &eval_result { + Ok((PipelineData::Value(val, metadata), ..)) => (val.clone(), metadata.clone()), + Ok((PipelineData::ListStream(.., metadata), ..)) => { + (Value::string("list stream", span), metadata.clone()) + } + Ok((PipelineData::ExternalStream { metadata, .. }, ..)) => { + (Value::string("raw stream", span), metadata.clone()) } - Ok((PipelineData::Empty, ..)) => Value::Nothing { span }, - Err(err) => Value::Error { error: err.clone() }, + Ok((PipelineData::Empty, ..)) => (Value::Nothing { span }, None), + Err(err) => (Value::Error { error: err.clone() }, None), }; let ns = (end_time - start_time).as_nanos() as i64; - stack.debug_output.push(Value::Record { + let record = Value::Record { cols: vec![ - "level".to_string(), + "pipeline_idx".to_string(), + "element_idx".to_string(), + "depth".to_string(), "source".to_string(), "value".to_string(), "ns".to_string(), ], vals: vec![ + Value::int(pipeline_idx as i64, span), + Value::int(i as i64, span), Value::int(stack.debug_depth + 1, span), - element_str, + element_str.clone(), value, Value::Duration { val: ns, span }, ], span, - }); - // println!( - // "\x1b[32m{}\x1b[0m : \x1b[33m{}\x1b[0m", - // element_str, - // format_duration((end_time - start_time).as_nanos() as i64) - // ); + }; + + if let Some(PipelineMetadata { + data_source: DataSource::Profiling(vals), + }) = element_metadata + { + if let Some(PipelineMetadata { + data_source: DataSource::Profiling(tgt_vals), + }) = &mut input_metadata + { + tgt_vals.extend(vals); + } else { + input_metadata = Some(PipelineMetadata { + data_source: DataSource::Profiling(vals), + }); + } + } + + if let Some(PipelineMetadata { + data_source: DataSource::Profiling(tgt_vals), + }) = &mut input_metadata + { + tgt_vals.push(record); + } else { + input_metadata = Some(PipelineMetadata { + data_source: DataSource::Profiling(vec![record]), + }); + } } match (eval_result, redirect_stderr) { @@ -1175,6 +1205,7 @@ pub fn eval_block( } } + input = input.set_metadata(input_metadata); Ok(input) } diff --git a/crates/nu-protocol/src/engine/stack.rs b/crates/nu-protocol/src/engine/stack.rs index a7af48f5eef92..1f59c3f9ad69e 100644 --- a/crates/nu-protocol/src/engine/stack.rs +++ b/crates/nu-protocol/src/engine/stack.rs @@ -36,7 +36,6 @@ pub struct Stack { pub active_overlays: Vec, pub recursion_count: Box, pub debug_depth: i64, - pub debug_output: Vec, } impl Stack { @@ -48,7 +47,6 @@ impl Stack { active_overlays: vec![DEFAULT_OVERLAY_NAME.to_string()], recursion_count: Box::new(0), debug_depth: 0, - debug_output: vec![], } } @@ -131,7 +129,6 @@ impl Stack { active_overlays: self.active_overlays.clone(), recursion_count: self.recursion_count.to_owned(), debug_depth: self.debug_depth, - debug_output: self.debug_output.clone(), } } @@ -158,7 +155,6 @@ impl Stack { active_overlays: self.active_overlays.clone(), recursion_count: self.recursion_count.to_owned(), debug_depth: self.debug_depth, - debug_output: self.debug_output.clone(), } } diff --git a/crates/nu-protocol/src/pipeline_data.rs b/crates/nu-protocol/src/pipeline_data.rs index d3b4eae8aa0b1..1950ce0d8b577 100644 --- a/crates/nu-protocol/src/pipeline_data.rs +++ b/crates/nu-protocol/src/pipeline_data.rs @@ -62,6 +62,7 @@ pub struct PipelineMetadata { pub enum DataSource { Ls, HtmlThemes, + Profiling(Vec), } impl PipelineData { @@ -82,6 +83,15 @@ impl PipelineData { } } + pub fn metadata_mut(&mut self) -> Option<&mut PipelineMetadata> { + match self { + PipelineData::ListStream(_, x) => x.as_mut(), + PipelineData::ExternalStream { metadata: x, .. } => x.as_mut(), + PipelineData::Value(_, x) => x.as_mut(), + PipelineData::Empty => None, + } + } + pub fn set_metadata(mut self, metadata: Option) -> Self { match &mut self { PipelineData::ListStream(_, x) => *x = metadata, From 54265b90703104571b58d415295d6b6d81d4bed7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Wed, 8 Feb 2023 22:30:38 +0200 Subject: [PATCH 07/17] Remove redundant metadata setting Did not have any effect --- crates/nu-engine/src/eval.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index 223295e3d9e4f..4e10b55ec8a0d 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -695,9 +695,7 @@ pub fn eval_expression_with_input( } elem => { - let input_metadata = input.metadata(); - input = eval_expression(engine_state, stack, elem)? - .into_pipeline_data_with_metadata(input_metadata); + input = eval_expression(engine_state, stack, elem)?.into_pipeline_data(); } }; From 8198bab020b9279dae63ff388d1dd8f30d545295 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Wed, 8 Feb 2023 22:33:22 +0200 Subject: [PATCH 08/17] Remove unused method --- crates/nu-protocol/src/pipeline_data.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/crates/nu-protocol/src/pipeline_data.rs b/crates/nu-protocol/src/pipeline_data.rs index 1950ce0d8b577..87e5cb6be69b2 100644 --- a/crates/nu-protocol/src/pipeline_data.rs +++ b/crates/nu-protocol/src/pipeline_data.rs @@ -83,15 +83,6 @@ impl PipelineData { } } - pub fn metadata_mut(&mut self) -> Option<&mut PipelineMetadata> { - match self { - PipelineData::ListStream(_, x) => x.as_mut(), - PipelineData::ExternalStream { metadata: x, .. } => x.as_mut(), - PipelineData::Value(_, x) => x.as_mut(), - PipelineData::Empty => None, - } - } - pub fn set_metadata(mut self, metadata: Option) -> Self { match &mut self { PipelineData::ListStream(_, x) => *x = metadata, From cc2b2c8176555756e4fd551bf5964c360bd92372 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Thu, 9 Feb 2023 00:22:04 +0200 Subject: [PATCH 09/17] Make source and value optional; Add element spans --- crates/nu-command/src/experimental/profile.rs | 28 +++--- crates/nu-engine/src/eval.rs | 85 ++++++++++++------- crates/nu-protocol/src/engine/stack.rs | 19 ++++- 3 files changed, 79 insertions(+), 53 deletions(-) diff --git a/crates/nu-command/src/experimental/profile.rs b/crates/nu-command/src/experimental/profile.rs index 88eb723b88b4f..4d06f2ac6be11 100644 --- a/crates/nu-command/src/experimental/profile.rs +++ b/crates/nu-command/src/experimental/profile.rs @@ -1,6 +1,6 @@ use nu_engine::{eval_block, CallExt}; use nu_protocol::ast::Call; -use nu_protocol::engine::{Closure, Command, EngineState, Stack}; +use nu_protocol::engine::{Closure, Command, EngineState, ProfilingConfig, Stack}; use nu_protocol::{ Category, DataSource, Example, IntoPipelineData, PipelineData, PipelineMetadata, Signature, Spanned, SyntaxShape, Type, Value, @@ -25,6 +25,8 @@ impl Command for Profile { SyntaxShape::Closure(Some(vec![SyntaxShape::Any])), "the closure to run", ) + .switch("source", "Collect source code in the report", None) + .switch("values", "Collect values in the report", None) .named( "max-depth", SyntaxShape::Int, @@ -62,28 +64,18 @@ impl Command for Profile { } } - stack.debug_depth = - if let Some(depth) = call.get_flag::(engine_state, &mut stack, "max-depth")? { - depth - } else { - 1 - }; + stack.profiling_config = ProfilingConfig { + depth: call + .get_flag::(engine_state, &mut stack, "max-depth")? + .unwrap_or(1), + collect_source: call.has_flag("source"), + collect_values: call.has_flag("values"), + }; let profiling_metadata = PipelineMetadata { data_source: DataSource::Profiling(vec![]), }; - // let result = eval_block( - // engine_state, - // &mut stack, - // block, - // input_val.into_pipeline_data_with_metadata(profiling_metadata), - // redirect_stdout, - // redirect_stderr, - // )? - // .metadata(); - // println!("GOT: {:?}", result); - let result = if let Some(PipelineMetadata { data_source: DataSource::Profiling(values), }) = eval_block( diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index 4e10b55ec8a0d..2e3f1ee39f234 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -991,8 +991,8 @@ pub fn eval_block( } } - let should_debug = if stack.debug_depth > 0 { - stack.debug_depth -= 1; + let should_debug = if stack.profiling_config.depth > 0 { + stack.profiling_config.depth -= 1; true } else { false @@ -1043,37 +1043,60 @@ pub fn eval_block( ), span, ); - let (value, element_metadata) = match &eval_result { - Ok((PipelineData::Value(val, metadata), ..)) => (val.clone(), metadata.clone()), - Ok((PipelineData::ListStream(.., metadata), ..)) => { - (Value::string("list stream", span), metadata.clone()) - } - Ok((PipelineData::ExternalStream { metadata, .. }, ..)) => { - (Value::string("raw stream", span), metadata.clone()) - } - Ok((PipelineData::Empty, ..)) => (Value::Nothing { span }, None), - Err(err) => (Value::Error { error: err.clone() }, None), - }; let ns = (end_time - start_time).as_nanos() as i64; - let record = Value::Record { - cols: vec![ - "pipeline_idx".to_string(), - "element_idx".to_string(), - "depth".to_string(), - "source".to_string(), - "value".to_string(), - "ns".to_string(), - ], - vals: vec![ - Value::int(pipeline_idx as i64, span), - Value::int(i as i64, span), - Value::int(stack.debug_depth + 1, span), - element_str.clone(), - value, - Value::Duration { val: ns, span }, - ], - span, + let mut cols = vec![ + "pipeline_idx".to_string(), + "element_idx".to_string(), + "depth".to_string(), + "span".to_string(), + ]; + + let mut vals = vec![ + Value::int(pipeline_idx as i64, span), + Value::int(i as i64, span), + Value::int(stack.profiling_config.depth + 1, span), + Value::record( + vec!["start".to_string(), "end".to_string()], + vec![ + Value::int(span.start as i64, span), + Value::int(span.end as i64, span), + ], + span, + ), + ]; + + if stack.profiling_config.collect_source { + cols.push("source".to_string()); + vals.push(element_str.clone()); + } + + if stack.profiling_config.collect_values { + let value = match &eval_result { + Ok((PipelineData::Value(val, ..), ..)) => val.clone(), + Ok((PipelineData::ListStream(..), ..)) => { + Value::string("list stream", span) + } + Ok((PipelineData::ExternalStream { .. }, ..)) => { + Value::string("raw stream", span) + } + Ok((PipelineData::Empty, ..)) => Value::Nothing { span }, + Err(err) => Value::Error { error: err.clone() }, + }; + + cols.push("value".to_string()); + vals.push(value); + } + + cols.push("ns".to_string()); + vals.push(Value::Duration { val: ns, span }); + + let record = Value::Record { cols, vals, span }; + + let element_metadata = if let Ok((pipeline_data, ..)) = &eval_result { + pipeline_data.metadata() + } else { + None }; if let Some(PipelineMetadata { diff --git a/crates/nu-protocol/src/engine/stack.rs b/crates/nu-protocol/src/engine/stack.rs index 1f59c3f9ad69e..0a0930c9bba0b 100644 --- a/crates/nu-protocol/src/engine/stack.rs +++ b/crates/nu-protocol/src/engine/stack.rs @@ -7,6 +7,13 @@ use crate::{ShellError, Span, Value, VarId}; /// Environment variables per overlay pub type EnvVars = HashMap>; +#[derive(Debug, Clone)] +pub struct ProfilingConfig { + pub depth: i64, + pub collect_source: bool, + pub collect_values: bool, +} + /// A runtime value stack used during evaluation /// /// A note on implementation: @@ -35,7 +42,7 @@ pub struct Stack { /// List of active overlays pub active_overlays: Vec, pub recursion_count: Box, - pub debug_depth: i64, + pub profiling_config: ProfilingConfig, } impl Stack { @@ -46,7 +53,11 @@ impl Stack { env_hidden: HashMap::new(), active_overlays: vec![DEFAULT_OVERLAY_NAME.to_string()], recursion_count: Box::new(0), - debug_depth: 0, + profiling_config: ProfilingConfig { + depth: 0, + collect_source: false, + collect_values: false, + }, } } @@ -128,7 +139,7 @@ impl Stack { env_hidden: HashMap::new(), active_overlays: self.active_overlays.clone(), recursion_count: self.recursion_count.to_owned(), - debug_depth: self.debug_depth, + profiling_config: self.profiling_config.clone(), } } @@ -154,7 +165,7 @@ impl Stack { env_hidden: HashMap::new(), active_overlays: self.active_overlays.clone(), recursion_count: self.recursion_count.to_owned(), - debug_depth: self.debug_depth, + profiling_config: self.profiling_config.clone(), } } From 83063ad40928441002ee31628f37231900287990 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Fri, 10 Feb 2023 00:08:19 +0200 Subject: [PATCH 10/17] Fix profiling config order and depth; Misc tweaks --- crates/nu-command/src/experimental/profile.rs | 11 ++- crates/nu-engine/src/eval.rs | 68 +++++++++++-------- crates/nu-protocol/src/engine/stack.rs | 37 ++++++++-- 3 files changed, 78 insertions(+), 38 deletions(-) diff --git a/crates/nu-command/src/experimental/profile.rs b/crates/nu-command/src/experimental/profile.rs index 4d06f2ac6be11..3080790674a3f 100644 --- a/crates/nu-command/src/experimental/profile.rs +++ b/crates/nu-command/src/experimental/profile.rs @@ -64,13 +64,12 @@ impl Command for Profile { } } - stack.profiling_config = ProfilingConfig { - depth: call - .get_flag::(engine_state, &mut stack, "max-depth")? + stack.profiling_config = ProfilingConfig::new( + call.get_flag::(engine_state, &mut stack, "max-depth")? .unwrap_or(1), - collect_source: call.has_flag("source"), - collect_values: call.has_flag("values"), - }; + call.has_flag("source"), + call.has_flag("values"), + ); let profiling_metadata = PipelineMetadata { data_source: DataSource::Profiling(vec![]), diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index 2e3f1ee39f234..ef80f7f713eb8 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -991,12 +991,13 @@ pub fn eval_block( } } - let should_debug = if stack.profiling_config.depth > 0 { - stack.profiling_config.depth -= 1; - true - } else { - false - }; + stack.profiling_config.enter_block(); + // let should_debug = if stack.profiling_config.depth > 0 { + // stack.profiling_config.depth -= 1; + // true + // } else { + // false + // }; let num_pipelines = block.len(); @@ -1015,8 +1016,13 @@ pub fn eval_block( | PipelineElement::SeparateRedirection { .. } ))); + let start_time = if stack.profiling_config.should_debug() { + Some(Instant::now()) + } else { + None + }; + // if eval internal command failed, it can just make early return with `Err(ShellError)`. - let start_time = Instant::now(); let eval_result = eval_element_with_input( engine_state, stack, @@ -1033,9 +1039,14 @@ pub fn eval_block( )), redirect_stderr, ); - let end_time = Instant::now(); - if should_debug { + let end_time = if stack.profiling_config.should_debug() { + Some(Instant::now()) + } else { + None + }; + + if let (Some(start_time), Some(end_time)) = (start_time, end_time) { let span = pipeline.elements[i].span(); let element_str = Value::string( String::from_utf8_lossy( @@ -1043,7 +1054,7 @@ pub fn eval_block( ), span, ); - let ns = (end_time - start_time).as_nanos() as i64; + let time_ns = (end_time - start_time).as_nanos() as i64; let mut cols = vec![ "pipeline_idx".to_string(), @@ -1055,7 +1066,7 @@ pub fn eval_block( let mut vals = vec![ Value::int(pipeline_idx as i64, span), Value::int(i as i64, span), - Value::int(stack.profiling_config.depth + 1, span), + Value::int(stack.profiling_config.depth, span), Value::record( vec!["start".to_string(), "end".to_string()], vec![ @@ -1088,8 +1099,8 @@ pub fn eval_block( vals.push(value); } - cols.push("ns".to_string()); - vals.push(Value::Duration { val: ns, span }); + cols.push("time".to_string()); + vals.push(Value::Duration { val: time_ns, span }); let record = Value::Record { cols, vals, span }; @@ -1100,31 +1111,31 @@ pub fn eval_block( }; if let Some(PipelineMetadata { - data_source: DataSource::Profiling(vals), + data_source: DataSource::Profiling(tgt_vals), + }) = &mut input_metadata + { + tgt_vals.push(record); + } else { + input_metadata = Some(PipelineMetadata { + data_source: DataSource::Profiling(vec![record]), + }); + } + + if let Some(PipelineMetadata { + data_source: DataSource::Profiling(element_vals), }) = element_metadata { if let Some(PipelineMetadata { data_source: DataSource::Profiling(tgt_vals), }) = &mut input_metadata { - tgt_vals.extend(vals); + tgt_vals.extend(element_vals); } else { input_metadata = Some(PipelineMetadata { - data_source: DataSource::Profiling(vals), + data_source: DataSource::Profiling(element_vals), }); } } - - if let Some(PipelineMetadata { - data_source: DataSource::Profiling(tgt_vals), - }) = &mut input_metadata - { - tgt_vals.push(record); - } else { - input_metadata = Some(PipelineMetadata { - data_source: DataSource::Profiling(vec![record]), - }); - } } match (eval_result, redirect_stderr) { @@ -1143,6 +1154,7 @@ pub fn eval_block( // make early return so remaining commands will not be executed. // don't return `Err(ShellError)`, so nushell wouldn't show extra error message. if output.1 { + stack.profiling_config.leave_block(); return Ok(input); } } @@ -1226,6 +1238,8 @@ pub fn eval_block( } } + stack.profiling_config.leave_block(); + input = input.set_metadata(input_metadata); Ok(input) } diff --git a/crates/nu-protocol/src/engine/stack.rs b/crates/nu-protocol/src/engine/stack.rs index 0a0930c9bba0b..0f43218630e66 100644 --- a/crates/nu-protocol/src/engine/stack.rs +++ b/crates/nu-protocol/src/engine/stack.rs @@ -9,11 +9,42 @@ pub type EnvVars = HashMap>; #[derive(Debug, Clone)] pub struct ProfilingConfig { + pub max_depth: i64, pub depth: i64, pub collect_source: bool, pub collect_values: bool, } +impl ProfilingConfig { + pub fn new(max_depth: i64, collect_source: bool, collect_values: bool) -> Self { + ProfilingConfig { + max_depth, + depth: 0, + collect_source, + collect_values, + } + } + + pub fn enter_block(&mut self) { + self.depth += 1; + } + + pub fn leave_block(&mut self) { + self.depth -= 1; + } + + pub fn should_debug(&self) -> bool { + self.depth <= self.max_depth + } + + pub fn reset(&mut self) { + self.max_depth = 0; + self.depth = 0; + self.collect_source = false; + self.collect_values = false; + } +} + /// A runtime value stack used during evaluation /// /// A note on implementation: @@ -53,11 +84,7 @@ impl Stack { env_hidden: HashMap::new(), active_overlays: vec![DEFAULT_OVERLAY_NAME.to_string()], recursion_count: Box::new(0), - profiling_config: ProfilingConfig { - depth: 0, - collect_source: false, - collect_values: false, - }, + profiling_config: ProfilingConfig::new(0, false, false), } } From ff1d7a7c4052fa8561f2e951424ca272049605cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Fri, 10 Feb 2023 22:39:10 +0200 Subject: [PATCH 11/17] Add help and examples for profile command --- crates/nu-command/src/experimental/profile.rs | 54 +++++++------------ 1 file changed, 18 insertions(+), 36 deletions(-) diff --git a/crates/nu-command/src/experimental/profile.rs b/crates/nu-command/src/experimental/profile.rs index a0d47f7c38079..f71188e57953f 100644 --- a/crates/nu-command/src/experimental/profile.rs +++ b/crates/nu-command/src/experimental/profile.rs @@ -15,7 +15,17 @@ impl Command for Profile { } fn usage(&self) -> &str { - "Time the running time of a closure" + "Profile each pipeline element in a closure." + } + + fn extra_usage(&self) -> &str { + r#"The command collects run time of every pipeline element, recursively stepping into child closures +until a maximum depth. Optionally, it also collects the source code and intermediate values. + +Current known limitations are: +* profiling data from subexpressions is not tracked +* it does not step into loop iterations +* other cases involving closure execution (e.g., `do { ... }`) might not be tracked."# } fn signature(&self) -> nu_protocol::Signature { @@ -33,10 +43,7 @@ impl Command for Profile { "How many levels of blocks to step into (default: 1)", Some('d'), ) - .input_output_types(vec![ - (Type::Any, Type::Duration), - (Type::Nothing, Type::Duration), - ]) + .input_output_types(vec![(Type::Any, Type::Table(vec![]))]) .allow_variants_without_examples(true) .category(Category::Debug) } @@ -96,36 +103,11 @@ impl Command for Profile { } fn examples(&self) -> Vec { - vec![] + vec![Example { + description: + "Profile some code, stepping into the `spam` command and collecting source.", + example: r#"def spam [] { "spam" }; profile { spam | str length } -d 2 --source"#, + result: None, + }] } } - -#[test] -// Due to difficulty in observing side-effects from benchmark closures, -// checks that the closures have run correctly must use the filesystem. -fn test_benchmark_closure() { - use nu_test_support::{nu, nu_repl_code, playground::Playground}; - Playground::setup("test_benchmark_closure", |dirs, _| { - let inp = [ - r#"[2 3 4] | profile { to nuon | save foo.txt }"#, - "open foo.txt", - ]; - let actual_repl = nu!(cwd: dirs.test(), nu_repl_code(&inp)); - assert_eq!(actual_repl.err, ""); - assert_eq!(actual_repl.out, "[2, 3, 4]"); - }); -} - -#[test] -fn test_benchmark_closure_2() { - use nu_test_support::{nu, nu_repl_code, playground::Playground}; - Playground::setup("test_benchmark_closure", |dirs, _| { - let inp = [ - r#"[2 3 4] | profile {|e| {result: $e} | to nuon | save foo.txt }"#, - "open foo.txt", - ]; - let actual_repl = nu!(cwd: dirs.test(), nu_repl_code(&inp)); - assert_eq!(actual_repl.err, ""); - assert_eq!(actual_repl.out, "{result: [2, 3, 4]}"); - }); -} From 378e4fa2df1705115b9b5f7a5d13b0fc3f842885 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Fri, 10 Feb 2023 22:45:50 +0200 Subject: [PATCH 12/17] Add bach accidentally removed command --- crates/nu-command/src/default_context.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/nu-command/src/default_context.rs b/crates/nu-command/src/default_context.rs index ed60eb378ceb0..89801084b0a19 100644 --- a/crates/nu-command/src/default_context.rs +++ b/crates/nu-command/src/default_context.rs @@ -478,6 +478,7 @@ pub fn create_default_context() -> EngineState { View, ViewFiles, ViewSource, + ViewSpan, }; // Deprecated From e83b7af45d500db7ca7e0fd401f96748e1be1c3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Fri, 10 Feb 2023 22:54:00 +0200 Subject: [PATCH 13/17] Remove comments --- crates/nu-engine/src/eval.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index ef80f7f713eb8..6746a06062122 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -992,12 +992,6 @@ pub fn eval_block( } stack.profiling_config.enter_block(); - // let should_debug = if stack.profiling_config.depth > 0 { - // stack.profiling_config.depth -= 1; - // true - // } else { - // false - // }; let num_pipelines = block.len(); From cc10e2d53e7ceb054bd6bfcc8e33d48483dd333a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Sat, 11 Feb 2023 22:06:46 +0200 Subject: [PATCH 14/17] Do not set metadata without profiling --- crates/nu-engine/src/eval.rs | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index 6746a06062122..d536041b8015b 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -995,7 +995,11 @@ pub fn eval_block( let num_pipelines = block.len(); - let mut input_metadata = input.metadata(); + let mut input_metadata = if stack.profiling_config.should_debug() { + input.metadata() + } else { + None + }; for (pipeline_idx, pipeline) in block.pipelines.iter().enumerate() { let mut i = 0; @@ -1040,7 +1044,9 @@ pub fn eval_block( None }; - if let (Some(start_time), Some(end_time)) = (start_time, end_time) { + if let (Some(start_time), Some(end_time), Some(input_metadata)) = + (start_time, end_time, &mut input_metadata) + { let span = pipeline.elements[i].span(); let element_str = Value::string( String::from_utf8_lossy( @@ -1104,30 +1110,30 @@ pub fn eval_block( None }; - if let Some(PipelineMetadata { + if let PipelineMetadata { data_source: DataSource::Profiling(tgt_vals), - }) = &mut input_metadata + } = input_metadata { tgt_vals.push(record); } else { - input_metadata = Some(PipelineMetadata { + *input_metadata = PipelineMetadata { data_source: DataSource::Profiling(vec![record]), - }); + }; } if let Some(PipelineMetadata { data_source: DataSource::Profiling(element_vals), }) = element_metadata { - if let Some(PipelineMetadata { + if let PipelineMetadata { data_source: DataSource::Profiling(tgt_vals), - }) = &mut input_metadata + } = input_metadata { tgt_vals.extend(element_vals); } else { - input_metadata = Some(PipelineMetadata { + *input_metadata = PipelineMetadata { data_source: DataSource::Profiling(element_vals), - }); + }; } } } @@ -1234,8 +1240,11 @@ pub fn eval_block( stack.profiling_config.leave_block(); - input = input.set_metadata(input_metadata); - Ok(input) + if stack.profiling_config.should_debug() { + Ok(input.set_metadata(input_metadata)) + } else { + Ok(input) + } } fn print_or_return(pipeline_data: PipelineData, config: &Config) -> Result<(), ShellError> { From 4b02bb19db3ba9e97e93fca0473c34835a4e3ddd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Sat, 11 Feb 2023 22:21:18 +0200 Subject: [PATCH 15/17] Enter/leave block only when profiling --- crates/nu-engine/src/eval.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index d536041b8015b..7df64b366e6de 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -991,11 +991,10 @@ pub fn eval_block( } } - stack.profiling_config.enter_block(); - let num_pipelines = block.len(); let mut input_metadata = if stack.profiling_config.should_debug() { + stack.profiling_config.enter_block(); input.metadata() } else { None @@ -1154,7 +1153,9 @@ pub fn eval_block( // make early return so remaining commands will not be executed. // don't return `Err(ShellError)`, so nushell wouldn't show extra error message. if output.1 { - stack.profiling_config.leave_block(); + if stack.profiling_config.should_debug() { + stack.profiling_config.leave_block(); + } return Ok(input); } } @@ -1238,9 +1239,8 @@ pub fn eval_block( } } - stack.profiling_config.leave_block(); - if stack.profiling_config.should_debug() { + stack.profiling_config.leave_block(); Ok(input.set_metadata(input_metadata)) } else { Ok(input) From d04176b4e4612a4027c66a692c78d1a7bf9aad85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Sat, 11 Feb 2023 23:05:42 +0200 Subject: [PATCH 16/17] Box the metadata --- .../nu-command/src/core_commands/metadata.rs | 10 +++++--- crates/nu-command/src/experimental/profile.rs | 8 +++---- crates/nu-command/src/filesystem/ls.rs | 4 ++-- crates/nu-command/src/filters/uniq.rs | 2 +- crates/nu-command/src/formats/to/html.rs | 4 ++-- crates/nu-command/src/viewers/table.rs | 4 ++-- crates/nu-engine/src/eval.rs | 4 ++-- crates/nu-explore/src/nu_common/value.rs | 2 +- crates/nu-protocol/src/pipeline_data.rs | 24 ++++++++++--------- 9 files changed, 34 insertions(+), 28 deletions(-) diff --git a/crates/nu-command/src/core_commands/metadata.rs b/crates/nu-command/src/core_commands/metadata.rs index d89d0403d36af..9799c011798e8 100644 --- a/crates/nu-command/src/core_commands/metadata.rs +++ b/crates/nu-command/src/core_commands/metadata.rs @@ -75,7 +75,7 @@ impl Command for Metadata { None => { let mut cols = vec![]; let mut vals = vec![]; - if let Some(x) = &input.metadata() { + if let Some(x) = input.metadata().as_deref() { match x { PipelineMetadata { data_source: DataSource::Ls, @@ -124,7 +124,11 @@ impl Command for Metadata { } } -fn build_metadata_record(arg: &Value, metadata: &Option, head: Span) -> Value { +fn build_metadata_record( + arg: &Value, + metadata: &Option>, + head: Span, +) -> Value { let mut cols = vec![]; let mut vals = vec![]; @@ -146,7 +150,7 @@ fn build_metadata_record(arg: &Value, metadata: &Option, head: }); } - if let Some(x) = &metadata { + if let Some(x) = metadata.as_deref() { match x { PipelineMetadata { data_source: DataSource::Ls, diff --git a/crates/nu-command/src/experimental/profile.rs b/crates/nu-command/src/experimental/profile.rs index f71188e57953f..19d8334773f6c 100644 --- a/crates/nu-command/src/experimental/profile.rs +++ b/crates/nu-command/src/experimental/profile.rs @@ -24,8 +24,7 @@ until a maximum depth. Optionally, it also collects the source code and intermed Current known limitations are: * profiling data from subexpressions is not tracked -* it does not step into loop iterations -* other cases involving closure execution (e.g., `do { ... }`) might not be tracked."# +* it does not step into loop iterations"# } fn signature(&self) -> nu_protocol::Signature { @@ -78,9 +77,9 @@ Current known limitations are: call.has_flag("values"), ); - let profiling_metadata = PipelineMetadata { + let profiling_metadata = Box::new(PipelineMetadata { data_source: DataSource::Profiling(vec![]), - }; + }); let result = if let Some(PipelineMetadata { data_source: DataSource::Profiling(values), @@ -93,6 +92,7 @@ Current known limitations are: redirect_stderr, )? .metadata() + .map(|m| *m) { Value::list(values, call.head) } else { diff --git a/crates/nu-command/src/filesystem/ls.rs b/crates/nu-command/src/filesystem/ls.rs index 97a079994509d..5ee4ee990347c 100644 --- a/crates/nu-command/src/filesystem/ls.rs +++ b/crates/nu-command/src/filesystem/ls.rs @@ -264,9 +264,9 @@ impl Command for Ls { _ => Some(Value::Nothing { span: call_span }), }) .into_pipeline_data_with_metadata( - PipelineMetadata { + Box::new(PipelineMetadata { data_source: DataSource::Ls, - }, + }), engine_state.ctrlc.clone(), )) } diff --git a/crates/nu-command/src/filters/uniq.rs b/crates/nu-command/src/filters/uniq.rs index af527b9887113..f35be77fe0a17 100644 --- a/crates/nu-command/src/filters/uniq.rs +++ b/crates/nu-command/src/filters/uniq.rs @@ -264,7 +264,7 @@ pub fn uniq( call: &Call, input: Vec, item_mapper: Box ValueCounter>, - metadata: Option, + metadata: Option>, ) -> Result { let ctrlc = engine_state.ctrlc.clone(); let head = call.head; diff --git a/crates/nu-command/src/formats/to/html.rs b/crates/nu-command/src/formats/to/html.rs index b1ce78e6c20ba..1a8371b094641 100644 --- a/crates/nu-command/src/formats/to/html.rs +++ b/crates/nu-command/src/formats/to/html.rs @@ -320,9 +320,9 @@ fn to_html( vals: result, span: head, } - .into_pipeline_data_with_metadata(PipelineMetadata { + .into_pipeline_data_with_metadata(Box::new(PipelineMetadata { data_source: DataSource::HtmlThemes, - })); + }))); } else { let theme_span = match &theme { Some(v) => v.span, diff --git a/crates/nu-command/src/viewers/table.rs b/crates/nu-command/src/viewers/table.rs index 9694d1a71b5ff..71f533c85f945 100644 --- a/crates/nu-command/src/viewers/table.rs +++ b/crates/nu-command/src/viewers/table.rs @@ -612,9 +612,9 @@ fn handle_row_stream( call: &Call, row_offset: usize, ctrlc: Option>, - metadata: Option, + metadata: Option>, ) -> Result { - let stream = match metadata { + let stream = match metadata.as_deref() { // First, `ls` sources: Some(PipelineMetadata { data_source: DataSource::Ls, diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index 7df64b366e6de..05ed1b9912d47 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -1044,7 +1044,7 @@ pub fn eval_block( }; if let (Some(start_time), Some(end_time), Some(input_metadata)) = - (start_time, end_time, &mut input_metadata) + (start_time, end_time, input_metadata.as_deref_mut()) { let span = pipeline.elements[i].span(); let element_str = Value::string( @@ -1122,7 +1122,7 @@ pub fn eval_block( if let Some(PipelineMetadata { data_source: DataSource::Profiling(element_vals), - }) = element_metadata + }) = element_metadata.map(|m| *m) { if let PipelineMetadata { data_source: DataSource::Profiling(tgt_vals), diff --git a/crates/nu-explore/src/nu_common/value.rs b/crates/nu-explore/src/nu_common/value.rs index c9dd8201559cc..40956e61b969d 100644 --- a/crates/nu-explore/src/nu_common/value.rs +++ b/crates/nu-explore/src/nu_common/value.rs @@ -17,7 +17,7 @@ pub fn collect_pipeline(input: PipelineData) -> (Vec, Vec>) { metadata, span, .. - } => collect_external_stream(stdout, stderr, exit_code, metadata, span), + } => collect_external_stream(stdout, stderr, exit_code, metadata.map(|m| *m), span), } } diff --git a/crates/nu-protocol/src/pipeline_data.rs b/crates/nu-protocol/src/pipeline_data.rs index 925c2373cbde4..4ada6ba3ce75f 100644 --- a/crates/nu-protocol/src/pipeline_data.rs +++ b/crates/nu-protocol/src/pipeline_data.rs @@ -40,14 +40,14 @@ const LINE_ENDING_PATTERN: &[char] = &['\r', '\n']; /// Nushell. #[derive(Debug)] pub enum PipelineData { - Value(Value, Option), - ListStream(ListStream, Option), + Value(Value, Option>), + ListStream(ListStream, Option>), ExternalStream { stdout: Option, stderr: Option, exit_code: Option, span: Span, - metadata: Option, + metadata: Option>, trim_end_newline: bool, }, Empty, @@ -66,7 +66,7 @@ pub enum DataSource { } impl PipelineData { - pub fn new_with_metadata(metadata: Option, span: Span) -> PipelineData { + pub fn new_with_metadata(metadata: Option>, span: Span) -> PipelineData { PipelineData::Value(Value::Nothing { span }, metadata) } @@ -74,7 +74,7 @@ impl PipelineData { PipelineData::Empty } - pub fn metadata(&self) -> Option { + pub fn metadata(&self) -> Option> { match self { PipelineData::ListStream(_, x) => x.clone(), PipelineData::ExternalStream { metadata: x, .. } => x.clone(), @@ -83,7 +83,7 @@ impl PipelineData { } } - pub fn set_metadata(mut self, metadata: Option) -> Self { + pub fn set_metadata(mut self, metadata: Option>) -> Self { match &mut self { PipelineData::ListStream(_, x) => *x = metadata, PipelineData::ExternalStream { metadata: x, .. } => *x = metadata, @@ -285,7 +285,7 @@ impl PipelineData { pub fn collect_string_strict( self, span: Span, - ) -> Result<(String, Span, Option), ShellError> { + ) -> Result<(String, Span, Option>), ShellError> { match self { PipelineData::Empty => Ok((String::new(), span, None)), PipelineData::Value(Value::String { val, span }, metadata) => Ok((val, span, metadata)), @@ -810,9 +810,10 @@ impl Iterator for PipelineIterator { pub trait IntoPipelineData { fn into_pipeline_data(self) -> PipelineData; + fn into_pipeline_data_with_metadata( self, - metadata: impl Into>, + metadata: impl Into>>, ) -> PipelineData; } @@ -823,9 +824,10 @@ where fn into_pipeline_data(self) -> PipelineData { PipelineData::Value(self.into(), None) } + fn into_pipeline_data_with_metadata( self, - metadata: impl Into>, + metadata: impl Into>>, ) -> PipelineData { PipelineData::Value(self.into(), metadata.into()) } @@ -835,7 +837,7 @@ pub trait IntoInterruptiblePipelineData { fn into_pipeline_data(self, ctrlc: Option>) -> PipelineData; fn into_pipeline_data_with_metadata( self, - metadata: impl Into>, + metadata: impl Into>>, ctrlc: Option>, ) -> PipelineData; } @@ -858,7 +860,7 @@ where fn into_pipeline_data_with_metadata( self, - metadata: impl Into>, + metadata: impl Into>>, ctrlc: Option>, ) -> PipelineData { PipelineData::ListStream( From aeb08d7627054656fd1efb9e0881810f9abc78d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Sat, 11 Feb 2023 23:15:26 +0200 Subject: [PATCH 17/17] Add boxing note --- crates/nu-protocol/src/pipeline_data.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/nu-protocol/src/pipeline_data.rs b/crates/nu-protocol/src/pipeline_data.rs index 4ada6ba3ce75f..d70433d1b78e5 100644 --- a/crates/nu-protocol/src/pipeline_data.rs +++ b/crates/nu-protocol/src/pipeline_data.rs @@ -40,6 +40,8 @@ const LINE_ENDING_PATTERN: &[char] = &['\r', '\n']; /// Nushell. #[derive(Debug)] pub enum PipelineData { + // Note: the PipelineMetadata is boxed everywhere because the DataSource::Profiling caused + // stack overflow on Windows CI when testing virtualenv Value(Value, Option>), ListStream(ListStream, Option>), ExternalStream {