From 69a4bedb55137d3d579617e4b40018b2a5e1b3e4 Mon Sep 17 00:00:00 2001 From: Mohammad AlSaleh Date: Wed, 9 Oct 2024 06:51:52 +0300 Subject: [PATCH 1/2] Significantly improve performance by using a buffered writer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before ``` Benchmark 1: seq 10000000 | bat Time (mean ± σ): 6.235 s ± 0.052 s [User: 3.664 s, System: 2.714 s] Range (min … max): 6.172 s … 6.355 s 10 runs ``` After ``` Benchmark 1: seq 10000000 | ./target/release/bat Time (mean ± σ): 215.9 ms ± 5.1 ms [User: 275.4 ms, System: 38.8 ms] Range (min … max): 210.3 ms … 224.9 ms 10 runs ``` Using `less` for comparison ``` Benchmark 1: seq 10000000 | less Time (mean ± σ): 637.3 ms ± 43.3 ms [User: 642.1 ms, System: 95.6 ms] Range (min … max): 584.5 ms … 700.1 ms 10 runs ``` And raw ``` Benchmark 1: seq 10000000 Time (mean ± σ): 63.1 ms ± 1.3 ms [User: 57.1 ms, System: 5.9 ms] Range (min … max): 62.1 ms … 66.0 ms 10 runs ``` Signed-off-by: Mohammad AlSaleh --- CHANGELOG.md | 1 + src/controller.rs | 26 ++++++++++-------- src/printer.rs | 68 ++++++++++++++++++++++++++++------------------- 3 files changed, 57 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d2ec2da9b5..36ff94de29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,7 @@ - Use bat's ANSI iterator during tab expansion, see #2998 (@eth-p) - Support 'statically linked binary' for aarch64 in 'Release' page, see #2992 (@tzq0301) - Update options in shell completions and the man page of `bat`, see #2995 (@akinomyoga) +- Significantly improve performance by using a buffered writer, see #3101 (@MoSal) ## Syntaxes diff --git a/src/controller.rs b/src/controller.rs index ffc5dd5b57..4acf0e9204 100644 --- a/src/controller.rs +++ b/src/controller.rs @@ -1,4 +1,4 @@ -use std::io::{self, BufRead, Write}; +use std::io::{self, BufRead, BufWriter, Write}; use crate::assets::HighlightingAssets; use crate::config::{Config, VisibleLines}; @@ -88,9 +88,12 @@ impl<'b> Controller<'b> { clircle::Identifier::stdout() }; + const BUF_W_SZ: usize = 1 << 14; let mut writer = match output_buffer { Some(buf) => OutputHandle::FmtWrite(buf), - None => OutputHandle::IoWrite(output_type.handle()?), + None => { + OutputHandle::IoWrite(BufWriter::with_capacity(BUF_W_SZ, output_type.handle()?)) + } }; let mut no_errors: bool = true; let stderr = io::stderr(); @@ -124,10 +127,10 @@ impl<'b> Controller<'b> { Ok(no_errors) } - fn print_input( + fn print_input( &self, input: Input, - writer: &mut OutputHandle, + writer: &mut OutputHandle, stdin: R, stdout_identifier: Option<&Identifier>, is_first: bool, @@ -174,7 +177,7 @@ impl<'b> Controller<'b> { None }; - let mut printer: Box = if self.config.loop_through { + let mut printer: Box> = if self.config.loop_through { Box::new(SimplePrinter::new(self.config)) } else { Box::new(InteractivePrinter::new( @@ -196,10 +199,10 @@ impl<'b> Controller<'b> { ) } - fn print_file( + fn print_file( &self, - printer: &mut dyn Printer, - writer: &mut OutputHandle, + printer: &mut dyn Printer, + writer: &mut OutputHandle, input: &mut OpenedInput, add_header_padding: bool, #[cfg(feature = "git")] line_changes: &Option, @@ -234,10 +237,10 @@ impl<'b> Controller<'b> { Ok(()) } - fn print_file_ranges( + fn print_file_ranges( &self, - printer: &mut dyn Printer, - writer: &mut OutputHandle, + printer: &mut dyn Printer, + writer: &mut OutputHandle, reader: &mut InputReader, line_ranges: &LineRanges, ) -> Result<()> { @@ -279,6 +282,7 @@ impl<'b> Controller<'b> { line_number += 1; line_buffer.clear(); } + writer.flush()?; Ok(()) } } diff --git a/src/printer.rs b/src/printer.rs index e9bea3fd8f..9149878f85 100644 --- a/src/printer.rs +++ b/src/printer.rs @@ -1,5 +1,5 @@ use std::fmt; -use std::io; +use std::io::{self, BufWriter, Write}; use std::vec::Vec; use nu_ansi_term::Color::{Fixed, Green, Red, Yellow}; @@ -67,35 +67,42 @@ const EMPTY_SYNTECT_STYLE: syntect::highlighting::Style = syntect::highlighting: font_style: FontStyle::empty(), }; -pub enum OutputHandle<'a> { - IoWrite(&'a mut dyn io::Write), +pub enum OutputHandle<'a, W: io::Write> { + IoWrite(BufWriter), FmtWrite(&'a mut dyn fmt::Write), } -impl<'a> OutputHandle<'a> { +impl<'a, W: io::Write> OutputHandle<'a, W> { fn write_fmt(&mut self, args: fmt::Arguments<'_>) -> Result<()> { match self { Self::IoWrite(handle) => handle.write_fmt(args).map_err(Into::into), Self::FmtWrite(handle) => handle.write_fmt(args).map_err(Into::into), } } + + pub(crate) fn flush(&mut self) -> Result<()> { + match self { + Self::IoWrite(handle) => handle.flush().map_err(Into::into), + Self::FmtWrite(_handle) => Ok(()), + } + } } -pub(crate) trait Printer { +pub(crate) trait Printer { fn print_header( &mut self, - handle: &mut OutputHandle, + handle: &mut OutputHandle, input: &OpenedInput, add_header_padding: bool, ) -> Result<()>; - fn print_footer(&mut self, handle: &mut OutputHandle, input: &OpenedInput) -> Result<()>; + fn print_footer(&mut self, handle: &mut OutputHandle, input: &OpenedInput) -> Result<()>; - fn print_snip(&mut self, handle: &mut OutputHandle) -> Result<()>; + fn print_snip(&mut self, handle: &mut OutputHandle) -> Result<()>; fn print_line( &mut self, out_of_range: bool, - handle: &mut OutputHandle, + handle: &mut OutputHandle, line_number: usize, line_buffer: &[u8], ) -> Result<()>; @@ -115,28 +122,28 @@ impl<'a> SimplePrinter<'a> { } } -impl<'a> Printer for SimplePrinter<'a> { +impl<'a, W: io::Write> Printer for SimplePrinter<'a> { fn print_header( &mut self, - _handle: &mut OutputHandle, + _handle: &mut OutputHandle, _input: &OpenedInput, _add_header_padding: bool, ) -> Result<()> { Ok(()) } - fn print_footer(&mut self, _handle: &mut OutputHandle, _input: &OpenedInput) -> Result<()> { + fn print_footer(&mut self, _handle: &mut OutputHandle, _input: &OpenedInput) -> Result<()> { Ok(()) } - fn print_snip(&mut self, _handle: &mut OutputHandle) -> Result<()> { + fn print_snip(&mut self, _handle: &mut OutputHandle) -> Result<()> { Ok(()) } fn print_line( &mut self, out_of_range: bool, - handle: &mut OutputHandle, + handle: &mut OutputHandle, _line_number: usize, line_buffer: &[u8], ) -> Result<()> { @@ -321,9 +328,9 @@ impl<'a> InteractivePrinter<'a> { }) } - fn print_horizontal_line_term( + fn print_horizontal_line_term( &mut self, - handle: &mut OutputHandle, + handle: &mut OutputHandle, style: Style, ) -> Result<()> { writeln!( @@ -334,7 +341,11 @@ impl<'a> InteractivePrinter<'a> { Ok(()) } - fn print_horizontal_line(&mut self, handle: &mut OutputHandle, grid_char: char) -> Result<()> { + fn print_horizontal_line( + &mut self, + handle: &mut OutputHandle, + grid_char: char, + ) -> Result<()> { if self.panel_width == 0 { self.print_horizontal_line_term(handle, self.colors.grid)?; } else { @@ -372,7 +383,10 @@ impl<'a> InteractivePrinter<'a> { } } - fn print_header_component_indent(&mut self, handle: &mut OutputHandle) -> Result<()> { + fn print_header_component_indent( + &mut self, + handle: &mut OutputHandle, + ) -> Result<()> { if self.config.style_components.grid() { write!( handle, @@ -387,18 +401,18 @@ impl<'a> InteractivePrinter<'a> { } } - fn print_header_component_with_indent( + fn print_header_component_with_indent( &mut self, - handle: &mut OutputHandle, + handle: &mut OutputHandle, content: &str, ) -> Result<()> { self.print_header_component_indent(handle)?; writeln!(handle, "{content}") } - fn print_header_multiline_component( + fn print_header_multiline_component( &mut self, - handle: &mut OutputHandle, + handle: &mut OutputHandle, content: &str, ) -> Result<()> { let mut content = content; @@ -446,10 +460,10 @@ impl<'a> InteractivePrinter<'a> { } } -impl<'a> Printer for InteractivePrinter<'a> { +impl<'a, W: io::Write> Printer for InteractivePrinter<'a> { fn print_header( &mut self, - handle: &mut OutputHandle, + handle: &mut OutputHandle, input: &OpenedInput, add_header_padding: bool, ) -> Result<()> { @@ -549,7 +563,7 @@ impl<'a> Printer for InteractivePrinter<'a> { Ok(()) } - fn print_footer(&mut self, handle: &mut OutputHandle, _input: &OpenedInput) -> Result<()> { + fn print_footer(&mut self, handle: &mut OutputHandle, _input: &OpenedInput) -> Result<()> { if self.config.style_components.grid() && (self.content_type.map_or(false, |c| c.is_text()) || self.config.show_nonprintable) { @@ -559,7 +573,7 @@ impl<'a> Printer for InteractivePrinter<'a> { } } - fn print_snip(&mut self, handle: &mut OutputHandle) -> Result<()> { + fn print_snip(&mut self, handle: &mut OutputHandle) -> Result<()> { let panel = self.create_fake_panel(" ..."); let panel_count = panel.chars().count(); @@ -586,7 +600,7 @@ impl<'a> Printer for InteractivePrinter<'a> { fn print_line( &mut self, out_of_range: bool, - handle: &mut OutputHandle, + handle: &mut OutputHandle, line_number: usize, line_buffer: &[u8], ) -> Result<()> { From 0afeef2ba8b68eb65012c5b8febfe6d4a7768fa0 Mon Sep 17 00:00:00 2001 From: Mohammad AlSaleh Date: Fri, 25 Oct 2024 15:27:31 +0300 Subject: [PATCH 2/2] Flush on each read line if input is terminal stdin bat supports interactive use by opening without file arguments or with `-`. This uses the terminal stdin for input. This won't work with a buffered writer if the writer is not flushed after each read line from input. So this commit enables such flushing behavior if stdin is detected as terminal input, while retaining the benefits of using a buffered writer otherwise. Note that `Input::from_stdin()` from the `pretty_printer` API will assume input is terminal to retain old (unbuffered) behavior while the API is kept the same. Signed-off-by: Mohammad AlSaleh --- src/bin/bat/app.rs | 5 ++++- src/bin/bat/input.rs | 4 ++-- src/controller.rs | 14 +++++++++++++- src/input.rs | 10 +++++++++- src/pretty_printer.rs | 2 +- 5 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/bin/bat/app.rs b/src/bin/bat/app.rs index d662866875..df3f19c78f 100644 --- a/src/bin/bat/app.rs +++ b/src/bin/bat/app.rs @@ -338,6 +338,8 @@ impl App { return Err("Must be one file name per input type.".into()); } + let stdin_is_terminal = || std::io::stdin().is_terminal(); + let mut filenames_or_none: Box>> = match filenames { Some(filenames) => Box::new(filenames.into_iter().map(Some)), None => Box::new(std::iter::repeat(None)), @@ -345,6 +347,7 @@ impl App { if files.is_none() { return Ok(vec![new_stdin_input( filenames_or_none.next().unwrap_or(None), + stdin_is_terminal(), )]); } let files_or_none: Box> = match files { @@ -356,7 +359,7 @@ impl App { for (filepath, provided_name) in files_or_none.zip(filenames_or_none) { if let Some(filepath) = filepath { if filepath.to_str().unwrap_or_default() == "-" { - file_input.push(new_stdin_input(provided_name)); + file_input.push(new_stdin_input(provided_name, stdin_is_terminal())); } else { file_input.push(new_file_input(filepath, provided_name)); } diff --git a/src/bin/bat/input.rs b/src/bin/bat/input.rs index 3c9289069d..18775f09e5 100644 --- a/src/bin/bat/input.rs +++ b/src/bin/bat/input.rs @@ -5,8 +5,8 @@ pub fn new_file_input<'a>(file: &'a Path, name: Option<&'a Path>) -> Input<'a> { named(Input::ordinary_file(file), name.or(Some(file))) } -pub fn new_stdin_input(name: Option<&Path>) -> Input { - named(Input::stdin(), name) +pub fn new_stdin_input(name: Option<&Path>, is_terminal: bool) -> Input { + named(Input::stdin(is_terminal), name) } fn named<'a>(input: Input<'a>, name: Option<&Path>) -> Input<'a> { diff --git a/src/controller.rs b/src/controller.rs index 4acf0e9204..d0e96c0262 100644 --- a/src/controller.rs +++ b/src/controller.rs @@ -230,7 +230,13 @@ impl<'b> Controller<'b> { } }; - self.print_file_ranges(printer, writer, &mut input.reader, &line_ranges)?; + self.print_file_ranges( + printer, + writer, + &mut input.reader, + &line_ranges, + input.is_terminal, + )?; } printer.print_footer(writer, input)?; @@ -243,6 +249,7 @@ impl<'b> Controller<'b> { writer: &mut OutputHandle, reader: &mut InputReader, line_ranges: &LineRanges, + input_is_terminal: bool, ) -> Result<()> { let mut line_buffer = Vec::new(); let mut line_number: usize = 1; @@ -281,6 +288,11 @@ impl<'b> Controller<'b> { line_number += 1; line_buffer.clear(); + + // flush per line if input is terminal to allow interactive use + if input_is_terminal { + writer.flush()?; + } } writer.flush()?; Ok(()) diff --git a/src/input.rs b/src/input.rs index 0ebaa4cead..6904784dfe 100644 --- a/src/input.rs +++ b/src/input.rs @@ -94,6 +94,7 @@ pub(crate) struct InputMetadata { pub struct Input<'a> { pub(crate) kind: InputKind<'a>, pub(crate) metadata: InputMetadata, + pub(crate) is_terminal: bool, pub(crate) description: InputDescription, } @@ -106,6 +107,7 @@ pub(crate) enum OpenedInputKind { pub(crate) struct OpenedInput<'a> { pub(crate) kind: OpenedInputKind, pub(crate) metadata: InputMetadata, + pub(crate) is_terminal: bool, pub(crate) reader: InputReader<'a>, pub(crate) description: InputDescription, } @@ -141,15 +143,17 @@ impl<'a> Input<'a> { description: kind.description(), metadata, kind, + is_terminal: false, } } - pub fn stdin() -> Self { + pub fn stdin(is_terminal: bool) -> Self { let kind = InputKind::StdIn; Input { description: kind.description(), metadata: InputMetadata::default(), kind, + is_terminal, } } @@ -159,6 +163,7 @@ impl<'a> Input<'a> { description: kind.description(), metadata: InputMetadata::default(), kind, + is_terminal: false, } } @@ -207,6 +212,7 @@ impl<'a> Input<'a> { kind: OpenedInputKind::StdIn, description, metadata: self.metadata, + is_terminal: self.is_terminal, reader: InputReader::new(stdin), }) } @@ -215,6 +221,7 @@ impl<'a> Input<'a> { kind: OpenedInputKind::OrdinaryFile(path.clone()), description, metadata: self.metadata, + is_terminal: self.is_terminal, reader: { let mut file = File::open(&path) .map_err(|e| format!("'{}': {}", path.to_string_lossy(), e))?; @@ -243,6 +250,7 @@ impl<'a> Input<'a> { description, kind: OpenedInputKind::CustomReader, metadata: self.metadata, + is_terminal: self.is_terminal, reader: InputReader::new(BufReader::new(reader)), }), } diff --git a/src/pretty_printer.rs b/src/pretty_printer.rs index eb123ea339..f842a4c406 100644 --- a/src/pretty_printer.rs +++ b/src/pretty_printer.rs @@ -348,7 +348,7 @@ impl<'a> Input<'a> { /// A new input from STDIN. pub fn from_stdin() -> Self { - input::Input::stdin().into() + input::Input::stdin(true).into() } /// The filename of the input.