From 325d419e39733a8511471438ef021a8b4116c12f Mon Sep 17 00:00:00 2001 From: Thayne McCombs Date: Wed, 25 Oct 2023 02:00:31 -0600 Subject: [PATCH 1/2] Replace num_cpus with std::thread::available_parallelism I think this might help startup time a little bit, since it looks like on linux at least the std implementation uses syscalls to determine the parallelism, whereas num_cpus was processing the /proc/cpuinfo file. It also removes another dependency. --- Cargo.lock | 17 ----------------- Cargo.toml | 1 - src/cli.rs | 19 ++++++++++++++----- 3 files changed, 14 insertions(+), 23 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 64efc0a7f..fe3273044 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -312,7 +312,6 @@ dependencies = [ "nix 0.26.4", "normpath", "nu-ansi-term", - "num_cpus", "regex", "regex-syntax 0.7.5", "tempfile", @@ -357,12 +356,6 @@ version = "0.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "95505c38b4572b2d910cecb0281560f54b440a19336cbbcb27bf6ce6adc6f5a8" -[[package]] -name = "hermit-abi" -version = "0.3.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d77f7ec81a6d05a3abb01ab6eb7590f6083d08449fe5a1c8b1e620283546ccb7" - [[package]] name = "home" version = "0.5.5" @@ -546,16 +539,6 @@ dependencies = [ "autocfg", ] -[[package]] -name = "num_cpus" -version = "1.16.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4161fcb6d602d4d2081af7c3a45852d875a03dd337a6bfdd6e06407b61342a43" -dependencies = [ - "hermit-abi", - "libc", -] - [[package]] name = "once_cell" version = "1.18.0" diff --git a/Cargo.toml b/Cargo.toml index 2eb133dfa..5eb578197 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,7 +38,6 @@ aho-corasick = "1.0" nu-ansi-term = "0.49" argmax = "0.3.1" ignore = "0.4.20" -num_cpus = "1.16" regex = "1.9.6" regex-syntax = "0.7" ctrlc = "3.2" diff --git a/src/cli.rs b/src/cli.rs index 541b7768d..a8cfb3d2c 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1,3 +1,4 @@ +use std::num::NonZeroUsize; use std::path::{Path, PathBuf}; use std::time::Duration; @@ -693,11 +694,9 @@ impl Opts { // unlikely fd will be running in such an environment, and even more unlikely someone would // be trying to use that many threads on such an environment, so I think panicing is an // appropriate way to handle that. - std::cmp::max( - self.threads - .map_or_else(num_cpus::get, |n| n.try_into().expect("too many threads")), - 1, - ) + self.threads.map_or_else(default_num_threads, |n| { + std::cmp::max(n.try_into().expect("too many threads"), 1) + }) } pub fn max_results(&self) -> Option { @@ -719,6 +718,16 @@ impl Opts { } } +/// Get the default number of threads to use, if not explicitly specified. +fn default_num_threads() -> usize { + // If we can't get the amount of parallelism for some reason, then + // default to a single thread, because that is safe. + const FALLBACK_PARALLELISM: NonZeroUsize = NonZeroUsize::MIN; + std::thread::available_parallelism() + .unwrap_or(FALLBACK_PARALLELISM) + .get() +} + #[derive(Copy, Clone, PartialEq, Eq, ValueEnum)] pub enum FileType { #[value(alias = "f")] From 1d57b3a06453d2602fc90e019a30c1ed9b792748 Mon Sep 17 00:00:00 2001 From: Thayne McCombs Date: Wed, 25 Oct 2023 23:56:59 -0600 Subject: [PATCH 2/2] Simplify parsing number of threads. --- src/cli.rs | 24 +++++++++--------------- src/main.rs | 2 +- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index a8cfb3d2c..abb559883 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -498,8 +498,8 @@ pub struct Opts { /// Set number of threads to use for searching & executing (default: number /// of available CPU cores) - #[arg(long, short = 'j', value_name = "num", hide_short_help = true, value_parser = clap::value_parser!(u32).range(1..))] - pub threads: Option, + #[arg(long, short = 'j', value_name = "num", hide_short_help = true, value_parser = str::parse::)] + pub threads: Option, /// Milliseconds to buffer before streaming search results to console /// @@ -688,15 +688,8 @@ impl Opts { self.min_depth.or(self.exact_depth) } - pub fn threads(&self) -> usize { - // This will panic if the number of threads passed in is more than usize::MAX in an environment - // where usize is less than 32 bits (for example 16-bit architectures). It's pretty - // unlikely fd will be running in such an environment, and even more unlikely someone would - // be trying to use that many threads on such an environment, so I think panicing is an - // appropriate way to handle that. - self.threads.map_or_else(default_num_threads, |n| { - std::cmp::max(n.try_into().expect("too many threads"), 1) - }) + pub fn threads(&self) -> NonZeroUsize { + self.threads.unwrap_or_else(default_num_threads) } pub fn max_results(&self) -> Option { @@ -719,13 +712,14 @@ impl Opts { } /// Get the default number of threads to use, if not explicitly specified. -fn default_num_threads() -> usize { +fn default_num_threads() -> NonZeroUsize { // If we can't get the amount of parallelism for some reason, then // default to a single thread, because that is safe. + // Note that the minimum value for a NonZeroUsize is 1. + // Unfortunately, we can't do `NonZeroUsize::new(1).unwrap()` + // in a const context. const FALLBACK_PARALLELISM: NonZeroUsize = NonZeroUsize::MIN; - std::thread::available_parallelism() - .unwrap_or(FALLBACK_PARALLELISM) - .get() + std::thread::available_parallelism().unwrap_or(FALLBACK_PARALLELISM) } #[derive(Copy, Clone, PartialEq, Eq, ValueEnum)] diff --git a/src/main.rs b/src/main.rs index a1bad2b38..b661f672b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -252,7 +252,7 @@ fn construct_config(mut opts: Opts, pattern_regexps: &[String]) -> Result