From e041fda51d59a63783648815f345830f1d0163d9 Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Wed, 6 Oct 2021 20:33:13 +0200 Subject: [PATCH 1/6] tac: do not re-compile regular expression for each file --- src/uu/tac/src/tac.rs | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/uu/tac/src/tac.rs b/src/uu/tac/src/tac.rs index 4a93a7c65..caf77c4a7 100644 --- a/src/uu/tac/src/tac.rs +++ b/src/uu/tac/src/tac.rs @@ -44,9 +44,9 @@ pub fn uumain(args: impl uucore::Args) -> i32 { raw_separator }; - let files: Vec = match matches.values_of(options::FILE) { - Some(v) => v.map(|v| v.to_owned()).collect(), - None => vec!["-".to_owned()], + let files: Vec<&str> = match matches.values_of(options::FILE) { + Some(v) => v.collect(), + None => vec!["-"], }; tac(files, before, regex, separator) @@ -102,7 +102,7 @@ pub fn uu_app() -> App<'static, 'static> { /// returns [`std::io::Error`]. fn buffer_tac_regex( data: &[u8], - pattern: regex::bytes::Regex, + pattern: ®ex::bytes::Regex, before: bool, ) -> std::io::Result<()> { let mut out = stdout(); @@ -208,10 +208,16 @@ fn buffer_tac(data: &[u8], before: bool, separator: &str) -> std::io::Result<()> Ok(()) } -fn tac(filenames: Vec, before: bool, regex: bool, separator: &str) -> i32 { +fn tac(filenames: Vec<&str>, before: bool, regex: bool, separator: &str) -> i32 { let mut exit_code = 0; - for filename in &filenames { + let pattern = if regex { + Some(crash_if_err!(1, regex::bytes::Regex::new(separator))) + } else { + None + }; + + for &filename in &filenames { let mut file = BufReader::new(if filename == "-" { Box::new(stdin()) as Box } else { @@ -244,8 +250,7 @@ fn tac(filenames: Vec, before: bool, regex: bool, separator: &str) -> i3 exit_code = 1; continue; }; - if regex { - let pattern = crash_if_err!(1, regex::bytes::Regex::new(separator)); + if let Some(pattern) = &pattern { buffer_tac_regex(&data, pattern, before) } else { buffer_tac(&data, before, separator) From 0d583754cad9f4d6587469a428fb41a69059818c Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Wed, 6 Oct 2021 21:38:08 +0200 Subject: [PATCH 2/6] tac: lock stdout only once instead for each line --- src/uu/tac/src/tac.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/uu/tac/src/tac.rs b/src/uu/tac/src/tac.rs index caf77c4a7..eab6a943b 100644 --- a/src/uu/tac/src/tac.rs +++ b/src/uu/tac/src/tac.rs @@ -105,7 +105,8 @@ fn buffer_tac_regex( pattern: ®ex::bytes::Regex, before: bool, ) -> std::io::Result<()> { - let mut out = stdout(); + let out = stdout(); + let mut out = out.lock(); // The index of the line separator for the current line. // @@ -171,7 +172,8 @@ fn buffer_tac_regex( /// `separator` appears at the beginning of each line, as in /// `"/abc/def"`. fn buffer_tac(data: &[u8], before: bool, separator: &str) -> std::io::Result<()> { - let mut out = stdout(); + let out = stdout(); + let mut out = out.lock(); // The number of bytes in the line separator. let slen = separator.as_bytes().len(); From 28b04fa89950ff1bac7863934050ef9c26a0022d Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Wed, 6 Oct 2021 20:50:34 +0200 Subject: [PATCH 3/6] tac: do not use a buffered read as fs::read is more efficient and io::Stdin is buffered internally --- src/uu/tac/src/tac.rs | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/uu/tac/src/tac.rs b/src/uu/tac/src/tac.rs index eab6a943b..92bf1ea00 100644 --- a/src/uu/tac/src/tac.rs +++ b/src/uu/tac/src/tac.rs @@ -12,8 +12,8 @@ extern crate uucore; use clap::{crate_version, App, Arg}; use memchr::memmem; -use std::io::{stdin, stdout, BufReader, Read, Write}; -use std::{fs::File, path::Path}; +use std::io::{stdin, stdout, Read, Write}; +use std::{fs::read, path::Path}; use uucore::display::Quotable; use uucore::InvalidEncodingHandling; @@ -220,8 +220,14 @@ fn tac(filenames: Vec<&str>, before: bool, regex: bool, separator: &str) -> i32 }; for &filename in &filenames { - let mut file = BufReader::new(if filename == "-" { - Box::new(stdin()) as Box + let data = if filename == "-" { + let mut data = Vec::new(); + if let Err(e) = stdin().read_to_end(&mut data) { + show_error!("failed to read from stdin: {}", e); + exit_code = 1; + continue; + } + data } else { let path = Path::new(filename); if path.is_dir() || path.metadata().is_err() { @@ -236,22 +242,16 @@ fn tac(filenames: Vec<&str>, before: bool, regex: bool, separator: &str) -> i32 exit_code = 1; continue; } - match File::open(path) { - Ok(f) => Box::new(f) as Box, + match read(path) { + Ok(data) => data, Err(e) => { - show_error!("failed to open {} for reading: {}", filename.quote(), e); + show_error!("failed to read {}: {}", filename.quote(), e); exit_code = 1; continue; } } - }); - - let mut data = Vec::new(); - if let Err(e) = file.read_to_end(&mut data) { - show_error!("failed to read {}: {}", filename.quote(), e); - exit_code = 1; - continue; }; + if let Some(pattern) = &pattern { buffer_tac_regex(&data, pattern, before) } else { From 4eab275235f2d89ea48bb7b2b921ea33a8db3fa7 Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Sat, 9 Oct 2021 12:12:57 +0200 Subject: [PATCH 4/6] tac: buffer stdout more coarsely than line-based following the GNU tac implementation. --- src/uu/tac/src/tac.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/uu/tac/src/tac.rs b/src/uu/tac/src/tac.rs index 92bf1ea00..370e92230 100644 --- a/src/uu/tac/src/tac.rs +++ b/src/uu/tac/src/tac.rs @@ -12,7 +12,7 @@ extern crate uucore; use clap::{crate_version, App, Arg}; use memchr::memmem; -use std::io::{stdin, stdout, Read, Write}; +use std::io::{stdin, stdout, BufWriter, Read, Write}; use std::{fs::read, path::Path}; use uucore::display::Quotable; use uucore::InvalidEncodingHandling; @@ -106,7 +106,7 @@ fn buffer_tac_regex( before: bool, ) -> std::io::Result<()> { let out = stdout(); - let mut out = out.lock(); + let mut out = BufWriter::new(out.lock()); // The index of the line separator for the current line. // @@ -173,7 +173,7 @@ fn buffer_tac_regex( /// `"/abc/def"`. fn buffer_tac(data: &[u8], before: bool, separator: &str) -> std::io::Result<()> { let out = stdout(); - let mut out = out.lock(); + let mut out = BufWriter::new(out.lock()); // The number of bytes in the line separator. let slen = separator.as_bytes().len(); From c526df57b8e10b94b096476c5eaf20eae52e7061 Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Wed, 6 Oct 2021 21:08:11 +0200 Subject: [PATCH 5/6] tac: opportunistically use memory maps Since tac must read its input files completely to start processing them from the end, it is particularly suited to use memory maps to benefit from the page cache maintained by the operating systems to bring the necessary data into memory as required. This does also include situations where the input is stdin, but not via a pipe but for example a file descriptor set up by the user's shell through an input redirection. --- Cargo.lock | 10 ++++++ src/uu/tac/Cargo.toml | 3 ++ src/uu/tac/src/tac.rs | 72 +++++++++++++++++++++++++++++++++---------- 3 files changed, 68 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6c30a182d..3633928c6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1064,6 +1064,15 @@ version = "2.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b16bd47d9e329435e309c58469fe0791c2d0d1ba96ec0954152a5ae2b04387dc" +[[package]] +name = "memmap2" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4647a11b578fead29cdbb34d4adef8dd3dc35b876c9c6d5240d83f205abfe96e" +dependencies = [ + "libc", +] + [[package]] name = "memoffset" version = "0.6.4" @@ -3025,6 +3034,7 @@ version = "0.0.7" dependencies = [ "clap", "memchr 2.4.0", + "memmap2", "regex", "uucore", "uucore_procs", diff --git a/src/uu/tac/Cargo.toml b/src/uu/tac/Cargo.toml index 1e436e916..00803c8d2 100644 --- a/src/uu/tac/Cargo.toml +++ b/src/uu/tac/Cargo.toml @@ -1,3 +1,5 @@ +# spell-checker:ignore memmap + [package] name = "uu_tac" version = "0.0.7" @@ -16,6 +18,7 @@ path = "src/tac.rs" [dependencies] memchr = "2" +memmap2 = "0.5" regex = "1" clap = { version = "2.33", features = ["wrap_help"] } uucore = { version=">=0.0.9", package="uucore", path="../../uucore" } diff --git a/src/uu/tac/src/tac.rs b/src/uu/tac/src/tac.rs index 370e92230..cdb2d74e3 100644 --- a/src/uu/tac/src/tac.rs +++ b/src/uu/tac/src/tac.rs @@ -5,15 +5,19 @@ // * For the full copyright and license information, please view the LICENSE // * file that was distributed with this source code. -// spell-checker:ignore (ToDO) sbytes slen dlen memmem +// spell-checker:ignore (ToDO) sbytes slen dlen memmem memmap Mmap mmap SIGBUS #[macro_use] extern crate uucore; use clap::{crate_version, App, Arg}; use memchr::memmem; +use memmap2::Mmap; use std::io::{stdin, stdout, BufWriter, Read, Write}; -use std::{fs::read, path::Path}; +use std::{ + fs::{read, File}, + path::Path, +}; use uucore::display::Quotable; use uucore::InvalidEncodingHandling; @@ -220,14 +224,23 @@ fn tac(filenames: Vec<&str>, before: bool, regex: bool, separator: &str) -> i32 }; for &filename in &filenames { - let data = if filename == "-" { - let mut data = Vec::new(); - if let Err(e) = stdin().read_to_end(&mut data) { - show_error!("failed to read from stdin: {}", e); - exit_code = 1; - continue; + let mmap; + let buf; + + let data: &[u8] = if filename == "-" { + if let Some(mmap1) = try_mmap_stdin() { + mmap = mmap1; + &mmap + } else { + let mut buf1 = Vec::new(); + if let Err(e) = stdin().read_to_end(&mut buf1) { + show_error!("failed to read from stdin: {}", e); + exit_code = 1; + continue; + } + buf = buf1; + &buf } - data } else { let path = Path::new(filename); if path.is_dir() || path.metadata().is_err() { @@ -242,22 +255,47 @@ fn tac(filenames: Vec<&str>, before: bool, regex: bool, separator: &str) -> i32 exit_code = 1; continue; } - match read(path) { - Ok(data) => data, - Err(e) => { - show_error!("failed to read {}: {}", filename.quote(), e); - exit_code = 1; - continue; + + if let Some(mmap1) = try_mmap_path(path) { + mmap = mmap1; + &mmap + } else { + match read(path) { + Ok(buf1) => { + buf = buf1; + &buf + } + Err(e) => { + show_error!("failed to read {}: {}", filename.quote(), e); + exit_code = 1; + continue; + } } } }; if let Some(pattern) = &pattern { - buffer_tac_regex(&data, pattern, before) + buffer_tac_regex(data, pattern, before) } else { - buffer_tac(&data, before, separator) + buffer_tac(data, before, separator) } .unwrap_or_else(|e| crash!(1, "failed to write to stdout: {}", e)); } exit_code } + +fn try_mmap_stdin() -> Option { + // SAFETY: If the file is truncated while we map it, SIGBUS will be raised + // and our process will be terminated, thus preventing access of invalid memory. + unsafe { Mmap::map(&stdin()).ok() } +} + +fn try_mmap_path(path: &Path) -> Option { + let file = File::open(path).ok()?; + + // SAFETY: If the file is truncated while we map it, SIGBUS will be raised + // and our process will be terminated, thus preventing access of invalid memory. + let mmap = unsafe { Mmap::map(&file).ok()? }; + + Some(mmap) +} From 86d22aaa1d4a1bd252083cfd2a144bbfa5198c8f Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Sun, 10 Oct 2021 13:54:35 +0200 Subject: [PATCH 6/6] tac: Add a simple how to for benchmarking --- src/uu/tac/BENCHMARKING.md | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 src/uu/tac/BENCHMARKING.md diff --git a/src/uu/tac/BENCHMARKING.md b/src/uu/tac/BENCHMARKING.md new file mode 100644 index 000000000..4e6d13ea2 --- /dev/null +++ b/src/uu/tac/BENCHMARKING.md @@ -0,0 +1,25 @@ +## Benchmarking `tac` + + + +`tac` is often used to process log files in reverse chronological order, i.e. from newer towards older entries. In this case, the performance target to yield results as fast as possible, i.e. without reading in the whole file that is to be reversed line-by-line. Therefore, a sensible benchmark is to read a large log file containing N lines and measure how long it takes to produce the last K lines from that file. + +Large text files can for example be found in the [Wikipedia database dumps](https://dumps.wikimedia.org/wikidatawiki/latest/), usually sized at multiple gigabytes and comprising more than 100M lines. + +After you have obtained and uncompressed such a file, you need to build `tac` in release mode + +```shell +$ cargo build --release --package uu_tac +``` + +and then you can time how it long it takes to extract the last 10M lines by running + +```shell +$ /usr/bin/time ./target/release/tac wikidatawiki-20211001-pages-logging.xml | head -n10000000 >/dev/null +``` + +For more systematic measurements that include warm-ups, repetitions and comparisons, [Hyperfine](https://github.com/sharkdp/hyperfine) can be helpful. For example, to compare this implementation to the one provided by your distribution run + +```shell +$ hyperfine "./target/release/tac wikidatawiki-20211001-pages-logging.xml | head -n10000000 >/dev/null" "/usr/bin/tac wikidatawiki-20211001-pages-logging.xml | head -n10000000 >/dev/null" +```