Skip to content

perf: reuse file metadata #39

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
mantono wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

perf: reuse file metadata #39

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 29 additions & 46 deletions src/find.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,43 +38,38 @@ impl Filter {
self
}

pub fn accept(&self, e: &DirEntry) -> bool {
pub fn accept(&self, e: &DirEntry) -> Option<u64> {
let metadata: Metadata = match e.metadata() {
Ok(metadata) => metadata,
Err(err) => {
log::warn!("Unable to obtain metadata for {:?}: {:?}", e.path(), err);
return false;
return None;
}
};

if let Mode::File = self.mode {
if metadata.len() < self.min_size {
return false;
return None;
}
}

if !metadata.is_file() {
return false;
return None;
}

let accept_age: bool = Filter::filter_mod_time(&metadata, &self.mod_age);

if !accept_age {
return false;
if !Filter::filter_mod_time(&metadata, &self.mod_age) {
return None;
}

let file_name: String = match Filter::file_name(e) {
Some(name) => name,
None => return false,
};
let file_name: String = Filter::file_name(e)?;

if e.path().starts_with(PROC) {
return false;
return None;
}

match &self.pattern {
Some(pattern) => pattern.is_match(&file_name),
None => true,
Some(pattern) if !pattern.is_match(&file_name) => None,
_ => Some(metadata.len()),
}
}

Expand Down Expand Up @@ -153,22 +148,11 @@ impl Default for Filter {
}
}

pub fn summarize(files: Vec<DirEntry>) -> (u64, u64) {
let found: u64 = files.len() as u64;
let size: u64 = files
.iter()
.filter_map(|f| f.metadata().ok())
.map(|m| m.len())
.sum();

(found, size)
}

#[cfg(test)]
mod tests {
use crate::find::Filter;
use crate::size::Size;
use crate::{cfg::Config, create_walker, find::summarize, walk_files};
use crate::{cfg::Config, create_walker, walk_files};
use regex::Regex;
use std::path::PathBuf;
use std::str::FromStr;
Expand All @@ -177,32 +161,18 @@ mod tests {
const TEST_DIR: &str = "test_dirs";
const PROC: &str = "/proc";

#[test]
fn test_stop_at_one_found_file() {
let dir = PathBuf::from(TEST_DIR);
let files: Vec<DirEntry> = create_walker(&Config::default(), &dir)
.into_iter()
.filter_map(|e| e.ok())
.take(1)
.collect();
let result: (u64, u64) = summarize(files);
assert_eq!(1, result.0);
}

#[cfg(unix)]
#[test]
fn test_filter_by_file_size() {
let dir = PathBuf::from(TEST_DIR);
let filter = Filter::new().with_min_size(Size::Byte(100));
let files: Vec<DirEntry> = create_walker(&Config::default(), &dir)
let sizes: Vec<u64> = create_walker(&Config::default(), &dir)
.into_iter()
.filter_map(|e| e.ok())
.filter(|e| filter.accept(&e))
.filter_map(Result::ok)
.filter_map(|entry: DirEntry| filter.accept(&entry))
.collect();

let result: (u64, u64) = summarize(files);
assert_eq!(1, result.0);
assert_eq!(100, result.1);
assert_eq!(vec![100], sizes);
}

#[cfg(target_os = "linux")]
Expand All @@ -213,6 +183,19 @@ mod tests {
assert_eq!(0, found);
}

#[test]
fn filter_returns_size_from_accepted_metadata() {
let dir: PathBuf = PathBuf::from(TEST_DIR);
let entry: DirEntry = create_walker(&Config::default(), &dir)
.into_iter()
.filter_map(Result::ok)
.find(|entry: &DirEntry| entry.file_name() == "file2")
.expect("test fixture should contain file2");
let filter: Filter = Filter::new().with_min_size(Size::Byte(1));

assert_eq!(Some(100), filter.accept(&entry));
}

#[test]
fn test_filter_by_file_pattern() {
let dir = PathBuf::from(TEST_DIR);
Expand All @@ -223,7 +206,7 @@ mod tests {
let files: Vec<DirEntry> = create_walker(&Config::default(), &dir)
.into_iter()
.filter_map(|e| e.ok())
.filter(|f| filter.accept(&f))
.filter(|f| filter.accept(f).is_some())
.collect();

assert_eq!(2, files.len());
Expand Down
35 changes: 13 additions & 22 deletions src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

use crate::cfg::Config;
use crate::dbg::dbg_info;
use crate::find::summarize;
use crate::logger::setup_logging;
use crate::print::{print_dir, print_file, print_summary};
use crate::structopt::StructOpt;
Expand Down Expand Up @@ -46,19 +45,17 @@
fn walk_files(cfg: &Config) -> (u64, u64) {
let limit: usize = cfg.limit.unwrap_or(usize::MAX);
let filter: Filter = cfg.into();
let files: Vec<DirEntry> = cfg
.paths()
cfg.paths()
.iter()
.map(|path: &PathBuf| create_walker(cfg, path))
.into_iter()
.flatten()

Check warning on line 51 in src/main.rs

View workflow job for this annotation

GitHub Actions / clippy

called `map(..).flatten()` on `Iterator` 

warning: called `map(..).flatten()` on `Iterator`
  --> src/main.rs:50:10
   |
50 |           .map(|path: &PathBuf| create_walker(cfg, path))
   |  __________^
51 | |         .flatten()
   | |__________________^ help: try replacing `map` with `flat_map` and remove the `.flatten()`: `flat_map(|path: &PathBuf| create_walker(cfg, path))`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.98.0/index.html#map_flatten
.filter_map(|e| e.ok())
.filter(|e: &DirEntry| filter.accept(e))
.filter_map(Result::ok)
.filter_map(|entry: DirEntry| filter.accept(&entry).map(|size| (entry, size)))
.take(limit)
.inspect(|f| print_file(f, cfg))
.collect();

summarize(files)
.fold((0, 0), |(found, total_size), (entry, size)| {
print_file(entry.path(), size, cfg);
(found + 1, total_size + size)
})
}

fn walk_dirs(cfg: &Config) -> (u64, u64) {
Expand All @@ -69,11 +66,14 @@

cfg.paths()
.iter()
.map(|path: &PathBuf| create_walker(cfg, path))
.flatten()

Check warning on line 70 in src/main.rs

View workflow job for this annotation

GitHub Actions / clippy

called `map(..).flatten()` on `Iterator` 

warning: called `map(..).flatten()` on `Iterator`
  --> src/main.rs:69:10
   |
69 |           .map(|path: &PathBuf| create_walker(cfg, path))
   |  __________^
70 | |         .flatten()
   | |__________________^ help: try replacing `map` with `flat_map` and remove the `.flatten()`: `flat_map(|path: &PathBuf| create_walker(cfg, path))`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.98.0/index.html#map_flatten
.filter_map(|e| e.ok())
.filter(|e: &DirEntry| filter.accept(e))
.map(|f: DirEntry| size_of(&f))
.filter_map(Result::ok)
.filter_map(|entry: DirEntry| {
filter
.accept(&entry)
.map(|size| (entry.path().parent().unwrap().to_path_buf(), size))
})
.for_each(|(dir, size)| update_size(&mut acc_size, dir, root, size));

let limit: usize = cfg.limit.unwrap_or(usize::MAX);
Expand Down Expand Up @@ -104,15 +104,6 @@
}
}

fn size_of(entry: &DirEntry) -> (PathBuf, u64) {
let size: u64 = match entry.metadata() {
Ok(metadata) => metadata.len(),
Err(_) => 0,
};
let parent: PathBuf = entry.path().parent().unwrap().to_path_buf();
(parent, size)
}

fn create_walker(cfg: &Config, path: &Path) -> WalkDir {
let walker = WalkDir::new(path)
.follow_links(false)
Expand Down
11 changes: 1 addition & 10 deletions src/print.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,8 @@ use crate::cfg::{Config, Mode};
use humansize::{file_size_opts as options, FileSize};
use itertools::Itertools;
use std::path::{Path, PathBuf};
use walkdir::DirEntry;

pub fn print_file(entry: &DirEntry, cfg: &Config) {
let file: &Path = entry.path();
let size: u64 = match file.metadata() {
Ok(metadata) => metadata.len(),
Err(err) => {
log_error(err, file);
return;
}
};
pub fn print_file(file: &Path, size: u64, cfg: &Config) {
if cfg.plumbing_mode {
print_plumbing(file, size)
} else {
Expand Down
Loading