Skip to content

Conversation

@carllerche
Copy link
Member

The windows selector uses a linked list of currently active level-triggered events. When an event arrives and the socket is registered with level-triggered, the event is tracked in this linked list. Every call to select events will include all elements in this list. When an operation is performed on the socket such that the socket is no longer ready, the event is removed from the linked list.

Closes #241

Remaining

  • Move calls to unset_readiness into schedule_foo functions.
  • Possibly combine Registration::deregister & deregister2
  • Remove LinkedList
  • UDP support
  • Uncomment prior tests using level

Copy link
Contributor

Choose a reason for hiding this comment

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

How come this ended up being exported? Shouldn't this be an internal implementation detail?

Copy link
Member Author

Choose a reason for hiding this comment

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

Poll is a public API that provides polling on only sockets (no timer / notifications). It returns IoEvent. I'm confused how Mio compiled before without having this exported.

However, I will track this as part of the api discussion

Copy link
Contributor

Choose a reason for hiding this comment

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

rustc's privacy tracking is not so thorough - if something is declared with pub rustc considers it public and allows it in exported signatures, it need not be actually exported at the crate level

@carllerche carllerche force-pushed the win-level branch 3 times, most recently from 28ac228 to 5571548 Compare December 2, 2015 20:16
@carllerche
Copy link
Member Author

@alexcrichton I have made the changes you suggest. Look good? TCP support should be done, but I still have some more outstanding items (tracked in the PR description).

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this may want to be deferred until the actual mem::forget below. If the me.iocp.defer branch is taken the readiness should still be applicable.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh wait scratch that, it looks like defer will end up calling push_event which ends up doing the right thing for level triggering, so this should be fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@carllerche
Copy link
Member Author

Ok, this is not a candidate for merging (I will squash before merging)

cc/ @alexcrichton

Copy link
Contributor

Choose a reason for hiding this comment

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

You could also do this match via or_insert(event) and then or-in the kind, it at least avoids the Entry import (but works either way)

@alexcrichton
Copy link
Contributor

I think there may also want to be some writable level handling for UdpSocket, but otherwise lgtm r+

The windows selector uses a linked list of currently active level-triggered
events. When an event arrives and the socket is registered with
level-triggered, the event is tracked in this linked list. Every call to
select events will include all elements in this list. When an operation is
performed on the socket such that the socket is no longer ready, the event is
removed from the linked list.

Closes #241
@carllerche carllerche changed the title WIP - Implement level triggering for windows Implement level triggering for windows Dec 3, 2015
@carllerche carllerche merged commit 7c6b41a into master Dec 3, 2015
@carllerche carllerche deleted the win-level branch December 4, 2015 04:14
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.

4 participants