From: Willy Tarreau <w@1wt.eu>
Date: Fri, 18 Jun 2021 17:08:23 +0000 (+0200)
Subject: MEDIUM: queue: refine the locking in process_srv_queue()
X-Git-Tag: v2.5-dev1~60
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=1b648c857bb9e0fb857e86838bcca0c9ed01e2bd;p=thirdparty%2Fhaproxy.git

MEDIUM: queue: refine the locking in process_srv_queue()

The lock in process_srv_queue() was placed around the whole loop to
avoid the cost of taking/releasing it multiple times. But in practice
almost all calls to this function only dequeue a single connection, so
that argument doesn't really stand. However by placing the lock inside
the loop, we'd make it possible to release it before manipulating the
pendconn and waking the task up. That's what this patch does.

This increases the performance from 431k to 491k req/s on 16 threads
with 20 servers under leastconn.

The performance profile changes from this:
  14.09%  haproxy             [.] process_srv_queue
  10.22%  haproxy             [.] fwlc_srv_reposition
   6.39%  haproxy             [.] fwlc_get_next_server
   3.97%  haproxy             [.] pendconn_dequeue
   3.84%  haproxy             [.] pendconn_add

to this:
  13.03%  haproxy             [.] fwlc_srv_reposition
   8.08%  haproxy             [.] fwlc_get_next_server
   3.62%  haproxy             [.] process_srv_queue
   1.78%  haproxy             [.] pendconn_dequeue
   1.74%  haproxy             [.] pendconn_add

The difference is even slightly more visible in roundrobin which
does not have take_conn() call.
---

diff --git a/src/queue.c b/src/queue.c
index 87ffca58df..be11227fe2 100644
--- a/src/queue.c
+++ b/src/queue.c
@@ -340,27 +340,29 @@ void process_srv_queue(struct server *s, int server_locked)
 	int done = 0;
 	int maxconn;
 
-	if (!server_locked)
-		HA_SPIN_LOCK(SERVER_LOCK, &s->lock);
-	HA_RWLOCK_WRLOCK(PROXY_LOCK,  &p->lock);
 	maxconn = srv_dynamic_maxconn(s);
 	while (s->served < maxconn) {
 		struct pendconn *pc;
 
+		if (!server_locked)
+			HA_SPIN_LOCK(SERVER_LOCK, &s->lock);
+		HA_RWLOCK_WRLOCK(PROXY_LOCK,  &p->lock);
+
 		pc = pendconn_process_next_strm(s, p);
+
+		HA_RWLOCK_WRUNLOCK(PROXY_LOCK,  &p->lock);
+		if (!server_locked)
+			HA_SPIN_UNLOCK(SERVER_LOCK, &s->lock);
+
 		if (!pc)
 			break;
 
 		done++;
 
 		_HA_ATOMIC_INC(&s->served);
-
 		stream_add_srv_conn(pc->strm, s);
 		task_wakeup(pc->strm->task, TASK_WOKEN_RES);
 	}
-	HA_RWLOCK_WRUNLOCK(PROXY_LOCK,  &p->lock);
-	if (!server_locked)
-		HA_SPIN_UNLOCK(SERVER_LOCK, &s->lock);
 
 	_HA_ATOMIC_ADD(&p->served, done);