-
-
Notifications
You must be signed in to change notification settings - Fork 376
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
consider an option to capture stdout without printing it #377
Comments
Thank you for this request.
Interesting. I haven't really seen any optimizations of this kind (specifically checking whether the output is attached to
I agree. This would be useful for the Another question is whether we want to introduce a new option for this or if it could be enabled by default (which would maybe even enable us to do things like #375). I would actually be rather hesitant to enable it by default, as I could imagine that it could potentially slow down the benchmarked program (if the reader on the other side is not keeping up), as compared to the |
With respect to the tty issue, I do think that is a distinct issue. It's "the user sees no output, so we can do something different" vs "the user sees output, and it's on a terminal, so we can emit terminal escape sequences." Or maybe what you meant is that the issues are related, and that perhaps there might be a better UX that tries to unify them somehow. I agree that that might be a good idea, assuming you can reliably fake being a tty. But I'm not sure about that. As for making it the default... I'm not sure about that either. For me personally, I'd be happy with an opt-in flag. But that doesn't help the folks creating errant benchmarks. On the flip side, this is just one of many issues that can result in benchmarks being in error. Your point about capturing the output causing the programs to run more slowly seems like the crux of the issue to me. If that is indeed a problem, then making it the default probably seems unwise. I would be somewhat surprised if it couldn't keep up though. I think the optimal implementation of this flag would be to read the process's stdout and drop its contents immediately. So, basically, a |
@BurntSushi I tried to implement this today. Unfortunately, I did not find a fast solution so far. SettingLet's assume we have a command that writes a lot to stdout. Here, we use dd if=/dev/random of=large bs=1M count=100 We now want to write code to launch a process (here: AttemptMy naive attempt looks like this: use std::io::{self, Result, Write};
use std::process::{Command, ExitStatus, Stdio};
struct DevNull {}
impl Write for DevNull {
fn write(&mut self, buf: &[u8]) -> Result<usize> {
Ok(buf.len())
}
fn flush(&mut self) -> Result<()> {
Ok(())
}
}
trait CommandExt {
fn discard_output_and_get_status(&mut self) -> Result<ExitStatus>;
}
impl CommandExt for Command {
fn discard_output_and_get_status(&mut self) -> Result<ExitStatus> {
let mut child = self.spawn()?;
let mut reader = child.stdout.take().unwrap();
let mut dev_null = DevNull {};
io::copy(&mut reader, &mut dev_null)?;
child.wait()
}
}
fn main() -> Result<()> {
let result = Command::new("cat")
.arg("large")
.stdout(Stdio::piped())
.discard_output_and_get_status();
println!("Exited successfully: {}", result?.success());
Ok(())
} BenchmarksIf we compare the runtime of this program ( But notice that a similar "black hole" program (
If anyone has ideas on how to improve the situation, please let me know. I checked if If there is no way to make this perform any faster, the question is if we want to implement this at all. Especially given that there is an easy workaround by basically just piping into hyperfine "grep foo file | cat" … which is equivalent to running
|
I suspect you cannot get writes to a pipe to be anywhere close to as fast as writes to I think that means |
You could beat |
So using |
One more thing: GNU # mknod -m a+rw ./null c 1 3
|
I'm not quite sure I follow. Your table of benchmark results for Using |
AFAIK, GNU |
@tavianator Aye, interesting. Good to know! I think |
It's clear now, but back then I didn't realize that there is probably no way to make it faster. I just didn't expect there to be such a huge difference. To summarize (mostly for me):
Given these results, I'm completely in favor of implementing this in #509 |
We can now repeat the benchmark from the reddit thread: The wrong way, triggering
And now with the new
|
w00t! Thank you @sharkdp and @tavianator!!! |
By default, Hyperfine will attach
Stdio::null()
to the stdout of the command it runs. Some programs, like greps, will detect this, alter how they execute and run more quickly as a result. For example, in the case of GNU grep, if it detects that stdout is/dev/null
, then it will behave as if the-q/--quiet
flag were present. This means GNU grep can exit as soon as a match is found. This is a perfectly legitimate optimization since its only observable behavior is its exit code, and its exit code is only influenced by whether a match exists or not (assuming no errors or interrupts occur).What this means is that, e.g.,
time grep foo file | wc -l
andhyperfine 'grep foo file'
aren't quite doing the same thing. That's no fault of Hyperfine, however, it would be nice to have a way to easily work around it. As of today, here are some work-arounds:--show-output
flag, which captures stdout. But this also prints stdout, which can be undesirable if there is a lot of output.--ignore-failure
flag.A better work-around might be to have some other flag, maybe
--capture-output
, that causes Hyperfine to capture stdout but then do nothing with it.I was motivated to write this feature request based on an errant benchmark found in the wild: https://old.reddit.com/r/commandline/comments/mibsw8/alternative_to_grep_less/gt90lmm/
The text was updated successfully, but these errors were encountered: