From aa3364b6065c881a51812c559b7e7fbe24a7ce19 Mon Sep 17 00:00:00 2001 From: Kevin Amado Date: Sun, 30 Jan 2022 23:06:28 -0500 Subject: [PATCH] perf: twice as fast - Was refactoring a little bit the code and seems that moving the path to the build_ct makes the code much more faster --- README.md | 10 +++---- src/builder.rs | 77 ++++++++++++++++++++++++++++---------------------- src/format.rs | 11 +++++--- src/main.rs | 7 +++-- tests/fmt.rs | 23 ++------------- 5 files changed, 63 insertions(+), 65 deletions(-) diff --git a/README.md b/README.md index ae3287a..efe08ab 100644 --- a/README.md +++ b/README.md @@ -41,11 +41,11 @@ | Cores | Seconds | | :---: | :-----: | - | 1 | 40 | - | 2 | 21 | - | 4 | 15 | - | 8 | 11 | - | 16 | 10 | + | 1 | 24 | + | 2 | 12 | + | 4 | 7 | + | 8 | 6 | + | 16 | 7 | - ✔️ **Powerful** diff --git a/src/builder.rs b/src/builder.rs index 7a7b435..aab364f 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -18,16 +18,18 @@ pub struct BuildCtx { pub indentation: usize, pub pos_new: crate::position::Position, pub pos_old: crate::position::Position, + pub path: String, } impl BuildCtx { pub fn new( config: crate::config::Config, force_wide: bool, + path: String, pos_new: crate::position::Position, pos_old: crate::position::Position, ) -> BuildCtx { - BuildCtx { config, force_wide, indentation: 0, pos_new, pos_old } + BuildCtx { config, force_wide, indentation: 0, path, pos_new, pos_old } } } @@ -35,12 +37,13 @@ pub fn build( config: &crate::config::Config, element: rnix::SyntaxElement, force_wide: bool, - path: &str, + path: String, ) -> Option { let mut builder = rowan::GreenNodeBuilder::new(); let mut build_ctx = BuildCtx::new( config.clone(), force_wide, + path, crate::position::Position::new(), crate::position::Position::new(), ); @@ -48,7 +51,6 @@ pub fn build( build_step( &mut builder, &mut build_ctx, - path, &crate::builder::Step::Format(element), ); @@ -62,7 +64,7 @@ pub fn build( fn build_step( builder: &mut rowan::GreenNodeBuilder, build_ctx: &mut BuildCtx, - path: &str, + step: &crate::builder::Step, ) { if build_ctx.force_wide && build_ctx.pos_new.line > 1 { @@ -102,10 +104,10 @@ fn build_step( build_ctx.indentation -= 1; } crate::builder::Step::Format(element) => { - format(builder, build_ctx, element, path); + format(builder, build_ctx, element); } crate::builder::Step::FormatWider(element) => { - format_wider(builder, build_ctx, element, path); + format_wider(builder, build_ctx, element); } crate::builder::Step::Indent => { build_ctx.indentation += 1; @@ -156,7 +158,6 @@ fn format( builder: &mut rowan::GreenNodeBuilder, build_ctx: &mut BuildCtx, element: &rnix::SyntaxElement, - path: &str, ) { let kind = element.kind(); @@ -177,7 +178,10 @@ fn format( rnix::SyntaxKind::NODE_DYNAMIC => crate::rules::dynamic::rule, // implementation detail of rnix-parser rnix::SyntaxKind::NODE_ERROR => { - eprintln!("Warning: found an error node at: {}", path); + eprintln!( + "Warning: found an error node at: {}", + build_ctx.path + ); crate::rules::default } // $identifier @@ -207,7 +211,7 @@ fn format( rnix::SyntaxKind::NODE_LEGACY_LET => { eprintln!( "Warning: found a `legacy let` expression at: {}", - path + build_ctx.path ); crate::rules::default } @@ -244,12 +248,15 @@ fn format( // with a; b rnix::SyntaxKind::NODE_WITH => crate::rules::with::rule, kind => { - panic!("Missing rule for {:?} at: {}", kind, path); + panic!( + "Missing rule for {:?} at: {}", + kind, build_ctx.path + ); } }; for step in rule(build_ctx, node) { - build_step(builder, build_ctx, path, &step); + build_step(builder, build_ctx, &step); } builder.finish_node(); @@ -265,37 +272,41 @@ fn format_wider( builder: &mut rowan::GreenNodeBuilder, build_ctx: &mut BuildCtx, element: &rnix::SyntaxElement, - path: &str, ) { match element { rnix::SyntaxElement::Node(node) => { - let maybe_green_node = build( - &build_ctx.config.with_layout(crate::config::Layout::Wide), - node.clone().into(), - true, - path, - ); - - let layout = match maybe_green_node { - Some(finished) => { - if build_ctx.pos_new.column - + finished.to_string().chars().count() - > build_ctx.config.max_width() - { - crate::config::Layout::Tall - } else { - crate::config::Layout::Wide - } - } - None => crate::config::Layout::Tall, + let layout = if fits_in_single_line(build_ctx, node) { + crate::config::Layout::Wide + } else { + crate::config::Layout::Tall }; let mut build_ctx_clone = build_ctx.clone(); build_ctx_clone.config = build_ctx.config.with_layout(layout); - format(builder, &mut build_ctx_clone, element, path); + format(builder, &mut build_ctx_clone, element); } rnix::SyntaxElement::Token(_) => { - format(builder, build_ctx, element, path); + format(builder, build_ctx, element); } }; } + +pub fn fits_in_single_line( + build_ctx: &crate::builder::BuildCtx, + node: &rnix::SyntaxNode, +) -> bool { + let maybe_green_node = build( + &build_ctx.config.with_layout(crate::config::Layout::Wide), + node.clone().into(), + true, + build_ctx.path.clone(), + ); + + match maybe_green_node { + Some(finished) => { + build_ctx.pos_new.column + finished.to_string().chars().count() + <= build_ctx.config.max_width() + } + None => false, + } +} diff --git a/src/format.rs b/src/format.rs index 67142ac..18f9994 100644 --- a/src/format.rs +++ b/src/format.rs @@ -1,6 +1,6 @@ pub fn string( config: &crate::config::Config, - path: &str, + path: String, string: String, ) -> String { let tokens = rnix::tokenizer::Tokenizer::new(&string); @@ -21,11 +21,14 @@ pub fn string( green_node.to_string() } -pub fn file(config: &crate::config::Config, path: &str) -> std::io::Result<()> { +pub fn file( + config: &crate::config::Config, + path: String, +) -> std::io::Result<()> { use std::io::Write; - let input = std::fs::read_to_string(path)?; - let output = crate::format::string(config, path, input); + let input = std::fs::read_to_string(&path)?; + let output = crate::format::string(config, path.clone(), input); std::fs::File::create(path)?.write_all(output.as_bytes())?; diff --git a/src/main.rs b/src/main.rs index 2d5d98b..c3bbb1c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -23,7 +23,7 @@ fn main() -> std::io::Result<()> { .par_iter() .map(|path| -> std::io::Result<()> { eprintln!("Formatting: {}", &path); - alejandra::format::file(&config, &path)?; + alejandra::format::file(&config, path.to_string())?; Ok(()) }) .collect::>>() @@ -35,7 +35,10 @@ fn main() -> std::io::Result<()> { eprintln!("Formatting stdin."); let mut stdin = String::new(); std::io::stdin().read_to_string(&mut stdin).unwrap(); - print!("{}", alejandra::format::string(&config, "stdin", stdin)); + print!( + "{}", + alejandra::format::string(&config, "stdin".to_string(), stdin) + ); } } diff --git a/tests/fmt.rs b/tests/fmt.rs index 2c3b4fa..b74ffb2 100644 --- a/tests/fmt.rs +++ b/tests/fmt.rs @@ -17,7 +17,7 @@ fn cases() { let path_out = format!("tests/cases/{}/out", case); let content_in = std::fs::read_to_string(path_in.clone()).unwrap(); let content_got = - alejandra::format::string(&config, &path_in, content_in.clone()); + alejandra::format::string(&config, path_in, content_in.clone()); if should_update { std::fs::File::create(&path_out) @@ -28,25 +28,6 @@ fn cases() { let content_out = std::fs::read_to_string(path_out.clone()).unwrap(); - assert!( - content_got == content_out, - indoc::indoc!( - r" - - - input from {}: - {} - after formatting: - {} - expected from {}: - {} - " - ), - path_in, - &content_in, - &content_got, - path_out, - &content_out, - ); + assert!(content_got == content_out); } }