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

[proposal] Ability to use time decided by the user #170

Closed
mrmlnc opened this issue May 30, 2019 · 3 comments
Closed

[proposal] Ability to use time decided by the user #170

mrmlnc opened this issue May 30, 2019 · 3 comments

Comments

@mrmlnc
Copy link

mrmlnc commented May 30, 2019

Hi, @sharkdp,

I know that such requests have already been initiated before and have been closed: #153, #135.

But I suggest not to provide a mechanism that will allow measurements to be made inside applications. I propose to take the values of the measurements from stdout.

We have a function execute_and_time where we measure the time:

#[cfg(windows)]
pub fn execute_and_time(
stdout: Stdio,
stderr: Stdio,
command: &str,
shell: &str,
) -> io::Result<ExecuteResult> {
let mut child = run_shell_command(stdout, stderr, command, shell)?;
let cpu_timer = get_cpu_timer(&child);
let status = child.wait()?;
let (user_time, system_time) = cpu_timer.stop();
Ok(ExecuteResult {
user_time,
system_time,
status,
})
}

Here we can get the time from stdout in a certain format, for example, USER_TIME, SYSTEM_TIME. If pass an argument at startup.

It turns out that we are still measuring the complete execution of the command, but where the command ends is decided by the user.

IMHO, this solution can be a compromise.

Example with Node.js

Basically, I'm interested in the question of measurements for Node.js, because the time to require dependencies is large and creates visual noise.

// useless time: ~200-500ms
const fg = require('fast-glob');

const before =performance.now();
fg.sync('**');
const after = performance.now();

// payload time: ~ 20-50ms
const time = after - before;

console.log(`${time}, ${time}`);
@sharkdp
Copy link
Owner

sharkdp commented May 31, 2019

Thank you for your request (and thank you for taking the time to search old issues!)

To be honest, I'm not convinced.

  • To me, that doesn't really look like a well-designed interface between two applications. hyperfine would need to read all of the output of the command and search for a particular pattern (which unit of time? what kind of number formats are allowed?). Searching for this pattern would probably need to be enabled by an additional command-line flag in hyperfine. What if no pattern is found? What if multiple patterns are found? Special cases like these would need to be handled in addition.
  • How would this feature work together with the shell-spawning-time correction? (it would probably need to be switched off?)

On the other hand, I don't want to deny that there seems to be some need for this, given that this came up a few times.

If we decide to implement this in one way or the other, I'd like to discuss alternatives first.

One thing that comes to my mind is something like a --subtract option. In your case, you could write a second script that only runs the first require line (or a command-line option in your original script that makes the script quit after the require section). We could then run something like:

hyperfine 'node script.js` --subtract 'node script.js --require-only'

(obviously, this is something you could already do now .. by running both benchmarks and subtracting the times)

The problem with this approach is that there could be negative timing results (if uncertainty is involved and if the actual benchmark time is very small)

@mrmlnc
Copy link
Author

mrmlnc commented Jun 5, 2019

… need to read all of the output of the command and search for a particular pattern…

Yeap. If a special flag is passed (probably, --custom-metrics), then read stdout (maybe stderr) and take the last fragment. Then we take only one record. The format is selected by hyperline.

If the user script generates too much information, then describe it in README, suggesting to suppress stdout. Or, if possible in Rust (I'm not familiar yet) expect a value in fd3. Yeah, it's a little awkward, but we'll get a lot less information.

What if no pattern is found?

Exception or write some message to stdout as before:

return Err(io::Error::new(
io::ErrorKind::Other,
"Command terminated with non-zero exit code. \
Use the '-i'/'--ignore-failure' option if you want to ignore this. \
Alternatively, use the '--show-output' option to debug what went wrong.",
));

What if multiple patterns are found?

Just take latest.

How would this feature work together with the shell-spawning-time correction?

I think we can just turn it off.

About --subtract option.

Unfortunately, it will not work properly. I tried to cheat: build Node.js application into executable file and run it as Shell.

hyperfine --warmup 3 '.\build\bench /C \"\"' '.\build\bench /C \"async node-glob *\"' '.\build\bench /C \"async tiny-glob *\"' '.\build\bench /C \"async fast-glob-current *\"'

(Here .\build\bench is an executable file (.exe) with the compiled application: webpack + pkg — one JS file without require).

The measurement results are very, very different, because the subtraction produces a lot of noise at large values. In addition, require works with a very large number of files, and synchronously. Unfortunately, I did not find the results, because I gave up this idea long ago.

IMHO, the manual mode is not so bad:

hyperfine --custom-metrics 'node ./suite.js --tiny-glob' 'node ./suite.js --fast-glob'

In fact, this will give access to more measurements on any languages.

@sharkdp
Copy link
Owner

sharkdp commented Sep 1, 2019

I think I've decided that I don't want to add this functionality to hyperfine. I'd like to keep the purpose of hyperfine focused on benchmarking external commands. If users want to perform micro benchmarks, they need to use what their programming language provides.

Thank you for taking the time to explain your view.

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

No branches or pull requests

2 participants