From 5850f787ffb254cdd47629eaa11728593f75d6f1 Mon Sep 17 00:00:00 2001 From: Anup Mahindre Date: Sun, 11 Apr 2021 18:32:03 +0530 Subject: [PATCH 01/11] ls: Remove allocations by eliminating collect/clones --- src/uu/ls/src/ls.rs | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 51453980929..96db3bb2900 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1144,24 +1144,30 @@ fn should_display(entry: &DirEntry, config: &Config) -> bool { } fn enter_directory(dir: &Path, config: &Config) { - let mut entries: Vec<_> = safe_unwrap!(fs::read_dir(dir).and_then(Iterator::collect)); + let mut entries: Vec<_> = if config.files == Files::All { + vec![dir.join("."), dir.join("..")] + } else { + vec![] + }; - entries.retain(|e| should_display(e, config)); + let mut temp: Vec<_> = safe_unwrap!(fs::read_dir(dir)) + .map(|res| safe_unwrap!(res)) + .filter(|e| should_display(e, config)) + .map(|e| DirEntry::path(&e)) + .collect(); - let mut entries: Vec<_> = entries.iter().map(DirEntry::path).collect(); - sort_entries(&mut entries, config); + sort_entries(&mut temp, config); - if config.files == Files::All { - let mut display_entries = entries.clone(); - display_entries.insert(0, dir.join("..")); - display_entries.insert(0, dir.join(".")); - display_items(&display_entries, Some(dir), config, false); - } else { - display_items(&entries, Some(dir), config, false); - } + entries.append(&mut temp); + + display_items(&entries, Some(dir), config, false); if config.recursive { - for e in entries.iter().filter(|p| p.is_dir()) { + for e in entries + .iter() + .skip(if config.files == Files::All { 2 } else { 0 }) + .filter(|p| p.is_dir()) + { println!("\n{}:", e.to_string_lossy()); enter_directory(&e, config); } From 76cc7fbd568d2e43d9d5c78fb7d60a6e22236b4a Mon Sep 17 00:00:00 2001 From: Anup Mahindre Date: Sat, 17 Apr 2021 14:22:29 +0530 Subject: [PATCH 02/11] ls: Introduce PathData structure - PathData will hold Path related metadata / strings that are required frequently in subsequent functions - All data is precomputed and cached and subsequent functions just use cached data --- src/uu/ls/src/ls.rs | 135 ++++++++++++++++++++++++-------------------- 1 file changed, 74 insertions(+), 61 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 96db3bb2900..d37033a9a34 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1038,12 +1038,32 @@ pub fn uumain(args: impl uucore::Args) -> i32 { list(locs, Config::from(matches)) } +struct PathData { + md: std::io::Result, + lossy_string: String, + p_buf: PathBuf, +} + +impl PathData { + fn new(p_buf: PathBuf, config: &Config, command_line: bool) -> Self { + let md = get_metadata(&p_buf, config, command_line); + let lossy_string = p_buf.to_string_lossy().into_owned(); + // let path_name: String = + Self { + md, + lossy_string, + p_buf, + } + } +} + fn list(locs: Vec, config: Config) -> i32 { let number_of_locs = locs.len(); - let mut files = Vec::::new(); - let mut dirs = Vec::::new(); + let mut files = Vec::::new(); + let mut dirs = Vec::::new(); let mut has_failed = false; + for loc in locs { let p = PathBuf::from(&loc); if !p.exists() { @@ -1054,36 +1074,28 @@ fn list(locs: Vec, config: Config) -> i32 { continue; } - let show_dir_contents = if !config.directory { - match config.dereference { - Dereference::None => { - if let Ok(md) = p.symlink_metadata() { - md.is_dir() - } else { - show_error!("'{}': {}", &loc, "No such file or directory"); - has_failed = true; - continue; - } - } - _ => p.is_dir(), - } + let path_data = PathData::new(p, &config, true); + + let show_dir_contents = if let Ok(md) = path_data.md.as_ref() { + !config.directory && md.is_dir() } else { + has_failed = true; false }; if show_dir_contents { - dirs.push(p); + dirs.push(path_data); } else { - files.push(p); + files.push(path_data); } } sort_entries(&mut files, &config); - display_items(&files, None, &config, true); + display_items(&files, None, &config); sort_entries(&mut dirs, &config); for dir in dirs { if number_of_locs > 1 { - println!("\n{}:", dir.to_string_lossy()); + println!("\n{}:", dir.lossy_string); } enter_directory(&dir, &config); } @@ -1094,22 +1106,22 @@ fn list(locs: Vec, config: Config) -> i32 { } } -fn sort_entries(entries: &mut Vec, config: &Config) { +fn sort_entries(entries: &mut Vec, config: &Config) { match config.sort { Sort::Time => entries.sort_by_key(|k| { Reverse( - get_metadata(k, false) + k.md.as_ref() .ok() .and_then(|md| get_system_time(&md, config)) .unwrap_or(UNIX_EPOCH), ) }), Sort::Size => { - entries.sort_by_key(|k| Reverse(get_metadata(k, false).map(|md| md.len()).unwrap_or(0))) + entries.sort_by_key(|k| Reverse(k.md.as_ref().map(|md| md.len()).unwrap_or(0))) } // The default sort in GNU ls is case insensitive - Sort::Name => entries.sort_by_key(|k| k.to_string_lossy().to_lowercase()), - Sort::Version => entries.sort_by(|a, b| version_cmp::version_cmp(a, b)), + Sort::Name => entries.sort_by_key(|k| k.lossy_string.to_lowercase()), + Sort::Version => entries.sort_by(|k, j| version_cmp::version_cmp(&k.p_buf, &j.p_buf)), Sort::None => {} } @@ -1143,38 +1155,57 @@ fn should_display(entry: &DirEntry, config: &Config) -> bool { true } -fn enter_directory(dir: &Path, config: &Config) { +fn enter_directory(dir: &PathData, config: &Config) { let mut entries: Vec<_> = if config.files == Files::All { - vec![dir.join("."), dir.join("..")] + vec![ + PathData::new(dir.p_buf.join("."), config, false), + PathData::new(dir.p_buf.join(".."), config, false), + ] } else { vec![] }; - let mut temp: Vec<_> = safe_unwrap!(fs::read_dir(dir)) + let mut temp: Vec<_> = safe_unwrap!(fs::read_dir(&dir.p_buf)) .map(|res| safe_unwrap!(res)) .filter(|e| should_display(e, config)) - .map(|e| DirEntry::path(&e)) + .map(|e| PathData::new(DirEntry::path(&e), config, false)) .collect(); sort_entries(&mut temp, config); entries.append(&mut temp); - display_items(&entries, Some(dir), config, false); + display_items(&entries, Some(&dir.p_buf), config); if config.recursive { for e in entries .iter() .skip(if config.files == Files::All { 2 } else { 0 }) - .filter(|p| p.is_dir()) + .filter(|p| p.md.as_ref().map_or(false, |md| md.is_dir())) { - println!("\n{}:", e.to_string_lossy()); + println!("\n{}:", e.lossy_string); enter_directory(&e, config); } } } -fn get_metadata(entry: &Path, dereference: bool) -> std::io::Result { +fn get_metadata(entry: &Path, config: &Config, command_line: bool) -> std::io::Result { + let dereference = match &config.dereference { + Dereference::All => true, + Dereference::Args => command_line, + Dereference::DirArgs => { + if command_line { + if let Ok(md) = entry.metadata() { + md.is_dir() + } else { + false + } + } else { + false + } + } + Dereference::None => false, + }; if dereference { entry.metadata().or_else(|_| entry.symlink_metadata()) } else { @@ -1182,8 +1213,8 @@ fn get_metadata(entry: &Path, dereference: bool) -> std::io::Result { } } -fn display_dir_entry_size(entry: &Path, config: &Config) -> (usize, usize) { - if let Ok(md) = get_metadata(entry, false) { +fn display_dir_entry_size(entry: &PathData, config: &Config) -> (usize, usize) { + if let Ok(md) = entry.md.as_ref() { ( display_symlink_count(&md).len(), display_file_size(&md, config).len(), @@ -1197,7 +1228,7 @@ fn pad_left(string: String, count: usize) -> String { format!("{:>width$}", string, width = count) } -fn display_items(items: &[PathBuf], strip: Option<&Path>, config: &Config, command_line: bool) { +fn display_items(items: &[PathData], strip: Option<&Path>, config: &Config) { if config.format == Format::Long { let (mut max_links, mut max_size) = (1, 1); for item in items { @@ -1206,18 +1237,18 @@ fn display_items(items: &[PathBuf], strip: Option<&Path>, config: &Config, comma max_size = size.max(max_size); } for item in items { - display_item_long(item, strip, max_links, max_size, config, command_line); + display_item_long(item, strip, max_links, max_size, config); } } else { let names = items.iter().filter_map(|i| { - let md = get_metadata(i, false); + let md = i.md.as_ref(); match md { Err(e) => { - let filename = get_file_name(i, strip); + let filename = get_file_name(&i.p_buf, strip); show_error!("'{}': {}", filename, e); None } - Ok(md) => Some(display_file_name(&i, strip, &md, config)), + Ok(md) => Some(display_file_name(&i.p_buf, strip, &md, config)), } }); @@ -1277,33 +1308,15 @@ fn display_grid(names: impl Iterator, width: u16, direction: Direct use uucore::fs::display_permissions; fn display_item_long( - item: &Path, + item: &PathData, strip: Option<&Path>, max_links: usize, max_size: usize, config: &Config, - command_line: bool, ) { - let dereference = match &config.dereference { - Dereference::All => true, - Dereference::Args => command_line, - Dereference::DirArgs => { - if command_line { - if let Ok(md) = item.metadata() { - md.is_dir() - } else { - false - } - } else { - false - } - } - Dereference::None => false, - }; - - let md = match get_metadata(item, dereference) { + let md = match &item.md { Err(e) => { - let filename = get_file_name(&item, strip); + let filename = get_file_name(&item.p_buf, strip); show_error!("{}: {}", filename, e); return; } @@ -1342,7 +1355,7 @@ fn display_item_long( " {} {} {}", pad_left(display_file_size(&md, config), max_size), display_date(&md, config), - display_file_name(&item, strip, &md, config).contents, + display_file_name(&item.p_buf, strip, &md, config).contents, ); } From af68ec853efbd2ecc9021b42205225b583a89b6a Mon Sep 17 00:00:00 2001 From: Anup Mahindre Date: Sat, 17 Apr 2021 18:09:04 +0530 Subject: [PATCH 03/11] ls: Cache more data related to paths - Cache filename and sort by filename instead of full path - Cache uid->usr and gid->grp mappings --- Cargo.lock | 238 ++++++++++++++++++++++++++++++++++++++++++- src/uu/ls/Cargo.toml | 1 + src/uu/ls/src/ls.rs | 25 ++++- 3 files changed, 259 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 461716b1bc7..9c8a058bdd0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -37,6 +37,26 @@ dependencies = [ "nodrop", ] +[[package]] +name = "async-mutex" +version = "1.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "479db852db25d9dbf6204e6cb6253698f175c15726470f78af0d918e99d6156e" +dependencies = [ + "event-listener", +] + +[[package]] +name = "async-trait" +version = "0.1.49" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "589652ce7ccb335d1e7ecb3be145425702b290dbcb7029bbeaae263fc1d87b48" +dependencies = [ + "proc-macro2", + "quote 1.0.9", + "syn", +] + [[package]] name = "atty" version = "0.2.14" @@ -125,6 +145,40 @@ version = "1.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "14c189c53d098945499cdfa7ecc63567cf3886b3332b312a5b4585d8d3a6a610" +[[package]] +name = "cached" +version = "0.23.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5e2afe73808fbaac302e39c9754bfc3c4b4d0f99c9c240b9f4e4efc841ad1b74" +dependencies = [ + "async-mutex", + "async-trait", + "cached_proc_macro", + "cached_proc_macro_types", + "futures", + "hashbrown", + "once_cell", +] + +[[package]] +name = "cached_proc_macro" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bf857ae42d910aede5c5186e62684b0d7a597ce2fe3bd14448ab8f7ef439848c" +dependencies = [ + "async-mutex", + "cached_proc_macro_types", + "darling", + "quote 1.0.9", + "syn", +] + +[[package]] +name = "cached_proc_macro_types" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3a4f925191b4367301851c6d99b09890311d74b0d43f274c0b34c86d308a3663" + [[package]] name = "cast" version = "0.2.3" @@ -172,7 +226,7 @@ dependencies = [ "ansi_term", "atty", "bitflags", - "strsim", + "strsim 0.8.0", "textwrap", "unicode-width", "vec_map", @@ -523,6 +577,41 @@ version = "0.1.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ef8ae57c4978a2acd8b869ce6b9ca1dfe817bff704c220209fdef2c0b75a01b9" +[[package]] +name = "darling" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0d706e75d87e35569db781a9b5e2416cff1236a47ed380831f959382ccd5f858" +dependencies = [ + "darling_core", + "darling_macro", +] + +[[package]] +name = "darling_core" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f0c960ae2da4de88a91b2d920c2a7233b400bc33cb28453a2987822d8392519b" +dependencies = [ + "fnv", + "ident_case", + "proc-macro2", + "quote 1.0.9", + "strsim 0.9.3", + "syn", +] + +[[package]] +name = "darling_macro" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d9b5a2f4ac4969822c62224815d069952656cadc7084fdca9751e6d959189b72" +dependencies = [ + "darling_core", + "quote 1.0.9", + "syn", +] + [[package]] name = "data-encoding" version = "2.1.2" @@ -560,6 +649,12 @@ dependencies = [ "regex", ] +[[package]] +name = "event-listener" +version = "2.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f7531096570974c3a9dcf9e4b8e1cede1ec26cf5046219fb3b9d897503b9be59" + [[package]] name = "fake-simd" version = "0.1.2" @@ -602,6 +697,98 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a06f77d526c1a601b7c4cdd98f54b5eaabffc14d5f2f0296febdc7f357c6d3ba" +[[package]] +name = "futures" +version = "0.3.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a9d5813545e459ad3ca1bff9915e9ad7f1a47dc6a91b627ce321d5863b7dd253" +dependencies = [ + "futures-channel", + "futures-core", + "futures-executor", + "futures-io", + "futures-sink", + "futures-task", + "futures-util", +] + +[[package]] +name = "futures-channel" +version = "0.3.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ce79c6a52a299137a6013061e0cf0e688fce5d7f1bc60125f520912fdb29ec25" +dependencies = [ + "futures-core", + "futures-sink", +] + +[[package]] +name = "futures-core" +version = "0.3.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "098cd1c6dda6ca01650f1a37a794245eb73181d0d4d4e955e2f3c37db7af1815" + +[[package]] +name = "futures-executor" +version = "0.3.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "10f6cb7042eda00f0049b1d2080aa4b93442997ee507eb3828e8bd7577f94c9d" +dependencies = [ + "futures-core", + "futures-task", + "futures-util", +] + +[[package]] +name = "futures-io" +version = "0.3.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "365a1a1fb30ea1c03a830fdb2158f5236833ac81fa0ad12fe35b29cddc35cb04" + +[[package]] +name = "futures-macro" +version = "0.3.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "668c6733a182cd7deb4f1de7ba3bf2120823835b3bcfbeacf7d2c4a773c1bb8b" +dependencies = [ + "proc-macro-hack", + "proc-macro2", + "quote 1.0.9", + "syn", +] + +[[package]] +name = "futures-sink" +version = "0.3.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c5629433c555de3d82861a7a4e3794a4c40040390907cfbfd7143a92a426c23" + +[[package]] +name = "futures-task" +version = "0.3.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ba7aa51095076f3ba6d9a1f702f74bd05ec65f555d70d2033d55ba8d69f581bc" + +[[package]] +name = "futures-util" +version = "0.3.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3c144ad54d60f23927f0a6b6d816e4271278b64f005ad65e4e35291d2de9c025" +dependencies = [ + "futures-channel", + "futures-core", + "futures-io", + "futures-macro", + "futures-sink", + "futures-task", + "memchr 2.3.4", + "pin-project-lite", + "pin-utils", + "proc-macro-hack", + "proc-macro-nested", + "slab", +] + [[package]] name = "generic-array" version = "0.8.4" @@ -663,6 +850,12 @@ version = "1.7.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "62aca2aba2d62b4a7f5b33f3712cb1b0692779a56fb510499d5c0aa594daeaf3" +[[package]] +name = "hashbrown" +version = "0.9.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d7afe4a420e3fe79967a00898cc1f4db7c8a49a9333a29f8a4bd76a253d5cd04" + [[package]] name = "heck" version = "0.3.2" @@ -698,6 +891,12 @@ dependencies = [ "winapi 0.3.9", ] +[[package]] +name = "ident_case" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b9e0384b61958566e926dc50660321d12159025e767c18e043daf26b70104c39" + [[package]] name = "if_rust_version" version = "1.0.0" @@ -897,6 +1096,12 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b8f8bdf33df195859076e54ab11ee78a1b208382d3a26ec40d142ffc1ecc49ef" +[[package]] +name = "once_cell" +version = "1.7.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "af8b08b04175473088b46763e51ee54da5f9a164bc162f615b91bc179dbf15a3" + [[package]] name = "onig" version = "4.3.3" @@ -944,6 +1149,18 @@ dependencies = [ "proc-macro-hack", ] +[[package]] +name = "pin-project-lite" +version = "0.2.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc0e1f259c92177c30a4c9d177246edd0a3568b25756a977d0632cf8fa37e905" + +[[package]] +name = "pin-utils" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" + [[package]] name = "pkg-config" version = "0.3.19" @@ -1000,6 +1217,12 @@ version = "0.5.19" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dbf0c48bc1d91375ae5c3cd81e3722dff1abcf81a30960240640d223f59fe0e5" +[[package]] +name = "proc-macro-nested" +version = "0.1.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bc881b2c22681370c6a780e47af9840ef841837bc98118431d4e1868bd0c1086" + [[package]] name = "proc-macro2" version = "1.0.26" @@ -1353,6 +1576,12 @@ dependencies = [ "generic-array", ] +[[package]] +name = "slab" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c111b5bd5695e56cffe5129854aa230b39c93a305372fdbb2668ca2394eea9f8" + [[package]] name = "smallvec" version = "0.6.14" @@ -1374,6 +1603,12 @@ version = "0.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8ea5119cdb4c55b55d432abb513a0429384878c15dde60cc77b1c99de1a95a6a" +[[package]] +name = "strsim" +version = "0.9.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6446ced80d6c486436db5c078dde11a9f73d42b57fb273121e160b84f63d894c" + [[package]] name = "strum" version = "0.20.0" @@ -1987,6 +2222,7 @@ name = "uu_ls" version = "0.0.6" dependencies = [ "atty", + "cached", "clap", "globset", "lazy_static", diff --git a/src/uu/ls/Cargo.toml b/src/uu/ls/Cargo.toml index dacdc7cd9fa..5684652b1b6 100644 --- a/src/uu/ls/Cargo.toml +++ b/src/uu/ls/Cargo.toml @@ -15,6 +15,7 @@ edition = "2018" path = "src/ls.rs" [dependencies] +cached = "0.23.0" clap = "2.33" lazy_static = "1.0.1" number_prefix = "0.4" diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index d37033a9a34..a5523951038 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -16,6 +16,7 @@ extern crate uucore; mod quoting_style; mod version_cmp; +use cached::proc_macro::cached; use clap::{App, Arg}; use globset::{self, Glob, GlobSet, GlobSetBuilder}; use number_prefix::NumberPrefix; @@ -1041,6 +1042,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { struct PathData { md: std::io::Result, lossy_string: String, + name: String, p_buf: PathBuf, } @@ -1048,10 +1050,13 @@ impl PathData { fn new(p_buf: PathBuf, config: &Config, command_line: bool) -> Self { let md = get_metadata(&p_buf, config, command_line); let lossy_string = p_buf.to_string_lossy().into_owned(); - // let path_name: String = + let name = p_buf + .file_name() + .map_or(String::new(), |s| s.to_string_lossy().into_owned()); Self { md, lossy_string, + name, p_buf, } } @@ -1120,7 +1125,7 @@ fn sort_entries(entries: &mut Vec, config: &Config) { entries.sort_by_key(|k| Reverse(k.md.as_ref().map(|md| md.len()).unwrap_or(0))) } // The default sort in GNU ls is case insensitive - Sort::Name => entries.sort_by_key(|k| k.lossy_string.to_lowercase()), + Sort::Name => entries.sort_by_key(|k| k.name.to_lowercase()), Sort::Version => entries.sort_by(|k, j| version_cmp::version_cmp(&k.p_buf, &j.p_buf)), Sort::None => {} } @@ -1369,21 +1374,33 @@ fn get_inode(metadata: &Metadata) -> String { #[cfg(unix)] use uucore::entries; +#[cfg(unix)] +#[cached] +fn cached_uid2usr(uid: u32) -> String { + entries::uid2usr(uid).unwrap_or_else(|_| uid.to_string()) +} + #[cfg(unix)] fn display_uname(metadata: &Metadata, config: &Config) -> String { if config.long.numeric_uid_gid { metadata.uid().to_string() } else { - entries::uid2usr(metadata.uid()).unwrap_or_else(|_| metadata.uid().to_string()) + cached_uid2usr(metadata.uid()) } } +#[cfg(unix)] +#[cached] +fn cached_gid2grp(gid: u32) -> String { + entries::gid2grp(gid).unwrap_or_else(|_| gid.to_string()) +} + #[cfg(unix)] fn display_group(metadata: &Metadata, config: &Config) -> String { if config.long.numeric_uid_gid { metadata.gid().to_string() } else { - entries::gid2grp(metadata.gid()).unwrap_or_else(|_| metadata.gid().to_string()) + cached_gid2grp(metadata.gid()) } } From a7bbef9a0fcdde10ea39a136705feb01b484f95d Mon Sep 17 00:00:00 2001 From: Anup Mahindre Date: Sun, 18 Apr 2021 15:26:45 +0530 Subject: [PATCH 04/11] ls: Add BENCHMARKING.md --- src/uu/ls/BENCHMARKING.md | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 src/uu/ls/BENCHMARKING.md diff --git a/src/uu/ls/BENCHMARKING.md b/src/uu/ls/BENCHMARKING.md new file mode 100644 index 00000000000..c23202ed2b3 --- /dev/null +++ b/src/uu/ls/BENCHMARKING.md @@ -0,0 +1,29 @@ +# Benchmarking ls + +ls majorly involves fetching a lot of details (depending upon what details are requested, eg. time/date, inode details, etc) for each path using system calls. Ideally, any system call should be done only once for each of the paths - not adhering to this principle leads to a lot of system call overhead multiplying and bubbling up, especially for recursive ls, therefore it is important to always benchmark multiple scenarios. +This is an overwiew over what was benchmarked, and if you make changes to `ls`, you are encouraged to check +how performance was affected for the workloads listed below. Feel free to add other workloads to the +list that we should improve / make sure not to regress. + +Run `cargo build --release` before benchmarking after you make a change! + +## Simple recursive ls + +- Get a large tree, for example linux kernel source tree. +- Benchmark simple recursive ls with hyperfine: `hyperfine --warmup 2 "target/release/coreutils ls -R tree > /dev/null"`. + +## Recursive ls with all and long options + +- Same tree as above +- Benchmark recursive ls with -al -R options with hyperfine: `hyperfine --warmup 2 "target/release/coreutils ls -al -R tree > /dev/null"`. + +## Comparing with GNU sort + +Hyperfine accepts multiple commands to run and will compare them. To compare performance with GNU sort +duplicate the string you passed to hyperfine but remove the `target/release/coreutils` bit from it. + +Example: `hyperfine --warmup 2 "target/release/coreutils ls -al -R tree > /dev/null"` becomes +`hyperfine --warmup 2 "target/release/coreutils ls -al -R tree > /dev/null" "ls -al -R tree > /dev/null"` +(This assumes GNU sort is installed as `sort`) + +This can also be used to compare with version of ls built before your changes to ensure your change does not regress this \ No newline at end of file From f89cb6d6c82a6fdcefdb46e69620eac13bdf4d89 Mon Sep 17 00:00:00 2001 From: Anup Mahindre Date: Sun, 18 Apr 2021 16:09:23 +0530 Subject: [PATCH 05/11] ls: Document PathData structure --- src/uu/ls/src/ls.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index a5523951038..1f5087c75c9 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1039,10 +1039,17 @@ pub fn uumain(args: impl uucore::Args) -> i32 { list(locs, Config::from(matches)) } +/// Represents a Path along with it's associated data +/// Any data that will be reused several times makes sense to be added to this structure +/// Caching data here helps eliminate redundant syscalls to fetch same information struct PathData { + // Result got from symlink_metadata() or metadata() based on config md: std::io::Result, + // String formed from get_lossy_string() for the path lossy_string: String, - name: String, + // Name of the file - will be empty for . or .. + file_name: String, + // PathBuf that all above data corresponds to p_buf: PathBuf, } @@ -1056,7 +1063,7 @@ impl PathData { Self { md, lossy_string, - name, + file_name: name, p_buf, } } @@ -1125,7 +1132,7 @@ fn sort_entries(entries: &mut Vec, config: &Config) { entries.sort_by_key(|k| Reverse(k.md.as_ref().map(|md| md.len()).unwrap_or(0))) } // The default sort in GNU ls is case insensitive - Sort::Name => entries.sort_by_key(|k| k.name.to_lowercase()), + Sort::Name => entries.sort_by_key(|k| k.file_name.to_lowercase()), Sort::Version => entries.sort_by(|k, j| version_cmp::version_cmp(&k.p_buf, &j.p_buf)), Sort::None => {} } From b4af0d02d5192924ae5252013e7673942cf1799c Mon Sep 17 00:00:00 2001 From: Anup Mahindre Date: Sun, 18 Apr 2021 16:59:23 +0530 Subject: [PATCH 06/11] tests/ls: Add testcase for error paths with width option --- tests/by-util/test_ls.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index d810cdc294f..5583dbaca5f 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -103,6 +103,14 @@ fn test_ls_width() { .succeeds() .stdout_only("test-width-1\ntest-width-2\ntest-width-3\ntest-width-4\n"); } + + for option in &["-w 1a", "-w=1a", "--width=1a", "--width 1a"] { + scene + .ucmd() + .args(&option.split(" ").collect::>()) + .fails() + .stderr_only("ls: error: invalid line width: ‘1a’"); + } } #[test] From 5731dc47548c8845324206c4473288c4aacc4a23 Mon Sep 17 00:00:00 2001 From: Anup Mahindre Date: Mon, 19 Apr 2021 19:25:38 +0530 Subject: [PATCH 07/11] ls: Fix unused import warning cached will be only used for unix currently as current use of caching gid/uid mappings is only relevant on unix --- src/uu/ls/src/ls.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 1f5087c75c9..0cf448b586e 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -16,6 +16,7 @@ extern crate uucore; mod quoting_style; mod version_cmp; +#[cfg(unix)] use cached::proc_macro::cached; use clap::{App, Arg}; use globset::{self, Glob, GlobSet, GlobSetBuilder}; From fc66b3205678b7423175a2f6e286b835e4e1a0c4 Mon Sep 17 00:00:00 2001 From: Anup Mahindre Date: Mon, 19 Apr 2021 20:16:52 +0530 Subject: [PATCH 08/11] ls: Suggest checking syscall count in BENCHMARKING.md --- src/uu/ls/BENCHMARKING.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/uu/ls/BENCHMARKING.md b/src/uu/ls/BENCHMARKING.md index c23202ed2b3..a902ea26103 100644 --- a/src/uu/ls/BENCHMARKING.md +++ b/src/uu/ls/BENCHMARKING.md @@ -26,4 +26,9 @@ Example: `hyperfine --warmup 2 "target/release/coreutils ls -al -R tree > /dev/n `hyperfine --warmup 2 "target/release/coreutils ls -al -R tree > /dev/null" "ls -al -R tree > /dev/null"` (This assumes GNU sort is installed as `sort`) -This can also be used to compare with version of ls built before your changes to ensure your change does not regress this \ No newline at end of file +This can also be used to compare with version of ls built before your changes to ensure your change does not regress this + +## Checking system call count + +- Another thing to look at would be system calls count using strace (on linux) or equivalent on other operating systems. +- Example: `strace -c target/release/coreutils ls -al -R tree` From 34b4ae08a7970eefb4560b457fd85becc80479fb Mon Sep 17 00:00:00 2001 From: Anup Mahindre Date: Mon, 19 Apr 2021 20:16:52 +0530 Subject: [PATCH 09/11] ls: Remove mentions of sort in BENCHMARKING.md --- src/uu/ls/BENCHMARKING.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/uu/ls/BENCHMARKING.md b/src/uu/ls/BENCHMARKING.md index a902ea26103..84a0c3d843a 100644 --- a/src/uu/ls/BENCHMARKING.md +++ b/src/uu/ls/BENCHMARKING.md @@ -17,14 +17,14 @@ Run `cargo build --release` before benchmarking after you make a change! - Same tree as above - Benchmark recursive ls with -al -R options with hyperfine: `hyperfine --warmup 2 "target/release/coreutils ls -al -R tree > /dev/null"`. -## Comparing with GNU sort +## Comparing with GNU ls -Hyperfine accepts multiple commands to run and will compare them. To compare performance with GNU sort +Hyperfine accepts multiple commands to run and will compare them. To compare performance with GNU ls duplicate the string you passed to hyperfine but remove the `target/release/coreutils` bit from it. Example: `hyperfine --warmup 2 "target/release/coreutils ls -al -R tree > /dev/null"` becomes `hyperfine --warmup 2 "target/release/coreutils ls -al -R tree > /dev/null" "ls -al -R tree > /dev/null"` -(This assumes GNU sort is installed as `sort`) +(This assumes GNU ls is installed as `ls`) This can also be used to compare with version of ls built before your changes to ensure your change does not regress this From 9415f6c15b6f3c8d6999e74a6fb05f4bbb228575 Mon Sep 17 00:00:00 2001 From: Anup Mahindre Date: Tue, 20 Apr 2021 21:23:11 +0530 Subject: [PATCH 10/11] ls: Remove dependency on cached Implement caching using HashMap and lazy_static --- Cargo.lock | 238 +------------------------------------------ src/uu/ls/Cargo.toml | 1 - src/uu/ls/src/ls.rs | 34 +++++-- 3 files changed, 29 insertions(+), 244 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9c8a058bdd0..461716b1bc7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -37,26 +37,6 @@ dependencies = [ "nodrop", ] -[[package]] -name = "async-mutex" -version = "1.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "479db852db25d9dbf6204e6cb6253698f175c15726470f78af0d918e99d6156e" -dependencies = [ - "event-listener", -] - -[[package]] -name = "async-trait" -version = "0.1.49" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "589652ce7ccb335d1e7ecb3be145425702b290dbcb7029bbeaae263fc1d87b48" -dependencies = [ - "proc-macro2", - "quote 1.0.9", - "syn", -] - [[package]] name = "atty" version = "0.2.14" @@ -145,40 +125,6 @@ version = "1.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "14c189c53d098945499cdfa7ecc63567cf3886b3332b312a5b4585d8d3a6a610" -[[package]] -name = "cached" -version = "0.23.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5e2afe73808fbaac302e39c9754bfc3c4b4d0f99c9c240b9f4e4efc841ad1b74" -dependencies = [ - "async-mutex", - "async-trait", - "cached_proc_macro", - "cached_proc_macro_types", - "futures", - "hashbrown", - "once_cell", -] - -[[package]] -name = "cached_proc_macro" -version = "0.6.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bf857ae42d910aede5c5186e62684b0d7a597ce2fe3bd14448ab8f7ef439848c" -dependencies = [ - "async-mutex", - "cached_proc_macro_types", - "darling", - "quote 1.0.9", - "syn", -] - -[[package]] -name = "cached_proc_macro_types" -version = "0.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3a4f925191b4367301851c6d99b09890311d74b0d43f274c0b34c86d308a3663" - [[package]] name = "cast" version = "0.2.3" @@ -226,7 +172,7 @@ dependencies = [ "ansi_term", "atty", "bitflags", - "strsim 0.8.0", + "strsim", "textwrap", "unicode-width", "vec_map", @@ -577,41 +523,6 @@ version = "0.1.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ef8ae57c4978a2acd8b869ce6b9ca1dfe817bff704c220209fdef2c0b75a01b9" -[[package]] -name = "darling" -version = "0.10.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0d706e75d87e35569db781a9b5e2416cff1236a47ed380831f959382ccd5f858" -dependencies = [ - "darling_core", - "darling_macro", -] - -[[package]] -name = "darling_core" -version = "0.10.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f0c960ae2da4de88a91b2d920c2a7233b400bc33cb28453a2987822d8392519b" -dependencies = [ - "fnv", - "ident_case", - "proc-macro2", - "quote 1.0.9", - "strsim 0.9.3", - "syn", -] - -[[package]] -name = "darling_macro" -version = "0.10.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d9b5a2f4ac4969822c62224815d069952656cadc7084fdca9751e6d959189b72" -dependencies = [ - "darling_core", - "quote 1.0.9", - "syn", -] - [[package]] name = "data-encoding" version = "2.1.2" @@ -649,12 +560,6 @@ dependencies = [ "regex", ] -[[package]] -name = "event-listener" -version = "2.5.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f7531096570974c3a9dcf9e4b8e1cede1ec26cf5046219fb3b9d897503b9be59" - [[package]] name = "fake-simd" version = "0.1.2" @@ -697,98 +602,6 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a06f77d526c1a601b7c4cdd98f54b5eaabffc14d5f2f0296febdc7f357c6d3ba" -[[package]] -name = "futures" -version = "0.3.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a9d5813545e459ad3ca1bff9915e9ad7f1a47dc6a91b627ce321d5863b7dd253" -dependencies = [ - "futures-channel", - "futures-core", - "futures-executor", - "futures-io", - "futures-sink", - "futures-task", - "futures-util", -] - -[[package]] -name = "futures-channel" -version = "0.3.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ce79c6a52a299137a6013061e0cf0e688fce5d7f1bc60125f520912fdb29ec25" -dependencies = [ - "futures-core", - "futures-sink", -] - -[[package]] -name = "futures-core" -version = "0.3.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "098cd1c6dda6ca01650f1a37a794245eb73181d0d4d4e955e2f3c37db7af1815" - -[[package]] -name = "futures-executor" -version = "0.3.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "10f6cb7042eda00f0049b1d2080aa4b93442997ee507eb3828e8bd7577f94c9d" -dependencies = [ - "futures-core", - "futures-task", - "futures-util", -] - -[[package]] -name = "futures-io" -version = "0.3.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "365a1a1fb30ea1c03a830fdb2158f5236833ac81fa0ad12fe35b29cddc35cb04" - -[[package]] -name = "futures-macro" -version = "0.3.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "668c6733a182cd7deb4f1de7ba3bf2120823835b3bcfbeacf7d2c4a773c1bb8b" -dependencies = [ - "proc-macro-hack", - "proc-macro2", - "quote 1.0.9", - "syn", -] - -[[package]] -name = "futures-sink" -version = "0.3.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5c5629433c555de3d82861a7a4e3794a4c40040390907cfbfd7143a92a426c23" - -[[package]] -name = "futures-task" -version = "0.3.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ba7aa51095076f3ba6d9a1f702f74bd05ec65f555d70d2033d55ba8d69f581bc" - -[[package]] -name = "futures-util" -version = "0.3.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3c144ad54d60f23927f0a6b6d816e4271278b64f005ad65e4e35291d2de9c025" -dependencies = [ - "futures-channel", - "futures-core", - "futures-io", - "futures-macro", - "futures-sink", - "futures-task", - "memchr 2.3.4", - "pin-project-lite", - "pin-utils", - "proc-macro-hack", - "proc-macro-nested", - "slab", -] - [[package]] name = "generic-array" version = "0.8.4" @@ -850,12 +663,6 @@ version = "1.7.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "62aca2aba2d62b4a7f5b33f3712cb1b0692779a56fb510499d5c0aa594daeaf3" -[[package]] -name = "hashbrown" -version = "0.9.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d7afe4a420e3fe79967a00898cc1f4db7c8a49a9333a29f8a4bd76a253d5cd04" - [[package]] name = "heck" version = "0.3.2" @@ -891,12 +698,6 @@ dependencies = [ "winapi 0.3.9", ] -[[package]] -name = "ident_case" -version = "1.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b9e0384b61958566e926dc50660321d12159025e767c18e043daf26b70104c39" - [[package]] name = "if_rust_version" version = "1.0.0" @@ -1096,12 +897,6 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b8f8bdf33df195859076e54ab11ee78a1b208382d3a26ec40d142ffc1ecc49ef" -[[package]] -name = "once_cell" -version = "1.7.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "af8b08b04175473088b46763e51ee54da5f9a164bc162f615b91bc179dbf15a3" - [[package]] name = "onig" version = "4.3.3" @@ -1149,18 +944,6 @@ dependencies = [ "proc-macro-hack", ] -[[package]] -name = "pin-project-lite" -version = "0.2.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dc0e1f259c92177c30a4c9d177246edd0a3568b25756a977d0632cf8fa37e905" - -[[package]] -name = "pin-utils" -version = "0.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" - [[package]] name = "pkg-config" version = "0.3.19" @@ -1217,12 +1000,6 @@ version = "0.5.19" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dbf0c48bc1d91375ae5c3cd81e3722dff1abcf81a30960240640d223f59fe0e5" -[[package]] -name = "proc-macro-nested" -version = "0.1.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bc881b2c22681370c6a780e47af9840ef841837bc98118431d4e1868bd0c1086" - [[package]] name = "proc-macro2" version = "1.0.26" @@ -1576,12 +1353,6 @@ dependencies = [ "generic-array", ] -[[package]] -name = "slab" -version = "0.4.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c111b5bd5695e56cffe5129854aa230b39c93a305372fdbb2668ca2394eea9f8" - [[package]] name = "smallvec" version = "0.6.14" @@ -1603,12 +1374,6 @@ version = "0.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8ea5119cdb4c55b55d432abb513a0429384878c15dde60cc77b1c99de1a95a6a" -[[package]] -name = "strsim" -version = "0.9.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6446ced80d6c486436db5c078dde11a9f73d42b57fb273121e160b84f63d894c" - [[package]] name = "strum" version = "0.20.0" @@ -2222,7 +1987,6 @@ name = "uu_ls" version = "0.0.6" dependencies = [ "atty", - "cached", "clap", "globset", "lazy_static", diff --git a/src/uu/ls/Cargo.toml b/src/uu/ls/Cargo.toml index 5684652b1b6..dacdc7cd9fa 100644 --- a/src/uu/ls/Cargo.toml +++ b/src/uu/ls/Cargo.toml @@ -15,7 +15,6 @@ edition = "2018" path = "src/ls.rs" [dependencies] -cached = "0.23.0" clap = "2.33" lazy_static = "1.0.1" number_prefix = "0.4" diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 0cf448b586e..47d1b708a06 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -16,8 +16,6 @@ extern crate uucore; mod quoting_style; mod version_cmp; -#[cfg(unix)] -use cached::proc_macro::cached; use clap::{App, Arg}; use globset::{self, Glob, GlobSet, GlobSetBuilder}; use number_prefix::NumberPrefix; @@ -1380,12 +1378,25 @@ fn get_inode(metadata: &Metadata) -> String { // Currently getpwuid is `linux` target only. If it's broken out into // a posix-compliant attribute this can be updated... #[cfg(unix)] +use std::sync::Mutex; +#[cfg(unix)] use uucore::entries; #[cfg(unix)] -#[cached] fn cached_uid2usr(uid: u32) -> String { - entries::uid2usr(uid).unwrap_or_else(|_| uid.to_string()) + lazy_static! { + static ref UID_CACHE: Mutex> = Mutex::new(HashMap::new()); + } + + let mut uid_cache = UID_CACHE.lock().unwrap(); + match uid_cache.get(&uid) { + Some(usr) => usr.clone(), + None => { + let usr = entries::uid2usr(uid).unwrap_or_else(|_| uid.to_string()); + uid_cache.insert(uid, usr.clone()); + usr + } + } } #[cfg(unix)] @@ -1398,9 +1409,20 @@ fn display_uname(metadata: &Metadata, config: &Config) -> String { } #[cfg(unix)] -#[cached] fn cached_gid2grp(gid: u32) -> String { - entries::gid2grp(gid).unwrap_or_else(|_| gid.to_string()) + lazy_static! { + static ref GID_CACHE: Mutex> = Mutex::new(HashMap::new()); + } + + let mut gid_cache = GID_CACHE.lock().unwrap(); + match gid_cache.get(&gid) { + Some(grp) => grp.clone(), + None => { + let grp = entries::gid2grp(gid).unwrap_or_else(|_| gid.to_string()); + gid_cache.insert(gid, grp.clone()); + grp + } + } } #[cfg(unix)] From 42ecac32ffc6e606d1d04ed906176ca5f757e382 Mon Sep 17 00:00:00 2001 From: Anup Mahindre Date: Tue, 20 Apr 2021 22:49:10 +0530 Subject: [PATCH 11/11] ls: Fix MSRV error related to map_or Rust 1.40 did not support map_or for result types --- src/uu/ls/src/ls.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 47d1b708a06..e0aa3ec4bc4 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1192,7 +1192,7 @@ fn enter_directory(dir: &PathData, config: &Config) { for e in entries .iter() .skip(if config.files == Files::All { 2 } else { 0 }) - .filter(|p| p.md.as_ref().map_or(false, |md| md.is_dir())) + .filter(|p| p.md.as_ref().map(|md| md.is_dir()).unwrap_or(false)) { println!("\n{}:", e.lossy_string); enter_directory(&e, config);