EventLoop::new: Use level-triggered events for inotify fd.

[jimblandy]

The inotify crate's Inotify::read_events method does not read all available
events from inotify (see hannobraun/inotify-rs#156), only one buffer's worth. Using
level-triggered events tells Mio to report events on the inotify fd until all
events have been read.

Fixes #267.

[JohnTitor]

The solution looks reasonable to me. This may have a negative impact on performance but I think it's a trade-off. @notify-rs/watchmakers Any concerns?

@jimblandy And this bugfix (i.e. behavior change) would be worth mentioning on the changelog, could you add an entry?

[jimblandy]

Okay, great! Yes, I'll add a changelog entry.

When the number of events is small enough to be handled by a single call to read_events, this shouldn't affect performance at all, unless I'm missing something. When we have so many events that we need multiple read_events calls to handle them all, there will be additional trips through the Mio event loop. But I don't think that's especially expensive. We could amortize it further by using a larger buffer.

A more complex fix that minimizes Mio events would be to have handle_inotify call read_events repeatedly until it returns an iterator that produces no Events. Then we could keep the file descriptor edge-triggered. But I doubt the costs justify that, and I don't want to implement it myself.

[jimblandy]

@JohnTitor Would it be worth backporting this to the last version 4 release? That has the same bug.

(In fact, I've already backported it - the question is just whether the project wants it.)

[JohnTitor]

When the number of events is small enough to be handled by a single call to read_events, this shouldn't affect performance at all, unless I'm missing something. When we have so many events that we need multiple read_events calls to handle them all, there will be additional trips through the Mio event loop. But I don't think that's especially expensive. We could amortize it further by using a larger buffer.

Makes sense, then there's no roadblock to merge!

Would it be worth backporting this to the last version 4 release? That has the same bug.

I believe it's safe to backport, yes. If you want, I'm happy to accept it :)

[jimblandy]

I believe it's safe to backport, yes. If you want, I'm happy to accept it :)

Filed as #269.