From 413f644048c6ac54d2290464e5d37e0126757e6e Mon Sep 17 00:00:00 2001 From: NIGHTMORE <297347036+220nightmore-spec@users.noreply.github.com> Date: Thu, 16 Jul 2026 17:32:24 +0800 Subject: [PATCH] Add line-backed parameter values --- README.md | 7 + doc/hyperfine.1 | 13 ++ src/benchmark/mod.rs | 2 +- src/benchmark/scheduler.rs | 15 +- src/cli.rs | 15 ++ src/command.rs | 177 ++++++++--------- src/command_product.rs | 210 ++++++++++++++++++++ src/main.rs | 1 + src/options.rs | 64 +++++- src/parameter/file_values.rs | 268 +++++++++++++++++++++++++ src/parameter/mod.rs | 3 + src/parameter/product.rs | 347 +++++++++++++++++++++++++++++++++ src/parameter/sources.rs | 205 +++++++++++++++++++ tests/execution_order_tests.rs | 30 ++- tests/integration_tests.rs | 21 ++ 15 files changed, 1270 insertions(+), 108 deletions(-) create mode 100644 src/command_product.rs create mode 100644 src/parameter/file_values.rs create mode 100644 src/parameter/product.rs create mode 100644 src/parameter/sources.rs diff --git a/README.md b/README.md index ff6020064..71f1f8206 100644 --- a/README.md +++ b/README.md @@ -87,6 +87,13 @@ option: hyperfine -L compiler gcc,clang '{compiler} -O2 main.cpp' ``` +Values can also be read one line at a time from a file with `--parameter-file`. LF and CRLF line +endings are supported, and the option can be combined with `--parameter-list`: +```sh +hyperfine --parameter-file compiler compilers.txt -L optimization O2,O3 \ + '{compiler} -{optimization} main.cpp' +``` + ### Intermediate shell By default, commands are executed using a predefined shell (`/bin/sh` on Unix, `cmd.exe` on Windows). diff --git a/doc/hyperfine.1 b/doc/hyperfine.1 index 449b470f2..c896687ac 100644 --- a/doc/hyperfine.1 +++ b/doc/hyperfine.1 @@ -29,6 +29,9 @@ hyperfine \- command\-line benchmarking tool .RB [ \-\-parameter\-list .IR VAR .IR VALUES ] +.RB [ \-\-parameter\-file +.IR VAR +.IR FILE ] .RB [ \-\-shell .IR SHELL ] .RB [ \-\-style @@ -193,6 +196,16 @@ This performs benchmarks for 'gcc \-O2 main.cpp' and 'clang \-O2 main.cpp'. The option can be specified multiple times to run benchmarks for all possible parameter combinations. .HP +\fB\-\-parameter\-file\fR \fIVAR\fP \fIFILE\fP +.IP +Perform benchmark runs for each line in \fIFILE\fP. Replaces the string +'{\fIVAR\fP}' in each command by the current line. LF and CRLF line endings +are supported, and values are read one line at a time. +.IP +The option can be specified multiple times and combined with +\fB\-\-parameter\-list\fR to run benchmarks for all possible parameter +combinations. +.HP \fB\-S\fR, \fB\-\-shell\fR \fISHELL\fP .IP Set the shell to use for executing benchmarked commands. This can be diff --git a/src/benchmark/mod.rs b/src/benchmark/mod.rs index e3534a7bc..97627d446 100644 --- a/src/benchmark/mod.rs +++ b/src/benchmark/mod.rs @@ -155,7 +155,7 @@ impl<'a> Benchmark<'a> { let mut exit_codes: Vec> = vec![]; let mut all_succeeded = true; - let output_policy = &self.options.command_output_policies[self.number]; + let output_policy = self.options.command_output_policy(self.number); let preparation_command = self.options.preparation_command.as_ref().map(|values| { let preparation_command = if values.len() == 1 { diff --git a/src/benchmark/scheduler.rs b/src/benchmark/scheduler.rs index 242dd0e7d..cc63f9b6d 100644 --- a/src/benchmark/scheduler.rs +++ b/src/benchmark/scheduler.rs @@ -46,13 +46,26 @@ impl<'a> Scheduler<'a> { executor.calibrate()?; - for (number, cmd) in reference.iter().chain(self.commands.iter()).enumerate() { + let mut number = 0; + if let Some(cmd) = reference.as_ref() { self.results .push(Benchmark::new(number, cmd, self.options, &*executor).run()?); // We export results after each individual benchmark, because // we would risk losing them if a later benchmark fails. self.export_manager.write_results(&self.results, true)?; + number += 1; + } + + for cmd in self.commands.iter()? { + let cmd = cmd?; + self.results + .push(Benchmark::new(number, &cmd, self.options, &*executor).run()?); + + // We export results after each individual benchmark, because + // we would risk losing them if a later benchmark fails. + self.export_manager.write_results(&self.results, true)?; + number += 1; } Ok(()) diff --git a/src/cli.rs b/src/cli.rs index b12f6d34c..0e43c9332 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -200,6 +200,21 @@ fn build_command() -> Command { possible parameter combinations.\n" ), ) + .arg( + Arg::new("parameter-file") + .long("parameter-file") + .action(ArgAction::Append) + .allow_hyphen_values(true) + .value_names(["VAR", "FILE"]) + .value_hint(ValueHint::FilePath) + .conflicts_with_all(["parameter-scan", "parameter-step-size"]) + .help( + "Perform benchmark runs for each line in FILE. Replaces the string '{VAR}' \ + in each command by the current line. LF and CRLF line endings are supported, \ + and values are read one line at a time. The option can be specified multiple \ + times and combined with --parameter-list.", + ), + ) .arg( Arg::new("shell") .long("shell") diff --git a/src/command.rs b/src/command.rs index 5d35cfaf7..788752563 100644 --- a/src/command.rs +++ b/src/command.rs @@ -1,20 +1,22 @@ use std::collections::BTreeMap; use std::fmt; +use std::slice; use std::str::FromStr; -use crate::parameter::tokenize::tokenize; +use crate::command_product::{LazyCommandProduct, LazyCommandProductIter}; use crate::parameter::ParameterValue; use crate::{ error::{OptionsError, ParameterScanError}, parameter::{ range_step::{Numeric, RangeStep}, + sources::product_from_cli_arguments, ParameterNameAndValue, }, }; use clap::{parser::ValuesRef, ArgMatches}; -use anyhow::{bail, Context, Result}; +use anyhow::{anyhow, bail, Context, Result}; use rust_decimal::Decimal; /// A command that should be benchmarked. @@ -130,8 +132,33 @@ impl<'a> Command<'a> { } } -/// A collection of commands that should be benchmarked -pub struct Commands<'a>(Vec>); +enum CommandSource<'a> { + Eager(Vec>), + Lazy(LazyCommandProduct<'a>), +} + +/// A collection of commands that should be benchmarked. +/// +/// Existing plain-command and numeric-scan paths stay eager. Text parameter +/// lists and files share a restartable lazy product so file-backed values are +/// not retained for every generated command. +pub struct Commands<'a>(CommandSource<'a>); + +pub enum CommandsIter<'commands, 'name> { + Eager(slice::Iter<'commands, Command<'name>>), + Lazy(LazyCommandProductIter<'commands, 'name>), +} + +impl<'name> Iterator for CommandsIter<'_, 'name> { + type Item = Result>; + + fn next(&mut self) -> Option { + match self { + Self::Eager(commands) => commands.next().cloned().map(Ok), + Self::Lazy(commands) => commands.next().map(|command| command.map_err(Into::into)), + } + } +} impl<'a> Commands<'a> { pub fn from_cli_arguments(matches: &'a ArgMatches) -> Result> { @@ -146,44 +173,20 @@ impl<'a> Commands<'a> { let step_size = matches .get_one::("parameter-step-size") .map(|s| s.as_str()); - Ok(Self(Self::get_parameter_scan_commands( - command_names, - command_strings, - args, - step_size, - )?)) - } else if let Some(args) = matches.get_many::("parameter-list") { + Ok(Self(CommandSource::Eager( + Self::get_parameter_scan_commands(command_names, command_strings, args, step_size)?, + ))) + } else if matches.get_many::("parameter-list").is_some() + || matches.get_many::("parameter-file").is_some() + { let command_names = command_names.map_or(vec![], |names| { names.map(|v| v.as_str()).collect::>() }); - let args: Vec<_> = args.map(|v| v.as_str()).collect::>(); - let param_names_and_values: Vec<(&str, Vec)> = args - .chunks_exact(2) - .map(|pair| { - let name = pair[0]; - let list_str = pair[1]; - (name, tokenize(list_str)) - }) - .collect(); - { - let duplicates = - Self::find_duplicates(param_names_and_values.iter().map(|(name, _)| *name)); - if !duplicates.is_empty() { - bail!("Duplicate parameter names: {}", &duplicates.join(", ")); - } - } - - let dimensions: Vec = std::iter::once(command_strings.len()) - .chain( - param_names_and_values - .iter() - .map(|(_, values)| values.len()), - ) - .collect(); - let param_space_size = dimensions.iter().product(); - if param_space_size == 0 { - return Ok(Self(Vec::new())); - } + let parameters = product_from_cli_arguments(matches)?; + let param_space_size = command_strings + .len() + .checked_mul(parameters.len()) + .ok_or_else(|| anyhow!("command product is too large"))?; // `--command-name` should appear exactly once or exactly B times, // where B is the total number of benchmarks. @@ -196,41 +199,11 @@ impl<'a> Commands<'a> { .into()); } - let mut i = 0; - let mut commands = Vec::with_capacity(param_space_size); - let mut index = vec![0usize; dimensions.len()]; - 'outer: loop { - let name = command_names - .get(i) - .or_else(|| command_names.first()) - .copied(); - i += 1; - - let (command_index, params_indices) = index.split_first().unwrap(); - let parameters: Vec<_> = param_names_and_values - .iter() - .zip(params_indices) - .map(|((name, values), i)| (*name, ParameterValue::Text(values[*i].clone()))) - .collect(); - commands.push(Command::new_parametrized( - name, - command_strings[*command_index], - parameters, - )); - - // Increment index, exiting loop on overflow. - for (i, n) in index.iter_mut().zip(dimensions.iter()) { - *i += 1; - if *i < *n { - continue 'outer; - } else { - *i = 0; - } - } - break 'outer; - } - - Ok(Self(commands)) + Ok(Self(CommandSource::Lazy(LazyCommandProduct::new( + command_names, + command_strings, + parameters, + )?))) } else { let command_names = command_names.map_or(vec![], |names| { names.map(|v| v.as_str()).collect::>() @@ -243,29 +216,23 @@ impl<'a> Commands<'a> { for (i, s) in command_strings.iter().enumerate() { commands.push(Command::new(command_names.get(i).copied(), s)); } - Ok(Self(commands)) + Ok(Self(CommandSource::Eager(commands))) } } - pub fn iter(&self) -> impl Iterator> { - self.0.iter() + pub fn iter(&self) -> Result> { + match &self.0 { + CommandSource::Eager(commands) => Ok(CommandsIter::Eager(commands.iter())), + CommandSource::Lazy(commands) => Ok(CommandsIter::Lazy(commands.iter()?)), + } } pub fn num_commands(&self, has_reference_command: bool) -> usize { - self.0.len() + if has_reference_command { 1 } else { 0 } - } - - /// Finds all the strings that appear multiple times in the input iterator, returning them in - /// sorted order. If no string appears more than once, the result is an empty vector. - fn find_duplicates<'b, I: IntoIterator>(i: I) -> Vec<&'b str> { - let mut counts = BTreeMap::<&'b str, usize>::new(); - for s in i { - *counts.entry(s).or_default() += 1; - } - counts - .into_iter() - .filter_map(|(k, n)| if n > 1 { Some(k) } else { None }) - .collect() + let command_count = match &self.0 { + CommandSource::Eager(commands) => commands.len(), + CommandSource::Lazy(commands) => commands.len(), + }; + command_count + if has_reference_command { 1 } else { 0 } } fn build_parameter_scan_commands<'b, T: Numeric>( @@ -405,7 +372,12 @@ fn test_build_commands_cross_product() { "echo {par1} {par2}", "printf '%s\n' {par1} {par2}", ]); - let result = Commands::from_cli_arguments(&matches).unwrap().0; + let result = Commands::from_cli_arguments(&matches) + .unwrap() + .iter() + .unwrap() + .collect::>>() + .unwrap(); // Iteration order: command list first, then parameters in listed order (here, "par1" before // "par2", which is distinct from their sorted order), with parameter values in listed order. @@ -441,7 +413,12 @@ fn test_build_parameter_list_commands() { "--command-name", "name-{foo}", ]); - let commands = Commands::from_cli_arguments(&matches).unwrap().0; + let commands = Commands::from_cli_arguments(&matches) + .unwrap() + .iter() + .unwrap() + .collect::>>() + .unwrap(); assert_eq!(commands.len(), 2); assert_eq!(commands[0].get_name(), "name-1"); assert_eq!(commands[1].get_name(), "name-2"); @@ -464,7 +441,12 @@ fn test_build_parameter_scan_commands() { "--command-name", "name-{val}", ]); - let commands = Commands::from_cli_arguments(&matches).unwrap().0; + let commands = Commands::from_cli_arguments(&matches) + .unwrap() + .iter() + .unwrap() + .collect::>>() + .unwrap(); assert_eq!(commands.len(), 2); assert_eq!(commands[0].get_name(), "name-1"); assert_eq!(commands[1].get_name(), "name-2"); @@ -494,7 +476,12 @@ fn test_build_parameter_scan_commands_named() { "--command-name", "sleep-2", ]); - let commands = Commands::from_cli_arguments(&matches).unwrap().0; + let commands = Commands::from_cli_arguments(&matches) + .unwrap() + .iter() + .unwrap() + .collect::>>() + .unwrap(); assert_eq!(commands.len(), 4); assert_eq!(commands[0].get_name(), "echo-1"); assert_eq!(commands[0].get_command_line(), "echo 1"); diff --git a/src/command_product.rs b/src/command_product.rs new file mode 100644 index 000000000..6fc9b1751 --- /dev/null +++ b/src/command_product.rs @@ -0,0 +1,210 @@ +use std::io; + +use crate::{ + command::Command, + parameter::{product::ParameterProduct, ParameterNameAndValue}, +}; + +/// Lazily expands parameterized commands. +/// +/// Base commands remain borrowed while each yielded command owns only the +/// current parameter values. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct LazyCommandProduct<'a> { + command_names: Vec<&'a str>, + command_strings: Vec<&'a str>, + parameters: ParameterProduct<'a>, + len: usize, +} + +impl<'a> LazyCommandProduct<'a> { + pub fn new( + command_names: Vec<&'a str>, + command_strings: Vec<&'a str>, + parameters: ParameterProduct<'a>, + ) -> io::Result { + let len = command_strings + .len() + .checked_mul(parameters.len()) + .ok_or_else(|| { + io::Error::new(io::ErrorKind::InvalidInput, "command product is too large") + })?; + + if command_names.len() > 1 && command_names.len() != len { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!( + "expected either one command name or {len}, got {}", + command_names.len() + ), + )); + } + + Ok(Self { + command_names, + command_strings, + parameters, + len, + }) + } + + pub fn len(&self) -> usize { + self.len + } + + pub fn is_empty(&self) -> bool { + self.len == 0 + } + + pub fn iter(&self) -> io::Result> { + Ok(LazyCommandProductIter { + product: self, + parameters: self.parameters.iter()?, + current_parameters: None, + command_index: 0, + yielded: 0, + }) + } +} + +pub struct LazyCommandProductIter<'product, 'name> { + product: &'product LazyCommandProduct<'name>, + parameters: crate::parameter::product::ParameterProductIter<'product, 'name>, + current_parameters: Option>>, + command_index: usize, + yielded: usize, +} + +impl<'name> Iterator for LazyCommandProductIter<'_, 'name> { + type Item = io::Result>; + + fn next(&mut self) -> Option { + if self.product.command_strings.is_empty() { + return None; + } + + if self.current_parameters.is_none() { + self.current_parameters = match self.parameters.next()? { + Ok(parameters) => Some(parameters), + Err(error) => return Some(Err(error)), + }; + self.command_index = 0; + } + + let name = self + .product + .command_names + .get(self.yielded) + .or_else(|| self.product.command_names.first()) + .copied(); + let command = Command::new_parametrized( + name, + self.product.command_strings[self.command_index], + self.current_parameters.as_ref().unwrap().clone(), + ); + + self.command_index += 1; + self.yielded += 1; + if self.command_index == self.product.command_strings.len() { + self.current_parameters = None; + } + + Some(Ok(command)) + } +} + +#[cfg(test)] +mod tests { + use super::LazyCommandProduct; + use crate::parameter::{ + file_values::FileValues, + product::{ParameterProduct, ParameterSource, ValuesSource}, + }; + use std::io::Write; + use tempfile::NamedTempFile; + + #[test] + fn base_command_changes_before_the_first_parameter() { + let mut file = NamedTempFile::new().unwrap(); + file.write_all(b"x\ny\n").unwrap(); + file.flush().unwrap(); + + let parameters = ParameterProduct::new(vec![ + ParameterSource::new( + "inline", + ValuesSource::inline(["a".to_owned(), "b".to_owned()]), + ), + ParameterSource::new( + "file", + ValuesSource::file(FileValues::open(file.path()).unwrap()), + ), + ]) + .unwrap(); + let commands = LazyCommandProduct::new( + vec!["benchmark-{inline}-{file}"], + vec!["echo {inline} {file}", "printf {inline} {file}"], + parameters, + ) + .unwrap(); + let lines = commands + .iter() + .unwrap() + .map(|command| command.unwrap().get_command_line()) + .collect::>(); + + assert_eq!(commands.len(), 8); + assert_eq!( + lines, + [ + "echo a x", + "printf a x", + "echo b x", + "printf b x", + "echo a y", + "printf a y", + "echo b y", + "printf b y", + ] + ); + } + + #[test] + fn supports_one_name_or_one_name_per_generated_command() { + let parameters = ParameterProduct::new(vec![ParameterSource::new( + "value", + ValuesSource::inline(["a".to_owned(), "b".to_owned()]), + )]) + .unwrap(); + let commands = + LazyCommandProduct::new(vec!["first", "second"], vec!["echo {value}"], parameters) + .unwrap(); + let names = commands + .iter() + .unwrap() + .map(|command| command.unwrap().get_name()) + .collect::>(); + + assert_eq!(names, ["first", "second"]); + } + + #[test] + fn rejects_an_ambiguous_command_name_count() { + let parameters = ParameterProduct::new(vec![ParameterSource::new( + "value", + ValuesSource::inline(["a".to_owned(), "b".to_owned()]), + )]) + .unwrap(); + let error = LazyCommandProduct::new( + vec!["first", "second", "third"], + vec!["echo {value}"], + parameters, + ) + .unwrap_err(); + + assert_eq!(error.kind(), std::io::ErrorKind::InvalidInput); + assert_eq!( + error.to_string(), + "expected either one command name or 2, got 3" + ); + } +} diff --git a/src/main.rs b/src/main.rs index 2a9750503..a6ac5b49f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -17,6 +17,7 @@ use colored::*; pub mod benchmark; pub mod cli; pub mod command; +pub mod command_product; pub mod error; pub mod export; pub mod options; diff --git a/src/options.rs b/src/options.rs index 7c83da1c5..4842a7eb2 100644 --- a/src/options.rs +++ b/src/options.rs @@ -487,18 +487,22 @@ impl Options { ); } + ensure!( + self.command_output_policies.len() == 1 + || self.command_output_policies.len() == num_commands, + "The '--output' option has to be provided just once or N times, where N={num_commands} is the \ + number of benchmark commands (including a potential reference)." + ); + + Ok(()) + } + + pub fn command_output_policy(&self, command_index: usize) -> &CommandOutputPolicy { if self.command_output_policies.len() == 1 { - self.command_output_policies = - vec![self.command_output_policies[0].clone(); num_commands]; + &self.command_output_policies[0] } else { - ensure!( - self.command_output_policies.len() == num_commands, - "The '--output' option has to be provided just once or N times, where N={num_commands} is the \ - number of benchmark commands (including a potential reference)." - ); + &self.command_output_policies[command_index] } - - Ok(()) } } @@ -545,3 +549,45 @@ fn test_can_parse_shell_command_line_from_str() { OptionsError::EmptyShell )); } + +#[test] +fn singleton_output_policy_is_not_expanded_per_command() { + use crate::{cli::get_cli_arguments, command::Commands}; + + let matches = get_cli_arguments(["hyperfine", "echo first", "echo second", "--output=inherit"]); + let commands = Commands::from_cli_arguments(&matches).unwrap(); + let mut options = Options::from_cli_arguments(&matches).unwrap(); + + options.validate_against_command_list(&commands).unwrap(); + + assert_eq!(options.command_output_policies.len(), 1); + assert_eq!( + options.command_output_policy(0), + &CommandOutputPolicy::Inherit + ); + assert_eq!( + options.command_output_policy(1), + &CommandOutputPolicy::Inherit + ); +} + +#[test] +fn per_command_output_policies_keep_their_existing_indices() { + use crate::{cli::get_cli_arguments, command::Commands}; + + let matches = get_cli_arguments([ + "hyperfine", + "echo first", + "echo second", + "--output=null", + "--output=pipe", + ]); + let commands = Commands::from_cli_arguments(&matches).unwrap(); + let mut options = Options::from_cli_arguments(&matches).unwrap(); + + options.validate_against_command_list(&commands).unwrap(); + + assert_eq!(options.command_output_policies.len(), 2); + assert_eq!(options.command_output_policy(0), &CommandOutputPolicy::Null); + assert_eq!(options.command_output_policy(1), &CommandOutputPolicy::Pipe); +} diff --git a/src/parameter/file_values.rs b/src/parameter/file_values.rs new file mode 100644 index 000000000..b54a58ba7 --- /dev/null +++ b/src/parameter/file_values.rs @@ -0,0 +1,268 @@ +use std::{ + fs::File, + io::{self, BufRead, BufReader, Lines}, + path::{Path, PathBuf}, +}; + +/// A parameter source backed by lines in a UTF-8 file. +/// +/// Construction performs the count pass allowed by issue #813, but retains only +/// the path and line count. Each call to [`FileValues::iter`] opens the file +/// again and yields one owned line at a time. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct FileValues { + path: PathBuf, + line_count: usize, +} + +impl FileValues { + pub fn open(path: impl Into) -> io::Result { + let path = path.into(); + let line_count = count_lines(&path)?; + + Ok(Self { path, line_count }) + } + + pub fn path(&self) -> &Path { + &self.path + } + + pub fn len(&self) -> usize { + self.line_count + } + + pub fn is_empty(&self) -> bool { + self.line_count == 0 + } + + pub fn iter(&self) -> io::Result { + let file = File::open(&self.path).map_err(|error| path_error(&self.path, "open", error))?; + let reader = BufReader::new(file); + Ok(FileValueLines { + lines: reader.lines(), + path: self.path.clone(), + expected_lines: self.line_count, + yielded_lines: 0, + finished: false, + }) + } +} + +pub struct FileValueLines { + lines: Lines>, + path: PathBuf, + expected_lines: usize, + yielded_lines: usize, + finished: bool, +} + +impl Iterator for FileValueLines { + type Item = io::Result; + + fn next(&mut self) -> Option { + if self.finished { + return None; + } + + match self.lines.next() { + Some(Ok(line)) if self.yielded_lines < self.expected_lines => { + self.yielded_lines += 1; + Some(Ok(line)) + } + Some(Ok(_)) => { + self.finished = true; + Some(Err(io::Error::new( + io::ErrorKind::InvalidData, + format!( + "parameter file '{}' contains more lines than during the initial count pass", + self.path.display() + ), + ))) + } + Some(Err(error)) => { + self.finished = true; + Some(Err(path_error(&self.path, "read", error))) + } + None if self.yielded_lines == self.expected_lines => { + self.finished = true; + None + } + None => { + self.finished = true; + Some(Err(io::Error::new( + io::ErrorKind::UnexpectedEof, + format!( + "parameter file '{}' contains fewer lines than during the initial count pass", + self.path.display() + ), + ))) + } + } + } +} + +fn count_lines(path: &Path) -> io::Result { + let file = File::open(path).map_err(|error| path_error(path, "open", error))?; + BufReader::new(file) + .lines() + .try_fold(0_usize, |count, line| { + line.map_err(|error| path_error(path, "read", error))?; + count.checked_add(1).ok_or_else(|| { + io::Error::other(format!( + "parameter file '{}' has too many lines", + path.display() + )) + }) + }) +} + +fn path_error(path: &Path, action: &str, error: io::Error) -> io::Error { + io::Error::new( + error.kind(), + format!( + "failed to {action} parameter file '{}': {error}", + path.display() + ), + ) +} + +#[cfg(test)] +mod tests { + use super::FileValues; + use std::{ + fs, + io::{BufWriter, Write}, + }; + use tempfile::NamedTempFile; + + fn file_with_contents(contents: &[u8]) -> NamedTempFile { + let mut file = NamedTempFile::new().unwrap(); + file.write_all(contents).unwrap(); + file.flush().unwrap(); + file + } + + fn values(source: &FileValues) -> Vec { + source + .iter() + .unwrap() + .collect::, _>>() + .unwrap() + } + + #[test] + fn supports_lf_crlf_blank_lines_and_a_final_line_without_newline() { + let lf = file_with_contents(b"alpha\n\nomega"); + let crlf = file_with_contents(b"alpha\r\n\r\nomega"); + + let lf_source = FileValues::open(lf.path()).unwrap(); + let crlf_source = FileValues::open(crlf.path()).unwrap(); + + assert_eq!(lf_source.len(), 3); + assert_eq!(crlf_source.len(), 3); + assert_eq!(values(&lf_source), ["alpha", "", "omega"]); + assert_eq!(values(&crlf_source), values(&lf_source)); + } + + #[test] + fn trailing_newline_does_not_create_an_extra_value() { + let file = file_with_contents(b"alpha\nbeta\n"); + let source = FileValues::open(file.path()).unwrap(); + + assert_eq!(source.len(), 2); + assert_eq!(values(&source), ["alpha", "beta"]); + } + + #[test] + fn empty_file_yields_no_values() { + let file = file_with_contents(b""); + let source = FileValues::open(file.path()).unwrap(); + + assert!(source.is_empty()); + assert!(values(&source).is_empty()); + } + + #[test] + fn can_reopen_and_iterate_again() { + let file = file_with_contents(b"one\ntwo\n"); + let source = FileValues::open(file.path()).unwrap(); + + assert_eq!(values(&source), ["one", "two"]); + assert_eq!(values(&source), ["one", "two"]); + } + + #[test] + fn rejects_invalid_utf8_during_the_count_pass() { + let file = file_with_contents(&[b'a', b'\n', 0xff, b'\n']); + let error = FileValues::open(file.path()).unwrap_err(); + + assert_eq!(error.kind(), std::io::ErrorKind::InvalidData); + assert!(error + .to_string() + .contains(&file.path().display().to_string())); + } + + #[test] + fn rejects_a_file_truncated_after_the_count_pass() { + let file = file_with_contents(b"one\ntwo\n"); + let source = FileValues::open(file.path()).unwrap(); + fs::write(file.path(), b"one\n").unwrap(); + + let error = source + .iter() + .unwrap() + .collect::, _>>() + .unwrap_err(); + + assert_eq!(error.kind(), std::io::ErrorKind::UnexpectedEof); + assert!(error.to_string().contains("fewer lines")); + assert!(error + .to_string() + .contains(&file.path().display().to_string())); + } + + #[test] + fn rejects_a_file_extended_after_the_count_pass() { + let file = file_with_contents(b"one\ntwo\n"); + let source = FileValues::open(file.path()).unwrap(); + fs::write(file.path(), b"one\ntwo\nthree\n").unwrap(); + + let error = source + .iter() + .unwrap() + .collect::, _>>() + .unwrap_err(); + + assert_eq!(error.kind(), std::io::ErrorKind::InvalidData); + assert!(error.to_string().contains("more lines")); + assert!(error + .to_string() + .contains(&file.path().display().to_string())); + } + + #[test] + fn counts_two_million_lines_without_retaining_the_values() { + const LINE_COUNT: usize = 2_000_000; + + let mut file = NamedTempFile::new().unwrap(); + { + let mut writer = BufWriter::new(file.as_file_mut()); + for _ in 0..LINE_COUNT { + writer.write_all(b"value\n").unwrap(); + } + writer.flush().unwrap(); + } + + let source = FileValues::open(file.path()).unwrap(); + assert_eq!(source.len(), LINE_COUNT); + assert_eq!( + source + .iter() + .unwrap() + .take(3) + .collect::, _>>() + .unwrap(), + ["value", "value", "value"] + ); + } +} diff --git a/src/parameter/mod.rs b/src/parameter/mod.rs index 579a3bf3b..da3af87c0 100644 --- a/src/parameter/mod.rs +++ b/src/parameter/mod.rs @@ -1,7 +1,10 @@ use crate::util::number::Number; use std::fmt::Display; +pub mod file_values; +pub mod product; pub mod range_step; +pub mod sources; pub mod tokenize; #[derive(Debug, Clone, PartialEq, Eq)] diff --git a/src/parameter/product.rs b/src/parameter/product.rs new file mode 100644 index 000000000..b2a2f55fe --- /dev/null +++ b/src/parameter/product.rs @@ -0,0 +1,347 @@ +use std::{io, slice}; + +use super::{ + file_values::{FileValueLines, FileValues}, + ParameterNameAndValue, ParameterValue, +}; + +/// A restartable source of text parameter values. +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum ValuesSource { + Inline(Vec), + File(FileValues), +} + +impl ValuesSource { + pub fn inline(values: impl IntoIterator) -> Self { + Self::Inline(values.into_iter().collect()) + } + + pub fn file(values: FileValues) -> Self { + Self::File(values) + } + + pub fn len(&self) -> usize { + match self { + Self::Inline(values) => values.len(), + Self::File(values) => values.len(), + } + } + + pub fn is_empty(&self) -> bool { + self.len() == 0 + } + + fn iter(&self) -> io::Result> { + match self { + Self::Inline(values) => Ok(ValuesSourceIter::Inline(values.iter())), + Self::File(values) => Ok(ValuesSourceIter::File(values.iter()?)), + } + } +} + +enum ValuesSourceIter<'a> { + Inline(slice::Iter<'a, String>), + File(FileValueLines), +} + +impl Iterator for ValuesSourceIter<'_> { + type Item = io::Result; + + fn next(&mut self) -> Option { + match self { + Self::Inline(values) => values.next().map(|value| Ok(value.clone())), + Self::File(values) => values.next(), + } + } +} + +/// One named dimension in a parameter product. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct ParameterSource<'a> { + name: &'a str, + values: ValuesSource, +} + +impl<'a> ParameterSource<'a> { + pub fn new(name: &'a str, values: ValuesSource) -> Self { + Self { name, values } + } + + pub fn name(&self) -> &'a str { + self.name + } + + pub fn values(&self) -> &ValuesSource { + &self.values + } +} + +/// Restartable Cartesian product of parameter sources. +/// +/// The first source changes fastest, matching the ordering used by the current +/// eager `--parameter-list` implementation. File sources retain only their +/// path/count metadata here; an iterator holds just the active line from each +/// source. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct ParameterProduct<'a> { + sources: Vec>, + len: usize, +} + +impl<'a> ParameterProduct<'a> { + pub fn new(sources: Vec>) -> io::Result { + let len = sources.iter().try_fold(1_usize, |product, source| { + product.checked_mul(source.values.len()).ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + "parameter product is too large", + ) + }) + })?; + + Ok(Self { sources, len }) + } + + pub fn len(&self) -> usize { + self.len + } + + pub fn is_empty(&self) -> bool { + self.len == 0 + } + + pub fn sources(&self) -> &[ParameterSource<'a>] { + &self.sources + } + + pub fn iter(&self) -> io::Result> { + let mut iterators = self + .sources + .iter() + .map(|source| source.values.iter()) + .collect::>>()?; + + let mut current = Vec::with_capacity(self.sources.len()); + let mut finished = self.is_empty(); + if !finished { + for values in &mut iterators { + match values.next() { + Some(Ok(value)) => current.push(value), + Some(Err(error)) => return Err(error), + None => { + finished = true; + break; + } + } + } + } + + Ok(ParameterProductIter { + product: self, + iterators, + current, + started: false, + finished, + }) + } +} + +pub struct ParameterProductIter<'product, 'name> { + product: &'product ParameterProduct<'name>, + iterators: Vec>, + current: Vec, + started: bool, + finished: bool, +} + +impl ParameterProductIter<'_, '_> { + fn advance(&mut self) -> io::Result<()> { + for index in 0..self.iterators.len() { + match self.iterators[index].next() { + Some(Ok(value)) => { + self.current[index] = value; + return Ok(()); + } + Some(Err(error)) => { + self.finished = true; + return Err(error); + } + None => { + let mut reset = match self.product.sources[index].values.iter() { + Ok(reset) => reset, + Err(error) => { + self.finished = true; + return Err(error); + } + }; + match reset.next() { + Some(Ok(value)) => { + self.current[index] = value; + self.iterators[index] = reset; + } + Some(Err(error)) => { + self.finished = true; + return Err(error); + } + None => { + self.finished = true; + return Ok(()); + } + } + } + } + } + + self.finished = true; + Ok(()) + } +} + +impl<'name> Iterator for ParameterProductIter<'_, 'name> { + type Item = io::Result>>; + + fn next(&mut self) -> Option { + if self.finished { + return None; + } + + if self.started { + if let Err(error) = self.advance() { + return Some(Err(error)); + } + if self.finished { + return None; + } + } else { + self.started = true; + } + + Some(Ok(self + .product + .sources + .iter() + .zip(&self.current) + .map(|(source, value)| (source.name, ParameterValue::Text(value.clone()))) + .collect())) + } +} + +#[cfg(test)] +mod tests { + use super::{ParameterProduct, ParameterSource, ValuesSource}; + use crate::parameter::{file_values::FileValues, ParameterValue}; + use std::io::{BufWriter, Write}; + use tempfile::NamedTempFile; + + fn text_values(parameters: Vec<(&str, ParameterValue)>) -> Vec<(&str, String)> { + parameters + .into_iter() + .map(|(name, value)| match value { + ParameterValue::Text(value) => (name, value), + ParameterValue::Numeric(_) => unreachable!(), + }) + .collect() + } + + #[test] + fn preserves_existing_first_dimension_fastest_order() { + let mut file = NamedTempFile::new().unwrap(); + file.write_all(b"x\ny\n").unwrap(); + file.flush().unwrap(); + + let product = ParameterProduct::new(vec![ + ParameterSource::new( + "inline", + ValuesSource::inline(["a".to_owned(), "b".to_owned()]), + ), + ParameterSource::new( + "file", + ValuesSource::file(FileValues::open(file.path()).unwrap()), + ), + ]) + .unwrap(); + + let combinations = product + .iter() + .unwrap() + .map(|item| text_values(item.unwrap())) + .collect::>(); + + assert_eq!(product.len(), 4); + assert_eq!( + combinations, + [ + [("inline", "a".into()), ("file", "x".into())], + [("inline", "b".into()), ("file", "x".into())], + [("inline", "a".into()), ("file", "y".into())], + [("inline", "b".into()), ("file", "y".into())], + ] + ); + } + + #[test] + fn empty_source_produces_no_combinations() { + let product = ParameterProduct::new(vec![ParameterSource::new( + "empty", + ValuesSource::inline([]), + )]) + .unwrap(); + + assert!(product.is_empty()); + assert_eq!(product.iter().unwrap().count(), 0); + } + + #[test] + fn no_sources_produces_one_empty_combination() { + let product = ParameterProduct::new(vec![]).unwrap(); + let combinations = product + .iter() + .unwrap() + .collect::, _>>() + .unwrap(); + + assert_eq!(product.len(), 1); + assert_eq!(combinations, [vec![]]); + } + + #[test] + fn iterates_two_million_file_values_without_materializing_the_product() { + const LINE_COUNT: usize = 2_000_000; + + let mut file = NamedTempFile::new().unwrap(); + { + let mut writer = BufWriter::new(file.as_file_mut()); + for index in 0..LINE_COUNT { + writeln!(writer, "value-{index}").unwrap(); + } + writer.flush().unwrap(); + } + + let product = ParameterProduct::new(vec![ParameterSource::new( + "value", + ValuesSource::file(FileValues::open(file.path()).unwrap()), + )]) + .unwrap(); + let first = product + .iter() + .unwrap() + .take(3) + .map(|item| text_values(item.unwrap())) + .collect::>(); + + assert_eq!(product.len(), LINE_COUNT); + assert!(matches!( + product.sources()[0].values(), + ValuesSource::File(_) + )); + assert_eq!( + first, + [ + [("value", "value-0".into())], + [("value", "value-1".into())], + [("value", "value-2".into())], + ] + ); + } +} diff --git a/src/parameter/sources.rs b/src/parameter/sources.rs new file mode 100644 index 000000000..350de4cac --- /dev/null +++ b/src/parameter/sources.rs @@ -0,0 +1,205 @@ +use std::{collections::BTreeMap, io, path::Path}; + +use clap::ArgMatches; + +use super::{ + file_values::FileValues, + product::{ParameterProduct, ParameterSource, ValuesSource}, + tokenize::tokenize, +}; + +/// Build a lazy parameter product while preserving the order in which +/// `--parameter-list` and `--parameter-file` occurrences appeared on the +/// command line. +pub fn product_from_cli_arguments<'a>(matches: &'a ArgMatches) -> io::Result> { + let mut indexed_sources = Vec::new(); + + for (index, name, values) in argument_pairs(matches, "parameter-list") { + indexed_sources.push(( + index, + ParameterSource::new(name, ValuesSource::inline(tokenize(values))), + )); + } + + for (index, name, path) in argument_pairs(matches, "parameter-file") { + indexed_sources.push(( + index, + ParameterSource::new(name, ValuesSource::file(FileValues::open(Path::new(path))?)), + )); + } + + indexed_sources.sort_by_key(|(index, _)| *index); + + let mut name_counts = BTreeMap::new(); + for (_, source) in &indexed_sources { + *name_counts.entry(source.name()).or_insert(0_usize) += 1; + } + let duplicates = name_counts + .into_iter() + .filter_map(|(name, count)| (count > 1).then_some(name)) + .collect::>(); + if !duplicates.is_empty() { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!("Duplicate parameter names: {}", duplicates.join(", ")), + )); + } + + ParameterProduct::new( + indexed_sources + .into_iter() + .map(|(_, source)| source) + .collect(), + ) +} + +fn argument_pairs<'a>(matches: &'a ArgMatches, id: &str) -> Vec<(usize, &'a str, &'a str)> { + let values = matches + .get_many::(id) + .into_iter() + .flatten() + .map(String::as_str) + .collect::>(); + let indices = matches + .indices_of(id) + .into_iter() + .flatten() + .collect::>(); + + debug_assert_eq!(values.len(), indices.len()); + debug_assert_eq!(values.len() % 2, 0); + + indices + .chunks_exact(2) + .zip(values.chunks_exact(2)) + .map(|(indices, values)| (indices[0], values[0], values[1])) + .collect() +} + +#[cfg(test)] +mod tests { + use std::io::Write; + + use tempfile::NamedTempFile; + + use super::product_from_cli_arguments; + use crate::{cli::get_cli_arguments, parameter::ParameterValue}; + + fn text_values(parameters: Vec<(&str, ParameterValue)>) -> Vec<(&str, String)> { + parameters + .into_iter() + .map(|(name, value)| match value { + ParameterValue::Text(value) => (name, value), + ParameterValue::Numeric(_) => unreachable!(), + }) + .collect() + } + + #[test] + fn preserves_interspersed_list_and_file_order() { + let mut first_file = NamedTempFile::new().unwrap(); + first_file.write_all(b"x\ny\n").unwrap(); + first_file.flush().unwrap(); + + let mut last_file = NamedTempFile::new().unwrap(); + last_file.write_all(b"q\nr\n").unwrap(); + last_file.flush().unwrap(); + + let matches = get_cli_arguments([ + "hyperfine", + "--parameter-file", + "first", + first_file.path().to_str().unwrap(), + "-L", + "middle", + "a,b", + "--parameter-file", + "last", + last_file.path().to_str().unwrap(), + "echo {first} {middle} {last}", + ]); + let product = product_from_cli_arguments(&matches).unwrap(); + + assert_eq!( + product + .sources() + .iter() + .map(|source| source.name()) + .collect::>(), + ["first", "middle", "last"] + ); + assert_eq!(product.len(), 8); + assert_eq!( + product + .iter() + .unwrap() + .take(5) + .map(|item| text_values(item.unwrap())) + .collect::>(), + [ + [ + ("first", "x".into()), + ("middle", "a".into()), + ("last", "q".into()) + ], + [ + ("first", "y".into()), + ("middle", "a".into()), + ("last", "q".into()) + ], + [ + ("first", "x".into()), + ("middle", "b".into()), + ("last", "q".into()) + ], + [ + ("first", "y".into()), + ("middle", "b".into()), + ("last", "q".into()) + ], + [ + ("first", "x".into()), + ("middle", "a".into()), + ("last", "r".into()) + ], + ] + ); + } + + #[test] + fn rejects_duplicate_names_across_source_kinds() { + let mut file = NamedTempFile::new().unwrap(); + file.write_all(b"x\n").unwrap(); + file.flush().unwrap(); + + let matches = get_cli_arguments([ + "hyperfine", + "-L", + "value", + "a,b", + "--parameter-file", + "value", + file.path().to_str().unwrap(), + "echo {value}", + ]); + let error = product_from_cli_arguments(&matches).unwrap_err(); + + assert_eq!(error.kind(), std::io::ErrorKind::InvalidInput); + assert_eq!(error.to_string(), "Duplicate parameter names: value"); + } + + #[test] + fn reports_a_missing_file_with_path_context() { + let missing = std::env::temp_dir().join("hyperfine-813-missing-parameter-values.txt"); + let matches = get_cli_arguments([ + "hyperfine", + "--parameter-file", + "value", + missing.to_str().unwrap(), + "echo {value}", + ]); + let error = product_from_cli_arguments(&matches).unwrap_err(); + + assert!(error.to_string().contains(missing.to_str().unwrap())); + } +} diff --git a/tests/execution_order_tests.rs b/tests/execution_order_tests.rs index 6ccc2e5ed..e197c48ad 100644 --- a/tests/execution_order_tests.rs +++ b/tests/execution_order_tests.rs @@ -1,6 +1,10 @@ -use std::{fs::File, io::Read, path::PathBuf}; +use std::{ + fs::File, + io::{Read, Write}, + path::PathBuf, +}; -use tempfile::{tempdir, TempDir}; +use tempfile::{tempdir, NamedTempFile, TempDir}; mod common; use common::hyperfine; @@ -370,6 +374,28 @@ fn multiple_parameter_values() { .run(); } +#[test] +fn parameter_file_values_are_executed_in_cli_product_order() { + let mut values = NamedTempFile::new().unwrap(); + values.write_all(b"1\r\n2\n").unwrap(); + values.flush().unwrap(); + + ExecutionOrderTest::new() + .arg("--runs=1") + .arg("--parameter-file") + .arg("number") + .arg(values.path().to_string_lossy()) + .arg("--parameter-list") + .arg("letter") + .arg("a,b") + .command("command {number} {letter}") + .expect_output("command 1 a") + .expect_output("command 2 a") + .expect_output("command 1 b") + .expect_output("command 2 b") + .run(); +} + #[test] fn reference_is_executed_first() { ExecutionOrderTest::new() diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index 2de3a5049..db47506f9 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -169,6 +169,27 @@ fn fails_with_duplicate_parameter_names() { .stderr(predicate::str::contains("Duplicate parameter names: x")); } +#[test] +fn fails_with_duplicate_parameter_names_across_list_and_file() { + use std::io::Write; + + let mut values = tempfile::NamedTempFile::new().unwrap(); + values.write_all(b"a\nb\n").unwrap(); + values.flush().unwrap(); + + hyperfine() + .arg("--parameter-list") + .arg("x") + .arg("1,2,3") + .arg("--parameter-file") + .arg("x") + .arg(values.path()) + .arg("echo test") + .assert() + .failure() + .stderr(predicate::str::contains("Duplicate parameter names: x")); +} + #[test] fn fails_for_unknown_command() { hyperfine()