Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Implement slowness checking and --reference flag to use a command as a reference when printing or exporting summary #579

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Animeshz
Copy link

As title enlists.

Closes #577.

…a reference when printing or exporting summary
@Animeshz
Copy link
Author

Example run:

$ hyperfine -N --reference 'bash -c "echo \"Hello, World!\""' -L shell dash,fish '{shell} -c "echo \"Hello, world!\""'
    Finished dev [unoptimized + debuginfo] target(s) in 0.08s
     Running `target/debug/hyperfine -N --reference 'bash -c "echo \"Hello, World'\!'\""' -L shell dash,fish '{shell} -c "echo \"Hello, world'\!'\""'`
Benchmark 1: bash -c "echo \"Hello, World!\""
  Time (mean ± σ):       4.0 ms ±   0.1 ms    [User: 2.8 ms, System: 1.1 ms]
  Range (min … max):     3.7 ms …   4.9 ms    677 runs
 
Benchmark 2: dash -c "echo \"Hello, world!\""
  Time (mean ± σ):       1.1 ms ±   0.1 ms    [User: 0.3 ms, System: 0.7 ms]
  Range (min … max):     0.9 ms …   1.6 ms    2489 runs
 
Benchmark 3: fish -c "echo \"Hello, world!\""
  Time (mean ± σ):      56.2 ms ±   0.8 ms    [User: 41.6 ms, System: 20.1 ms]
  Range (min … max):    54.4 ms …  58.2 ms    54 runs
 
Summary
  'bash -c "echo \"Hello, World!\""' ran
    3.72 ± 0.25 times slower than 'dash -c "echo \"Hello, world!\""'
   14.06 ± 0.42 times faster than 'fish -c "echo \"Hello, world!\""'

@Animeshz

This comment was marked as off-topic.

@sharkdp
Copy link
Owner

sharkdp commented Oct 29, 2022

Thank you very much for your contribution!

As title enlists.

What does "slowness checking" mean?

Comment on lines +11 to +12
pub is_reference: bool,
pub is_faster: bool,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe rename the latter to

Suggested change
pub is_reference: bool,
pub is_faster: bool,
pub is_reference: bool,
pub is_faster_than_reference: bool,

Thinking about this some more, it's actually not clear what the value of the latter bool should be for the case where is_reference is true. So maybe introduce an enum instead pub enum RelativeOrdering { Reference, FasterThanReference, SlowerThanReference } and use this instead of the two bool ("make illegal states unrepresentable"):

Suggested change
pub is_reference: bool,
pub is_faster: bool,
pub relative_ordering: RelativeOrdering,

What do you think?

.options
.reference_command
.as_ref()
.map(|cmd| Command::new(None, &cmd));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.map(|cmd| Command::new(None, &cmd));
.map(|cmd| Command::new(None, cmd));

.takes_value(true)
.number_of_values(1)
.value_name("CMD")
.help("The reference command to measure the results against."),
Copy link
Owner

@sharkdp sharkdp Oct 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.help("The reference command to measure the results against."),
.help("The reference command for the relative comparison of results. If this is unset, results are compared with the fastest command as reference."),

@@ -178,6 +178,9 @@ pub struct Options {
/// Whether or not to ignore non-zero exit codes
pub command_failure_action: CmdFailureAction,

/// Command to run before each *batch* of timing runs, i.e. before each individual benchmark
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy & paste error?

.arg(
Arg::new("reference")
.long("reference")
.short('R')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not introduce a short command line option for this right away. We can always add it later if this is really a commonly used option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow choice of reference benchmark
2 participants