MEDIUM: polling: start to move maxfd computation to the pollers

Since only select() and poll() still make use of maxfd, let's move
its computation right there in the pollers themselves, and only
during each fd update pass. The computation doesn't need a lock
anymore, only a few atomic ops. It will be accurate, be done much
less often and will not be required anymore in the FD's fast patch.

This provides a small performance increase of about 1% in connection
rate when using epoll since we get rid of this computation which was
performed under a lock.

diff --git a/include/proto/fd.h b/include/proto/fd.h
index cc559ac32f82f0f2a32c855b0430740fd83dc3c8..76d4c7e593aeecdc236dbc41f6d875b0f4dc2cc3 100644 (file)
--- a/include/proto/fd.h
+++ b/include/proto/fd.h
@@ -44,12 +44,12 @@ __decl_hathreads(extern HA_SPINLOCK_T __attribute__((aligned(64))) fdtab_lock);
 __decl_hathreads(extern HA_RWLOCK_T   __attribute__((aligned(64))) fdcache_lock);    /* global lock to protect fd_cache array */
 __decl_hathreads(extern HA_SPINLOCK_T __attribute__((aligned(64))) poll_lock);       /* global lock to protect poll info */
 
-/* Deletes an FD from the fdsets, and recomputes the maxfd limit.
+/* Deletes an FD from the fdsets.
  * The file descriptor is also closed.
  */
 void fd_delete(int fd);
 
-/* Deletes an FD from the fdsets, and recomputes the maxfd limit.
+/* Deletes an FD from the fdsets.
  * The file descriptor is kept open.
  */
 void fd_remove(int fd);
@@ -409,11 +409,6 @@ static inline void fd_insert(int fd, unsigned long thread_mask)
         * still knows this FD from a possible previous round.
         */
        HA_SPIN_UNLOCK(FD_LOCK, &fdtab[fd].lock);
-
-       HA_SPIN_LOCK(FDTAB_LOCK, &fdtab_lock);
-       if (fd + 1 > maxfd)
-               maxfd = fd + 1;
-       HA_SPIN_UNLOCK(FDTAB_LOCK, &fdtab_lock);
 }
 
 

diff --git a/include/types/fd.h b/include/types/fd.h
index 9f2c5feea73cd94b3cc7df79e107acfec19f70c1..da9f31eddb6aa7478f28b615e7c44ebcb63edaf7 100644 (file)
--- a/include/types/fd.h
+++ b/include/types/fd.h
@@ -149,7 +149,6 @@ extern struct poller pollers[MAX_POLLERS];   /* all registered pollers */
 
 extern struct fdtab *fdtab;             /* array of all the file descriptors */
 extern struct fdinfo *fdinfo;           /* less-often used infos for file descriptors */
-extern int maxfd;                       /* # of the highest fd + 1 */
 extern int totalconn;                   /* total # of terminated sessions */
 extern int actconn;                     /* # of active sessions */
 

diff --git a/src/ev_poll.c b/src/ev_poll.c
index f24bf69a9849383eb432e240fcacdb011e192a0c..2e67a9dc3fb6262596c55a0c2f6cb87467bcedfc 100644 (file)
--- a/src/ev_poll.c
+++ b/src/ev_poll.c
@@ -32,6 +32,7 @@
 #define POLLRDHUP 0
 #endif
 
+static int maxfd;   /* # of the highest fd + 1 */
 static unsigned int *fd_evts[2];
 
 /* private data */
@@ -67,8 +68,11 @@ REGPRM2 static void _do_poll(struct poller *p, int exp)
        int updt_idx, en, eo;
        int fds, count;
        int sr, sw;
+       int old_maxfd, new_maxfd, max_add_fd;
        unsigned rn, wn; /* read new, write new */
 
+       max_add_fd = -1;
+
        /* first, scan the update list to find changes */
        for (updt_idx = 0; updt_idx < fd_nbupdt; updt_idx++) {
                fd = fd_updt[updt_idx];
@@ -100,8 +104,32 @@ REGPRM2 static void _do_poll(struct poller *p, int exp)
                        else if ((en & ~eo) & FD_EV_POLLED_W)
                                hap_fd_set(fd, fd_evts[DIR_WR]);
                        HA_SPIN_UNLOCK(POLL_LOCK, &poll_lock);
+
+                       if (fd > max_add_fd)
+                               max_add_fd = fd;
                }
        }
+
+       /* maybe we added at least one fd larger than maxfd */
+       for (old_maxfd = maxfd; old_maxfd <= max_add_fd; ) {
+               if (HA_ATOMIC_CAS(&maxfd, &old_maxfd, max_add_fd + 1))
+                       break;
+       }
+
+       /* maxfd doesn't need to be precise but it needs to cover *all* active
+        * FDs. Thus we only shrink it if we have such an opportunity. The algo
+        * is simple : look for the previous used place, try to update maxfd to
+        * point to it, abort if maxfd changed in the mean time.
+        */
+       old_maxfd = maxfd;
+       do {
+               new_maxfd = old_maxfd;
+               while (new_maxfd - 1 >= 0 && !fdtab[new_maxfd - 1].owner)
+                       new_maxfd--;
+               if (new_maxfd >= old_maxfd)
+                       break;
+       } while (!HA_ATOMIC_CAS(&maxfd, &old_maxfd, new_maxfd));
+
        fd_nbupdt = 0;
 
        nbfd = 0;

diff --git a/src/ev_select.c b/src/ev_select.c
index 19b13805cf95688b23222218f78272fdf0d6fced..9d53a8502e470ef520f7a5b44c6fa98ed1da4df1 100644 (file)
--- a/src/ev_select.c
+++ b/src/ev_select.c
@@ -25,6 +25,7 @@
 
 
 /* private data */
+static int maxfd;   /* # of the highest fd + 1 */
 static fd_set *fd_evts[2];
 static THREAD_LOCAL fd_set *tmp_evts[2];
 
@@ -50,6 +51,9 @@ REGPRM2 static void _do_poll(struct poller *p, int exp)
        int updt_idx, en, eo;
        char count;
        int readnotnull, writenotnull;
+       int old_maxfd, new_maxfd, max_add_fd;
+
+       max_add_fd = -1;
 
        /* first, scan the update list to find changes */
        for (updt_idx = 0; updt_idx < fd_nbupdt; updt_idx++) {
@@ -74,16 +78,44 @@ REGPRM2 static void _do_poll(struct poller *p, int exp)
                        HA_SPIN_LOCK(POLL_LOCK, &poll_lock);
                        if ((eo & ~en) & FD_EV_POLLED_R)
                                FD_CLR(fd, fd_evts[DIR_RD]);
-                       else if ((en & ~eo) & FD_EV_POLLED_R)
+                       else if ((en & ~eo) & FD_EV_POLLED_R) {
                                FD_SET(fd, fd_evts[DIR_RD]);
+                               if (fd > max_add_fd)
+                                       max_add_fd = fd;
+                       }
 
                        if ((eo & ~en) & FD_EV_POLLED_W)
                                FD_CLR(fd, fd_evts[DIR_WR]);
-                       else if ((en & ~eo) & FD_EV_POLLED_W)
+                       else if ((en & ~eo) & FD_EV_POLLED_W) {
                                FD_SET(fd, fd_evts[DIR_WR]);
+                               if (fd > max_add_fd)
+                                       max_add_fd = fd;
+                       }
+
                        HA_SPIN_UNLOCK(POLL_LOCK, &poll_lock);
                }
        }
+
+       /* maybe we added at least one fd larger than maxfd */
+       for (old_maxfd = maxfd; old_maxfd <= max_add_fd; ) {
+               if (HA_ATOMIC_CAS(&maxfd, &old_maxfd, max_add_fd + 1))
+                       break;
+       }
+
+       /* maxfd doesn't need to be precise but it needs to cover *all* active
+        * FDs. Thus we only shrink it if we have such an opportunity. The algo
+        * is simple : look for the previous used place, try to update maxfd to
+        * point to it, abort if maxfd changed in the mean time.
+        */
+       old_maxfd = maxfd;
+       do {
+               new_maxfd = old_maxfd;
+               while (new_maxfd - 1 >= 0 && !fdtab[new_maxfd - 1].owner)
+                       new_maxfd--;
+               if (new_maxfd >= old_maxfd)
+                       break;
+       } while (!HA_ATOMIC_CAS(&maxfd, &old_maxfd, new_maxfd));
+
        fd_nbupdt = 0;
 
        /* let's restore fdset state */

diff --git a/src/fd.c b/src/fd.c
index 4397f6e31b1864abde5f8503c7479b282033ced4..1fa170b58e96235f65481309c9cda2496b0b2c53 100644 (file)
--- a/src/fd.c
+++ b/src/fd.c
@@ -160,7 +160,6 @@
 
 struct fdtab *fdtab = NULL;     /* array of all the file descriptors */
 struct fdinfo *fdinfo = NULL;   /* less-often used infos for file descriptors */
-int maxfd;                      /* # of the highest fd + 1 */
 int totalconn;                  /* total # of terminated sessions */
 int actconn;                    /* # of active sessions */
 
@@ -179,7 +178,7 @@ __decl_hathreads(HA_SPINLOCK_T fdtab_lock);       /* global lock to protect fdta
 __decl_hathreads(HA_RWLOCK_T   fdcache_lock);     /* global lock to protect fd_cache array */
 __decl_hathreads(HA_SPINLOCK_T poll_lock);        /* global lock to protect poll info */
 
-/* Deletes an FD from the fdsets, and recomputes the maxfd limit.
+/* Deletes an FD from the fdsets.
  * The file descriptor is also closed.
  */
 static void fd_dodelete(int fd, int do_close)
@@ -207,14 +206,9 @@ static void fd_dodelete(int fd, int do_close)
                close(fd);
        }
        HA_SPIN_UNLOCK(FD_LOCK, &fdtab[fd].lock);
-
-       HA_SPIN_LOCK(FDTAB_LOCK, &fdtab_lock);
-       while ((maxfd-1 >= 0) && !fdtab[maxfd-1].owner)
-               maxfd--;
-       HA_SPIN_UNLOCK(FDTAB_LOCK, &fdtab_lock);
 }
 
-/* Deletes an FD from the fdsets, and recomputes the maxfd limit.
+/* Deletes an FD from the fdsets.
  * The file descriptor is also closed.
  */
 void fd_delete(int fd)
@@ -222,7 +216,7 @@ void fd_delete(int fd)
        fd_dodelete(fd, 1);
 }
 
-/* Deletes an FD from the fdsets, and recomputes the maxfd limit.
+/* Deletes an FD from the fdsets.
  * The file descriptor is kept open.
  */
 void fd_remove(int fd)

diff --git a/src/haproxy.c b/src/haproxy.c
index 34e8f7549f5a072341af5855a1a26a163d1c6abf..11849fe22f96769e432f39441de86b9bace46437 100644 (file)
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -1298,7 +1298,7 @@ static void init(int argc, char **argv)
         * Initialize the previously static variables.
         */
 
-       totalconn = actconn = maxfd = listeners = stopping = 0;
+       totalconn = actconn = listeners = stopping = 0;
        killed = 0;