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

Non-persistent and edge-triggered modes for FileDescriptorEvent. #1596

Merged
merged 2 commits into from
Dec 5, 2016
Merged

Non-persistent and edge-triggered modes for FileDescriptorEvent. #1596

merged 2 commits into from
Dec 5, 2016

Conversation

japplegame
Copy link
Contributor

Both non-persistent and edge-triggered modes work perfectly for my libpq (PostgreSQL) wrapper.

@s-ludwig
Copy link
Member

If edge-triggers works, I'd prefer to remove the non-persistent option for now, to reduce the implementation burden for other drivers and the new vibe-core module. Otherwise, looks good, except that the libasync and win32 drivers also need to be adjusted to take the additional parameter (even though they only define a stub implementation).

@japplegame
Copy link
Contributor Author

I'm not sure that the edge-triggered events are supported by all backends. What about platforms without epoll?
In addition, I encountered problems with my libpq wrapper. In edge-triggered mode sometimes waiting for read event is endless. Because libpq reading function (PQconsumeInput) most likely reads not all available data from the socket. To avoid endless waiting you can use ioctl to check data availability or use non-persistent mode.

@s-ludwig
Copy link
Member

In the libpq wrapper, do you always try to read before waiting for an event? In that case it should work regardless of the read function reading everything available.

Edge-triggered can be simulated internally with the non-persistent approach without causing issues, but the reverse is not possible. For that reason, I'd like to go with the most restricted mode, rather than facilitating dependent code to rely on the more relaxed semantics of non-persistent.

@japplegame
Copy link
Contributor Author

In the libpq wrapper, do you always try to read before waiting for an event? In that case it should work regardless of the read function reading everything available.

Just trying to read isn't enough, because next edge-triggered event will be send only after reading function returns EAGAIN. But unfortunately I have no way to control the reading process as it is hidden inside libpq library. All I have is a function for reading portion of data from a socket. This function returns only information about errors, but nothing about data availability.

Edge-triggered can be simulated internally with the non-persistent approach without causing issues, but the reverse is not possible. For that reason, I'd like to go with the most restricted mode, rather than facilitating dependent code to rely on the more relaxed semantics of non-persistent.

Agree.

@japplegame
Copy link
Contributor Author

libasync and win32 drivers are adjusted.
Unfortunately I can't avoid nonPersistent mode in my libpg wrapper.
I suggest not to remove this mode.

@s-ludwig
Copy link
Member

Okay, let's keep it for now. I'm going to deal with it somehow for the new backend (which could mean . Would be interesting anyway to see if libpq can somehow be fixed/wrapped to avoid it.

Do you see any value in the persistent mode? AFAICS removing it wouldn't break existing use and it often tends to result in 100% CPU load anyway.

@japplegame
Copy link
Contributor Author

japplegame commented Nov 29, 2016

Use of edge-triggered mode instead of non-persistent sometimes leads to strange infinite waiting for read events. At now I can't understand the reasons, but will try.

@japplegame
Copy link
Contributor Author

Finally, I managed to get to work my libpq wrapper with edge-triggered events.

@s-ludwig
Copy link
Member

s-ludwig commented Dec 5, 2016

Cool! That means at least that there is less pressure on the vibe-core implementation in that regard.

I'll merge the PR like it is an will then gradually deprecate the level triggered mode. I'll also still see if I can make the one-shot mode work for vibe-core.

@s-ludwig s-ludwig merged commit 64551cf into vibe-d:master Dec 5, 2016
s-ludwig added a commit that referenced this pull request Dec 19, 2016
Non-persistent and edge-triggered modes for FileDescriptorEvent.
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