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

1 files changed, 19 insertions, 16 deletions

diff --git a/epoll.c b/epoll.c
index 9c8f0e99..672025f5 100644
--- a/epoll.c
+++ b/epoll.c
@@ -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) ||