Simplify the logic for choosing EPOLL_CTL_ADD vs EPOLL_CTL_MOD

Previously, we chose "ADD" whenever old_events==new_events, (since
we expected the add to fail with EEXIST), or whenever old_events
was==0, and MOD otherwise (i.e., when old_events was nonzero and not
equal to new_events).

But now that we retry failed MOD events as ADD *and* failed ADD
events as MOD, the important thing is now to try to guess right the
largest amount of the time, since guessing right means we do only
one syscall, but guessing wrong means we do two.

When old_events is 0, ADD is probably right (unless we're hitting
the dup bug, when we'll fall back).

And when old_events is set and != new_events, MOD is almost
certainly right for the same reasons as before.

But when old_events is equal to new events, then MOD will work fine
unless we closed and reopened the fd, in which case we'll have to
fall back to the ADD case.  (Redundant del/add pairs are more common
than closes for most use cases.)

This change lets us avoid calculating new_events, which ought to
save a little time in epoll.c
This commit is contained in:
Nick Mathewson 2010-10-24 11:51:14 -04:00
parent c281aba30e
commit 2c66983a6d

35
epoll.c
View File

@ -169,43 +169,46 @@ epoll_apply_changes(struct event_base *base)
*/ */
/* TODO: Turn this into a switch or a table lookup. */
if ((ch->read_change & EV_CHANGE_ADD) || if ((ch->read_change & EV_CHANGE_ADD) ||
(ch->write_change & EV_CHANGE_ADD)) { (ch->write_change & EV_CHANGE_ADD)) {
/* If we are adding anything at all, we'll want to do /* If we are adding anything at all, we'll want to do
* either an ADD or a MOD. */ * either an ADD or a MOD. */
short new_events = ch->old_events;
events = 0; events = 0;
op = EPOLL_CTL_ADD; op = EPOLL_CTL_ADD;
if (ch->read_change & EV_CHANGE_ADD) { if (ch->read_change & EV_CHANGE_ADD) {
events |= EPOLLIN; events |= EPOLLIN;
new_events |= EV_READ;
} else if (ch->read_change & EV_CHANGE_DEL) { } else if (ch->read_change & EV_CHANGE_DEL) {
new_events &= ~EV_READ; ;
} else if (ch->old_events & EV_READ) { } else if (ch->old_events & EV_READ) {
events |= EPOLLIN; events |= EPOLLIN;
} }
if (ch->write_change & EV_CHANGE_ADD) { if (ch->write_change & EV_CHANGE_ADD) {
events |= EPOLLOUT; events |= EPOLLOUT;
new_events |= EV_WRITE;
} else if (ch->write_change & EV_CHANGE_DEL) { } else if (ch->write_change & EV_CHANGE_DEL) {
new_events &= ~EV_WRITE; ;
} else if (ch->old_events & EV_WRITE) { } else if (ch->old_events & EV_WRITE) {
events |= EPOLLOUT; events |= EPOLLOUT;
} }
if ((ch->read_change|ch->write_change) & EV_ET) if ((ch->read_change|ch->write_change) & EV_ET)
events |= EPOLLET; events |= EPOLLET;
if (new_events == ch->old_events) { if (ch->old_events) {
/* /* If MOD fails, we retry as an ADD, and if
If the changelist has an "add" operation, * ADD fails we will retry as a MOD. So the
but no visible change to the events enabled * only hard part here is to guess which one
on the fd, we need to try the ADD anyway, in * will work. As a heuristic, we'll try
case the fd was closed at some in the * MOD first if we think there were old
middle. If it wasn't, the ADD operation * events and ADD if we think there were none.
will fail with EEXIST; and we retry it as a *
MOD. */ * We can be wrong about the MOD if the file
op = EPOLL_CTL_ADD; * has in fact been closed and re-opened.
} else if (ch->old_events) { *
* We can be wrong about the ADD if the
* the fd has been re-created with a dup()
* of the same file that it was before.
*/
op = EPOLL_CTL_MOD; op = EPOLL_CTL_MOD;
} }
} else if ((ch->read_change & EV_CHANGE_DEL) || } else if ((ch->read_change & EV_CHANGE_DEL) ||