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

stdout buffering #18

Closed
sstadick opened this issue Sep 28, 2020 · 1 comment · Fixed by #27
Closed

stdout buffering #18

sstadick opened this issue Sep 28, 2020 · 1 comment · Fixed by #27

Comments

@sstadick
Copy link

Hi! Super cool project!

I was looking at your io related code an noticed you are using std::io::stdout, which has issues: rust-lang/rust#60673

I've worked around this in other libraries of my own by using burtsushi's https://docs.rs/grep-cli/0.1.5/grep_cli/, which provides a correctly buffering stdout method when piping to a file, and line buffering when connected to a TTY.

I've seen significant speedups in my own libraries switching to buffered stdout. This may help close the single threaded gap with tsv utils.

@ezrosent
Copy link
Owner

Hey, thanks for this pointer! It does indeed look like I should be using this. To set expectations on this improving benchmarks, though, all writes are batched in a local thread and then sent to a background thread to be issued. Beyond that, the background thread uses write_all_vectored, which lets you write a whole batch without necessarily flushing after each line if I'm reading the LineBuffered implementation correctly.

Still, like I said I still think frawk should be using grep_cli; so I'll keep this open until I migrate.

ezrosent added a commit that referenced this issue Oct 28, 2020
Based on the suggestion in #18, this change adopts grep_cli to detect
when to line-buffer output, and adds support for line-buffering to the
`writers` module.

This change accomplishes this by:

* Plumbing through "flush on newlines" information into FileHandles for
  stdout, when stdout is a tty.
* Allowing write requests to trigger a flush of the underlying writer.
@ezrosent ezrosent linked a pull request Oct 28, 2020 that will close this issue
ezrosent added a commit that referenced this issue Oct 28, 2020
Based on the suggestion in #18, this change adopts grep_cli to detect
when to line-buffer output, and adds support for line-buffering to the
`writers` module.

This change accomplishes this by:

* Plumbing through "flush on newlines" information into FileHandles for
  stdout, when stdout is a tty.
* Allowing write requests to trigger a flush of the underlying writer.
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 a pull request may close this issue.

2 participants