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

Implement vectored I/O for TcpStream #1016

Merged
merged 5 commits into from
Jul 16, 2019

Conversation

Thomasdezeeuw
Copy link
Collaborator

@Thomasdezeeuw Thomasdezeeuw commented Jul 4, 2019

@carllerche I know you've said your not a fan of this approach but at least we can start using more nightly features on the crate without breaking the stable builds this way.

TODO:

  • Remove TcpStream::{read_bufs, write_bufs}.
  • Windows implementation, already uses IoVec so it shouldn't be too hard.
  • Remove iovec dependency.

Updates #957.

@Thomasdezeeuw Thomasdezeeuw requested a review from carllerche July 4, 2019 15:04
@Thomasdezeeuw
Copy link
Collaborator Author

I want to take the same approach for #977.

@Thomasdezeeuw
Copy link
Collaborator Author

@carllerche What do you think about it?

@Thomasdezeeuw
Copy link
Collaborator Author

Well 1.36 was just released, so this doesn't have to work with nightly features any more. But this can still apply to peer_addr.

@carllerche
Copy link
Member

@Thomasdezeeuw my take is, let’s target a future rust release and use nightly until then. I just prefer avoiding the feature flag. I expect we won’t cut 0.7 until at least async / await is on stable.

@carllerche
Copy link
Member

Basically I’m fine with this change, just without the feature flag :-)

@Thomasdezeeuw
Copy link
Collaborator Author

@carllerche Sounds good to me. Could you change the CI to use the nightly compiler only? I haven't figured out the Azure pipeline stuff.

@Thomasdezeeuw Thomasdezeeuw marked this pull request as ready for review July 5, 2019 08:33
@Thomasdezeeuw
Copy link
Collaborator Author

I've forced push changes to just implemented {read, write}_vectored. But this will fail the CI on old stable compilers.

@carllerche
Copy link
Member

We can probably just switch stable to nightly in Ci for now. That is what we do for Tokio.

@Thomasdezeeuw
Copy link
Collaborator Author

Looking at another build from earlier today (https://dev.azure.com/tokio-rs/Tokio/_build/results?buildId=1324&view=logs), it seems 1.36 is only used for macOS builds, other OSes are still on 1.35. I'll wait off on a rebase when the other OSes also use 1.36.

@carllerche
Copy link
Member

@Thomasdezeeuw this comments explains how to get CI on the latest Rust.

@Thomasdezeeuw
Copy link
Collaborator Author

Pr #1028 fixes the problems with the CI, so I'll wait for that to be merged before rebasing this.

@Thomasdezeeuw
Copy link
Collaborator Author

I've rebased on master, @carllerche can you give this a final look over to see if I didn't mess anything up while merging?

@Thomasdezeeuw
Copy link
Collaborator Author

Great, both min Rust version and Windows fails because they both use 1.35.

@carllerche carllerche merged commit 2a55db6 into tokio-rs:master Jul 16, 2019
@Thomasdezeeuw Thomasdezeeuw deleted the vectored-io branch July 17, 2019 09:43
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.

2 participants