MEDIUM: task: don't grab the WR lock just to check the WQ

When profiling locks, it appears that the WQ's lock has become the most
contended one, despite the WQ being split by thread. The reason is that
each thread takes the WQ lock before checking if it it does have something
to do. In practice the WQ almost only contains health checks and rare tasks
that can be scheduled anywhere, so this is a real waste of resources.

This patch proceeds differently. Now that the WQ's lock was turned to RW
lock, we proceed in 3 phases :
  1) locklessly check for the queue's emptiness

  2) take an R lock to retrieve the first element and check if it is
     expired. This way most visits are performed with an R lock to find
     and return the next expiration date.

  3) if one expiration is found, we perform the WR-locked lookup as
     usual.

As a result, on a one-minute test involving 8 threads and 64 streams at
1.3 million ctxsw/s, before this patch the lock profiler reported this :

    Stats about Lock TASK_WQ:
         # write lock  : 1125496
         # write unlock: 1125496 (0)
         # wait time for write     : 263.143 msec
         # wait time for write/lock: 233.802 nsec
         # read lock   : 0
         # read unlock : 0 (0)
         # wait time for read      : 0.000 msec
         # wait time for read/lock : 0.000 nsec

And after :

    Stats about Lock TASK_WQ:
         # write lock  : 173
         # write unlock: 173 (0)
         # wait time for write     : 0.018 msec
         # wait time for write/lock: 103.988 nsec
         # read lock   : 1072706
         # read unlock : 1072706 (0)
         # wait time for read      : 60.702 msec
         # wait time for read/lock : 56.588 nsec

Thus the contention was divided by 4.3.

diff --git a/src/task.c b/src/task.c
index 5845ffe044a0b4ef9f8ca2a487ec8970d4fbaf63..0799d0089fb0d3b5b6de2cc3a58f0006e7160275 100644 (file)
--- a/src/task.c
+++ b/src/task.c
@@ -163,6 +163,7 @@ int wake_expired_tasks()
 {
        struct task *task;
        struct eb32_node *eb;
+       __decl_hathreads(int key);
        int ret = TICK_ETERNITY;
 
        while (1) {
@@ -210,6 +211,29 @@ int wake_expired_tasks()
        }
 
 #ifdef USE_THREAD
+       if (eb_is_empty(&timers))
+               goto leave;
+
+       HA_RWLOCK_RDLOCK(TASK_WQ_LOCK, &wq_lock);
+       eb = eb32_lookup_ge(&timers, now_ms - TIMER_LOOK_BACK);
+       if (!eb) {
+               eb = eb32_first(&timers);
+               if (likely(!eb)) {
+                       HA_RWLOCK_RDUNLOCK(TASK_WQ_LOCK, &wq_lock);
+                       goto leave;
+               }
+       }
+       key = eb->key;
+       HA_RWLOCK_RDUNLOCK(TASK_WQ_LOCK, &wq_lock);
+
+       if (tick_is_lt(now_ms, key)) {
+               /* timer not expired yet, revisit it later */
+               ret = tick_first(ret, key);
+               goto leave;
+       }
+
+       /* There's really something of interest here, let's visit the queue */
+
        while (1) {
                HA_RWLOCK_WRLOCK(TASK_WQ_LOCK, &wq_lock);
   lookup_next:
@@ -258,6 +282,7 @@ int wake_expired_tasks()
 
        HA_RWLOCK_WRUNLOCK(TASK_WQ_LOCK, &wq_lock);
 #endif
+leave:
        return ret;
 }