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

Support output to files directly instead of just via output redirection #205

Closed
wants to merge 4 commits into from

Conversation

jasonrhansen
Copy link
Collaborator

Closes #193

Adds --outfile option to all of the CLI programs so you can output directly to a file instead of STDOUT.

@jonhoo
Copy link
Owner

jonhoo commented Jan 5, 2021

Hmm, what's the benefit of this over just having the caller do output redirection?

@jasonrhansen
Copy link
Collaborator Author

Based on what @saethlin reported in #193, it looks like it was requested for performance reasons because "the standard library's stdout wrapper is trying to detect newlines and flush on them".

@jonhoo
Copy link
Owner

jonhoo commented Jan 5, 2021

Ah! My proposal then is rather that we be smarter about how we access stdout. The standard trick here is to detect if STDOUT is a TTY, and if it is, then line-buffer, otherwise use a BufWriter. This PR is probably a good starting point for that, since we'll then have to write to a writer directly anyway. That means we can continue to use the primitives the OS provides us with, while still supporting both interactive and batch use-cases.

@jasonrhansen
Copy link
Collaborator Author

So do you mean something like this?

    if atty::is(atty::Stream::Stdout) {
        flamegraph::from_files(&mut options, &infiles, io::stdout().lock())?;
    } else {
        flamegraph::from_files(&mut options, &infiles, io::BufWriter::new(io::stdout().lock()))?;
    }

@jasonrhansen jasonrhansen deleted the outfile branch January 6, 2021 01:49
@jasonrhansen
Copy link
Collaborator Author

Opened #206 as a better solution.

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.

Support output to files directly instead of just via output redirection
2 participants