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

std: Implement LineWriter::write_vectored #67270

Merged
merged 1 commit into from
Dec 19, 2019

Conversation

alexcrichton
Copy link
Member

This commit implements the write_vectored method of the LineWriter
type. First discovered in bytecodealliance/wasmtime#629 the
write_vectored method of Stdout bottoms out here but only ends up
writing the first buffer due to the default implementation of
write_vectored.

Like BufWriter, however, LineWriter can have a non-default
implementation of write_vectored which tries to preserve the
vectored-ness as much as possible. Namely we can have a vectored write
for everything before the newline and everything after the newline if
all the stars align well.

Also like BufWriter, though, special care is taken to ensure that
whenever bytes are written we're sure to signal success since that
represents a "commit" of writing bytes.

This commit implements the `write_vectored` method of the `LineWriter`
type. First discovered in bytecodealliance/wasmtime#629 the
`write_vectored` method of `Stdout` bottoms out here but only ends up
writing the first buffer due to the default implementation of
`write_vectored`.

Like `BufWriter`, however, `LineWriter` can have a non-default
implementation of `write_vectored` which tries to preserve the
vectored-ness as much as possible. Namely we can have a vectored write
for everything before the newline and everything after the newline if
all the stars align well.

Also like `BufWriter`, though, special care is taken to ensure that
whenever bytes are written we're sure to signal success since that
represents a "commit" of writing bytes.
@rust-highfive
Copy link
Collaborator

r? @cramertj

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 13, 2019
@alexcrichton
Copy link
Member Author

r? @sfackler

@rust-highfive rust-highfive assigned sfackler and unassigned cramertj Dec 13, 2019
self.need_flush = true;
}
if n == prefix_amt {
match self.inner.write(&buf[..=j]) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure how I feel about one write_vectored call turning into up to write_vectored, write, flush, write, write_vectored. Maybe it makes sense for LineWriter, but IIRC in general we tend to want to keep write/write_vectored relative 1:1 with downstream calls.

Copy link
Member Author

Choose a reason for hiding this comment

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

FWIW LineWriter::write is already a call to write, flush, then write. We could skip a few calls here by creating a local Vec<IoSlice>, but I figured that since it's all buffered here anyway a few more calls wouldn't matter too much.

The main thing is that we want to slice a &[IoSlice] as if it were &[u8] but we can't do that since we can't mutate IoSlice in place.

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough.

@sfackler
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Dec 16, 2019

📌 Commit 2fee28e has been approved by sfackler

@bors
Copy link
Contributor

bors commented Dec 16, 2019

🌲 The tree is currently closed for pull requests below priority 100, this pull request will be tested once the tree is reopened

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 16, 2019
Centril added a commit to Centril/rust that referenced this pull request Dec 19, 2019
…, r=sfackler

std: Implement `LineWriter::write_vectored`

This commit implements the `write_vectored` method of the `LineWriter`
type. First discovered in bytecodealliance/wasmtime#629 the
`write_vectored` method of `Stdout` bottoms out here but only ends up
writing the first buffer due to the default implementation of
`write_vectored`.

Like `BufWriter`, however, `LineWriter` can have a non-default
implementation of `write_vectored` which tries to preserve the
vectored-ness as much as possible. Namely we can have a vectored write
for everything before the newline and everything after the newline if
all the stars align well.

Also like `BufWriter`, though, special care is taken to ensure that
whenever bytes are written we're sure to signal success since that
represents a "commit" of writing bytes.
bors added a commit that referenced this pull request Dec 19, 2019
Rollup of 8 pull requests

Successful merges:

 - #67189 (Unify binop wording)
 - #67270 (std: Implement `LineWriter::write_vectored`)
 - #67286 (Fix the configure.py TOML field for a couple LLVM options)
 - #67321 (make htons const fn)
 - #67382 (Remove some unnecessary `ATTR_*` constants.)
 - #67389 (Remove `SO_NOSIGPIPE` dummy variable on platforms that don't use it.)
 - #67394 (Remove outdated references to @t from comments)
 - #67406 (Suggest associated type when the specified one cannot be found)

Failed merges:

r? @ghost
@bors bors merged commit 2fee28e into rust-lang:master Dec 19, 2019
@alexcrichton alexcrichton deleted the write-more-line-writer branch January 7, 2020 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants