From 7e891f17c1c6f59f7a8be18e83927271f307dd3d Mon Sep 17 00:00:00 2001 From: Nick Young Date: Sun, 15 Sep 2019 00:23:53 +1000 Subject: [PATCH] perf: Lazy load files from directory (#335) Changes context to use `once_cell` to lazily evaluate directory listing on first use. --- src/context.rs | 56 +++++++++++++++++------------------ src/module.rs | 10 +++---- src/modules/golang.rs | 4 +-- src/modules/nodejs.rs | 4 +-- src/modules/python.rs | 4 +-- src/modules/ruby.rs | 4 +-- src/modules/rust.rs | 4 +-- src/modules/time.rs | 4 +-- src/segment.rs | 4 +-- tests/testsuite/git_branch.rs | 1 - tests/testsuite/git_status.rs | 1 - tests/testsuite/time.rs | 4 --- 12 files changed, 46 insertions(+), 54 deletions(-) diff --git a/src/context.rs b/src/context.rs index 836a18db..3687eb74 100644 --- a/src/context.rs +++ b/src/context.rs @@ -20,7 +20,7 @@ pub struct Context<'a> { pub current_dir: PathBuf, /// A vector containing the full paths of all the files in `current_dir`. - pub dir_files: Vec, + dir_files: OnceCell>, /// The map of arguments that were passed when starship was called. pub arguments: ArgMatches<'a>, @@ -52,22 +52,11 @@ impl<'a> Context<'a> { // TODO: Currently gets the physical directory. Get the logical directory. let current_dir = Context::expand_tilde(dir.into()); - let dir_files = fs::read_dir(¤t_dir) - .unwrap_or_else(|_| { - panic!( - "Unable to read current directory: {}", - current_dir.to_string_lossy() - ) - }) - .filter_map(Result::ok) - .map(|entry| entry.path()) - .collect::>(); - Context { config, arguments, current_dir, - dir_files, + dir_files: OnceCell::new(), repo: OnceCell::new(), } } @@ -100,19 +89,18 @@ impl<'a> Context<'a> { // returns a new ScanDir struct with reference to current dir_files of context // see ScanDir for methods - pub fn new_scan_dir(&'a self) -> ScanDir<'a> { - ScanDir { - dir_files: self.dir_files.as_ref(), + pub fn try_begin_scan(&'a self) -> Option> { + Some(ScanDir { + dir_files: self.get_dir_files().ok()?, files: &[], folders: &[], extensions: &[], - } + }) } /// Will lazily get repo root and branch when a module requests it. pub fn get_repo(&self) -> Result<&Repo, std::io::Error> { - let repo = self - .repo + self.repo .get_or_try_init(|| -> Result { let repository = Repository::discover(&self.current_dir).ok(); let branch = repository @@ -128,9 +116,19 @@ impl<'a> Context<'a> { root, state, }) - })?; + }) + } - Ok(repo) + pub fn get_dir_files(&self) -> Result<&Vec, std::io::Error> { + self.dir_files + .get_or_try_init(|| -> Result, std::io::Error> { + let dir_files = fs::read_dir(&self.current_dir)? + .filter_map(Result::ok) + .map(|entry| entry.path()) + .collect::>(); + + Ok(dir_files) + }) } } @@ -150,7 +148,7 @@ pub struct Repo { // A struct of Criteria which will be used to verify current PathBuf is // of X language, criteria can be set via the builder pattern pub struct ScanDir<'a> { - dir_files: &'a Vec, // Replace with reference + dir_files: &'a Vec, files: &'a [&'a str], folders: &'a [&'a str], extensions: &'a [&'a str], @@ -174,7 +172,7 @@ impl<'a> ScanDir<'a> { /// based on the current Pathbuf check to see /// if any of this criteria match or exist and returning a boolean - pub fn scan(&mut self) -> bool { + pub fn is_match(&self) -> bool { self.dir_files.iter().any(|path| { if path.is_dir() { path_has_name(path, self.folders) @@ -261,7 +259,7 @@ mod tests { #[test] fn test_criteria_scan_fails() { - let mut failing_criteria = ScanDir { + let failing_criteria = ScanDir { dir_files: &vec![PathBuf::new()], files: &["package.json"], extensions: &["js"], @@ -269,9 +267,9 @@ mod tests { }; // fails if buffer does not match any criteria - assert_eq!(failing_criteria.scan(), false); + assert_eq!(failing_criteria.is_match(), false); - let mut failing_dir_criteria = ScanDir { + let failing_dir_criteria = ScanDir { dir_files: &vec![PathBuf::from("/package.js/dog.go")], files: &["package.json"], extensions: &["js"], @@ -279,18 +277,18 @@ mod tests { }; // fails when passed a pathbuf dir matches extension path - assert_eq!(failing_dir_criteria.scan(), false); + assert_eq!(failing_dir_criteria.is_match(), false); } #[test] fn test_criteria_scan_passes() { - let mut passing_criteria = ScanDir { + let passing_criteria = ScanDir { dir_files: &vec![PathBuf::from("package.json")], files: &["package.json"], extensions: &["js"], folders: &["node_modules"], }; - assert_eq!(passing_criteria.scan(), true); + assert_eq!(passing_criteria.is_match(), true); } } diff --git a/src/module.rs b/src/module.rs index c2f4580e..dde073fb 100644 --- a/src/module.rs +++ b/src/module.rs @@ -35,7 +35,7 @@ pub struct Module<'a> { config: Option<&'a toml::value::Table>, /// The module's name, to be used in configuration and logging. - name: String, + _name: String, /// The styling to be inherited by all segments contained within this module. style: Style, @@ -55,7 +55,7 @@ impl<'a> Module<'a> { pub fn new(name: &str, config: Option<&'a toml::value::Table>) -> Module<'a> { Module { config, - name: name.to_string(), + _name: name.to_string(), style: Style::default(), prefix: Affix::default_prefix(name), segments: Vec::new(), @@ -204,7 +204,7 @@ fn ansi_strings_modified(ansi_strings: Vec, shell: String) -> Vec Self { Self { - name: format!("{}_prefix", name), + _name: format!("{}_prefix", name), style: Style::default(), value: "via ".to_string(), } @@ -224,7 +224,7 @@ impl Affix { pub fn default_suffix(name: &str) -> Self { Self { - name: format!("{}_suffix", name), + _name: format!("{}_suffix", name), style: Style::default(), value: " ".to_string(), } diff --git a/src/modules/golang.rs b/src/modules/golang.rs index 4676f9e6..9bbf0c16 100644 --- a/src/modules/golang.rs +++ b/src/modules/golang.rs @@ -15,11 +15,11 @@ use super::{Context, Module}; /// - Current directory contains a file with the `.go` extension pub fn module<'a>(context: &'a Context) -> Option> { let is_go_project = context - .new_scan_dir() + .try_begin_scan()? .set_files(&["go.mod", "go.sum", "glide.yaml", "Gopkg.yml", "Gopkg.lock"]) .set_extensions(&["go"]) .set_folders(&["Godeps"]) - .scan(); + .is_match(); if !is_go_project { return None; diff --git a/src/modules/nodejs.rs b/src/modules/nodejs.rs index 180e143f..b30f2f53 100644 --- a/src/modules/nodejs.rs +++ b/src/modules/nodejs.rs @@ -11,11 +11,11 @@ use super::{Context, Module}; /// - Current directory contains a `node_modules` directory pub fn module<'a>(context: &'a Context) -> Option> { let is_js_project = context - .new_scan_dir() + .try_begin_scan()? .set_files(&["package.json"]) .set_extensions(&["js"]) .set_folders(&["node_modules"]) - .scan(); + .is_match(); if !is_js_project { return None; diff --git a/src/modules/python.rs b/src/modules/python.rs index 487d033d..84ca0e7e 100644 --- a/src/modules/python.rs +++ b/src/modules/python.rs @@ -16,7 +16,7 @@ use super::{Context, Module}; /// - Current directory contains a `Pipfile` file pub fn module<'a>(context: &'a Context) -> Option> { let is_py_project = context - .new_scan_dir() + .try_begin_scan()? .set_files(&[ "requirements.txt", ".python-version", @@ -24,7 +24,7 @@ pub fn module<'a>(context: &'a Context) -> Option> { "Pipfile", ]) .set_extensions(&["py"]) - .scan(); + .is_match(); if !is_py_project { return None; diff --git a/src/modules/ruby.rs b/src/modules/ruby.rs index 7e689234..0faa2ccf 100644 --- a/src/modules/ruby.rs +++ b/src/modules/ruby.rs @@ -10,10 +10,10 @@ use super::{Context, Module}; /// - Current directory contains a `Gemfile` file pub fn module<'a>(context: &'a Context) -> Option> { let is_rb_project = context - .new_scan_dir() + .try_begin_scan()? .set_files(&["Gemfile"]) .set_extensions(&["rb"]) - .scan(); + .is_match(); if !is_rb_project { return None; diff --git a/src/modules/rust.rs b/src/modules/rust.rs index 26ee377c..2008a539 100644 --- a/src/modules/rust.rs +++ b/src/modules/rust.rs @@ -10,10 +10,10 @@ use super::{Context, Module}; /// - Current directory contains a `Cargo.toml` file pub fn module<'a>(context: &'a Context) -> Option> { let is_rs_project = context - .new_scan_dir() + .try_begin_scan()? .set_files(&["Cargo.toml"]) .set_extensions(&["rs"]) - .scan(); + .is_match(); if !is_rs_project { return None; diff --git a/src/modules/time.rs b/src/modules/time.rs index 0450e86f..61a68c72 100644 --- a/src/modules/time.rs +++ b/src/modules/time.rs @@ -41,8 +41,8 @@ pub fn module<'a>(context: &'a Context) -> Option> { /// Format a given time into the given string. This function should be referentially /// transparent, which makes it easy to test (unlike anything involving the actual time) -fn format_time(time_format: &str, localtime: DateTime) -> String { - localtime.format(time_format).to_string() +fn format_time(time_format: &str, local_time: DateTime) -> String { + local_time.format(time_format).to_string() } /* Because we cannot make acceptance tests for the time module, these unit diff --git a/src/segment.rs b/src/segment.rs index 8419ec31..ebae80d5 100644 --- a/src/segment.rs +++ b/src/segment.rs @@ -6,7 +6,7 @@ use std::fmt; /// (e.g. The version that software is running). pub struct Segment { /// The segment's name, to be used in configuration and logging. - name: String, + _name: String, /// The segment's style. If None, will inherit the style of the module containing it. style: Option