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

Mio-st changes #901

Closed
Thomasdezeeuw opened this issue Dec 27, 2018 · 5 comments
Closed

Mio-st changes #901

Thomasdezeeuw opened this issue Dec 27, 2018 · 5 comments
Labels
rfc Request for comments.

Comments

@Thomasdezeeuw
Copy link
Collaborator

Carl (@carllerche) asked me to help maintain mio after I forked mio to change it to be single threaded. In that fork I made some changes, of which the following (might) make sense for mio. Everything below is my opinion, feel free to disagree and discuss!

  • Renamed Token into EventedId, Token is too generic, EventedId is a bit
    more specific.
  • Split Ready into Ready and Interests. Interests is used in
    registering (Evented) and can only be readable and/or writable, but never be
    empty. Ready is only used as output in Event and can also be error or hup.
  • Moved Ready to event module, as now it's really only a part of Event.
  • Placed Interests in the poll module, as it's input to poll.(re)register.
  • Changed PollOpt into an enum, disallowing empty poll options, also moved
    it to the poll module.
  • Changed Events to use ArrayVec, dropping the need for an allocation and
    for the user to determine the capacity of events.
  • Merge Events and event::Iter, now &mut Events implements Iterator.
  • Document when Evented handles don't have to be deregistered, for example.
  • Made unix::EventedIo public.
  • Added ConnectedUdpSocket, same as UdpSocket but then with send instead
    of send_to methods. Also dropped send_to and friends from UdpSocket.
    Allows for less misuse of the API.
  • Added INTERESTS: Interests (Ready) constants to all types that implement
    Evented, allows people to easily use the interests that make the most sense,
    e.g. readable & writable for TcpStream.

Low priority

  • Renamed Poll to Poller, poller.poll looks slightly better to me.
  • Renamed PollOpt to PollOption, it's a long name already.
  • Created an abstraction around unix pipe.
  • In the net module types take SocketAddr, rather then &SocketAddr as
    SocketAddr is copy anyway.

Maybe out of scope

  • Added timers.
  • Added an option to only poll user space events.
  • Moved user space events directly to Poller (Poll), dropping Registration
    and SetReadiness.
  • Adding support for signals (work in progress).

Overview

The module layout in mio_st looks like this:

event::{Event, EventedId (now Token), Events, Ready, Evented};
net::{ConnectedUdpSocket, TcpListener, TcpStream, UdpSocket};
poll::{Interest (now Ready), Poller (now Poll), PollOption (now PollOpt)};
unix::{EventedFd, EventedIo, Receiver, Sender, pipe};

// Reexported in the root:
{EventedId, Events, Ready, Poller, PollOption};

P.S. if discussions around 1 change becomes to long it might be best to create a seperate issue and make this more of a meta issue.

@carllerche
Copy link
Member

Thanks for listing these out. It might be easiest to have a dedicated issue per proposal for discussion and tackle it a couple at a time.

Token -> EventId.

I'm OK renaming it, but generally idiomatic rust tries to avoid "stuttering". So, if EventId lives in an event module, it could just be named Id and referenced as event::Id.

Merge Events and event::Iter, now &mut Events implements Iterator.

That seems odd to me as it would require Events to have a built-in cursor.

I believe that &Events can implement IntoIterator, then you can still do the following:

for event in &events { }

@carllerche
Copy link
Member

Changed PollOpt into an enum.

This would create a forwards compat hazard. Adding variants is a breaking change. I think the desired goal of preventing empty PollOpt can be achieved by making PollOpt a struct that wraps an enum and removing the bit opts API.

@Thomasdezeeuw
Copy link
Collaborator Author

Thanks for listing these out. It might be easiest to have a dedicated issue per proposal for discussion and tackle it a couple at a time.

Token -> EventId.

I'm OK renaming it, but generally idiomatic rust tries to avoid "stuttering". So, if EventId lives in an event module, it could just be named Id and referenced as event::Id.

I've opened #905 to discuss the renaming of Token.

Merge Events and event::Iter, now &mut Events implements Iterator.

That seems odd to me as it would require Events to have a built-in cursor.

I believe that &Events can implement IntoIterator, then you can still do the following:

for event in &events { }

This is not a change I feel strongly about, but I did it to reduce the API surface. In mio-st Events has three methods: new, len and is_empty and then also implements Iterator. Really Events main purpose is to implement Iterator, so I though to merge the two types.

@Thomasdezeeuw
Copy link
Collaborator Author

Changed PollOpt into an enum.

This would create a forwards compat hazard. Adding variants is a breaking change. I think the desired goal of preventing empty PollOpt can be achieved by making PollOpt a struct that wraps an enum and removing the bit opts API.

There is the unstable #[non_exhaustive] attribute, rfc and tracking issues. It designed to address this issue, but I don't know about a timeline for stability. In the mean time your approach can be used.

@carllerche carllerche added the rfc Request for comments. label Jan 13, 2019
@Thomasdezeeuw
Copy link
Collaborator Author

With #986 most things are done, so closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rfc Request for comments.
Projects
None yet
Development

No branches or pull requests

2 participants