Skip to content

Commit

Permalink
Simplify the logic for choosing EPOLL_CTL_ADD vs EPOLL_CTL_MOD
Browse files Browse the repository at this point in the history
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
  • Loading branch information
nmathewson committed Oct 24, 2010
1 parent c281aba commit 2c66983
Showing 1 changed file with 19 additions and 16 deletions.
35 changes: 19 additions & 16 deletions epoll.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) ||
(ch->write_change & EV_CHANGE_ADD)) {
/* If we are adding anything at all, we'll want to do
* either an ADD or a MOD. */
short new_events = ch->old_events;
events = 0;
op = EPOLL_CTL_ADD;
if (ch->read_change & EV_CHANGE_ADD) {
events |= EPOLLIN;
new_events |= EV_READ;
} else if (ch->read_change & EV_CHANGE_DEL) {
new_events &= ~EV_READ;
;
} else if (ch->old_events & EV_READ) {
events |= EPOLLIN;
}
if (ch->write_change & EV_CHANGE_ADD) {
events |= EPOLLOUT;
new_events |= EV_WRITE;
} else if (ch->write_change & EV_CHANGE_DEL) {
new_events &= ~EV_WRITE;
;
} else if (ch->old_events & EV_WRITE) {
events |= EPOLLOUT;
}
if ((ch->read_change|ch->write_change) & EV_ET)
events |= EPOLLET;

if (new_events == ch->old_events) {
/*
If the changelist has an "add" operation,
but no visible change to the events enabled
on the fd, we need to try the ADD anyway, in
case the fd was closed at some in the
middle. If it wasn't, the ADD operation
will fail with EEXIST; and we retry it as a
MOD. */
op = EPOLL_CTL_ADD;
} else if (ch->old_events) {
if (ch->old_events) {
/* If MOD fails, we retry as an ADD, and if
* ADD fails we will retry as a MOD. So the
* only hard part here is to guess which one
* will work. As a heuristic, we'll try
* MOD first if we think there were old
* events and ADD if we think there were none.
*
* We can be wrong about the MOD if the file
* has in fact been closed and re-opened.
*
* 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;
}
} else if ((ch->read_change & EV_CHANGE_DEL) ||
Expand Down

0 comments on commit 2c66983

Please sign in to comment.