MEDIUM: connection: reintegrate conn_hash_node into connection

Previously the conn_hash_node was placed outside the connection due
to the big size of the eb64_node that could have negatively impacted
frontend connections. But having it outside also means that one
extra allocation is needed for each backend connection, and that one
memory indirection is needed for each lookup.

With the compact trees, the tree node is smaller (16 bytes vs 40) so
the overhead is much lower. By integrating it into the connection,
We're also eliminating one pointer from the connection to the hash
node and one pointer from the hash node to the connection (in addition
to the extra object bookkeeping). This results in saving at least 24
bytes per total backend connection, and only inflates connections by
16 bytes (from 240 to 256), which is a reasonable compromise.

Tests on a 64-core EPYC show a 2.4% increase in the request rate
(from 2.08 to 2.13 Mrps).

diff --git a/include/haproxy/connection-t.h b/include/haproxy/connection-t.h
index d02c777c0e635c34231dcf8aed822e1203feabfb..835c70b81b7ac9ad4c133a4dfaf2af4731da4298 100644 (file)
--- a/include/haproxy/connection-t.h
+++ b/include/haproxy/connection-t.h
@@ -598,6 +598,14 @@ struct conn_tlv_list {
 } __attribute__((packed));
 
 
+/* node for backend connection in the idle trees for http-reuse
+ * A connection is identified by a hash generated from its specific parameters
+ */
+struct conn_hash_node {
+       struct ceb_node node;    /* indexes the hashing key for safe/idle/avail */
+       uint64_t key;            /* the hashing key, also used by session-owned */
+};
+
 /* This structure describes a connection with its methods and data.
  * A connection may be performed to proxy or server via a local or remote
  * socket, and can also be made to an internal applet. It can support
@@ -643,7 +651,7 @@ struct connection {
        /* used to identify a backend connection for http-reuse,
         * thus only present if conn.target is of type OBJ_TYPE_SERVER
         */
-       struct conn_hash_node *hash_node;
+       struct conn_hash_node hash_node;
 
        /* Members used if connection must be reversed. */
        struct {
@@ -656,15 +664,6 @@ struct connection {
        uint8_t tos;                   /* set ip tos, if CO_FL_OPT_TOS is set */
 };
 
-/* node for backend connection in the idle trees for http-reuse
- * A connection is identified by a hash generated from its specific parameters
- */
-struct conn_hash_node {
-       struct ceb_node node;    /* indexes the hashing key for safe/idle/avail */
-       uint64_t key;            /* the hashing key, also used by session-owned */
-       struct connection *conn; /* connection owner of the node */
-};
-
 struct mux_proto_list {
        const struct ist token;    /* token name and length. Empty is catch-all */
        enum proto_proxy_mode mode;

diff --git a/include/haproxy/connection.h b/include/haproxy/connection.h
index d796991b449fde3431e8744402a4e45a7b9bd3ca..7943ee39f866c8352d9c30bbafa5f0c492adb643 100644 (file)
--- a/include/haproxy/connection.h
+++ b/include/haproxy/connection.h
@@ -39,7 +39,6 @@
 #include <haproxy/task-t.h>
 
 extern struct pool_head *pool_head_connection;
-extern struct pool_head *pool_head_conn_hash_node;
 extern struct pool_head *pool_head_sockaddr;
 extern struct pool_head *pool_head_pp_tlv_128;
 extern struct pool_head *pool_head_pp_tlv_256;
@@ -91,7 +90,6 @@ struct connection *conn_new(void *target);
 void conn_free(struct connection *conn);
 void conn_release(struct connection *conn);
 void conn_set_errno(struct connection *conn, int err);
-struct conn_hash_node *conn_alloc_hash_node(struct connection *conn);
 struct sockaddr_storage *sockaddr_alloc(struct sockaddr_storage **sap, const struct sockaddr_storage *orig, socklen_t len);
 void sockaddr_free(struct sockaddr_storage **sap);
 

diff --git a/src/backend.c b/src/backend.c
index 7b21c85b6645e1f39498d73f0b22aba8170e8422..0db098bbe9ecc6d93c670fffcc29e8efe072b719 100644 (file)
--- a/src/backend.c
+++ b/src/backend.c
@@ -1479,17 +1479,15 @@ static int do_connect_server(struct stream *s, struct connection *conn)
 static struct connection *
 takeover_random_idle_conn(struct ceb_root **root, int curtid)
 {
-       struct conn_hash_node *hash_node;
        struct connection *conn = NULL;
 
-       hash_node = ceb64_item_first(root, node, key, struct conn_hash_node);
-       while (hash_node) {
-               conn = hash_node->conn;
+       conn = ceb64_item_first(root, hash_node.node, hash_node.key, struct connection);
+       while (conn) {
                if (conn->mux->takeover && conn->mux->takeover(conn, curtid, 1) == 0) {
                        conn_delete_from_tree(conn, curtid);
                        return conn;
                }
-               hash_node = ceb64_item_next(root, node, key, hash_node);
+               conn = ceb64_item_next(root, hash_node.node, hash_node.key, conn);
        }
 
        return NULL;
@@ -1974,7 +1972,7 @@ int connect_server(struct stream *s)
                                srv_conn->flags |= CO_FL_OPT_TOS;
                        }
 
-                       srv_conn->hash_node->key = hash;
+                       srv_conn->hash_node.key = hash;
                } else if (srv && (srv->flags & SRV_F_STRICT_MAXCONN))
                        _HA_ATOMIC_DEC(&srv->curr_total_conns);
        }

diff --git a/src/connection.c b/src/connection.c
index 238a595e43d88c615f55560af04b66ba5d0325fe..74b2fa1f1d0837c186589d829cf5984cc6c9b543 100644 (file)
--- a/src/connection.c
+++ b/src/connection.c
@@ -39,7 +39,6 @@
 
 
 DECLARE_TYPED_POOL(pool_head_connection,     "connection",     struct connection, 0, 64);
-DECLARE_TYPED_POOL(pool_head_conn_hash_node, "conn_hash_node", struct conn_hash_node);
 DECLARE_TYPED_POOL(pool_head_sockaddr,       "sockaddr",       struct sockaddr_storage);
 DECLARE_TYPED_POOL(pool_head_pp_tlv_128,     "pp_tlv_128",     struct conn_tlv_list, HA_PP2_TLV_VALUE_128);
 DECLARE_TYPED_POOL(pool_head_pp_tlv_256,     "pp_tlv_256",     struct conn_tlv_list, HA_PP2_TLV_VALUE_256);
@@ -100,7 +99,7 @@ void conn_delete_from_tree(struct connection *conn, int thr)
                conn_tree = &srv->per_thr[thr].avail_conns;
        }
 
-       ceb64_item_delete(conn_tree, node, key, conn->hash_node);
+       ceb64_item_delete(conn_tree, hash_node.node, hash_node.key, conn);
 }
 
 int conn_create_mux(struct connection *conn, int *closed_connection)
@@ -518,7 +517,8 @@ void conn_init(struct connection *conn, void *target)
        conn->subs = NULL;
        conn->src = NULL;
        conn->dst = NULL;
-       conn->hash_node = NULL;
+       conn->hash_node.key = 0;
+       ceb_init_node(&conn->hash_node.node);
        conn->xprt = NULL;
        conn->reverse.target = NULL;
        conn->reverse.name = BUF_NULL;
@@ -537,10 +537,6 @@ static int conn_backend_init(struct connection *conn)
        if (!sockaddr_alloc(&conn->dst, 0, 0))
                return 1;
 
-       conn->hash_node = conn_alloc_hash_node(conn);
-       if (unlikely(!conn->hash_node))
-               return 1;
-
        return 0;
 }
 
@@ -573,11 +569,7 @@ static void conn_backend_deinit(struct connection *conn)
        }
 
        /* Make sure the connection is not left in the idle connection tree */
-       if (conn->hash_node != NULL)
-               BUG_ON(ceb_intree(&conn->hash_node->node));
-
-       pool_free(pool_head_conn_hash_node, conn->hash_node);
-       conn->hash_node = NULL;
+       BUG_ON(ceb_intree(&conn->hash_node.node));
 
        /* Remove from BE purge list. Necessary if conn already scheduled for
         * purge but finally freed before by another code path.
@@ -678,19 +670,6 @@ void conn_release(struct connection *conn)
        }
 }
 
-struct conn_hash_node *conn_alloc_hash_node(struct connection *conn)
-{
-       struct conn_hash_node *hash_node = NULL;
-
-       hash_node = pool_zalloc(pool_head_conn_hash_node);
-       if (unlikely(!hash_node))
-               return NULL;
-
-       hash_node->conn = conn;
-
-       return hash_node;
-}
-
 /* Allocates a struct sockaddr from the pool if needed, assigns it to *sap and
  * returns it. If <sap> is NULL, the address is always allocated and returned.
  * if <sap> is non-null, an address will only be allocated if it points to a
@@ -3010,7 +2989,7 @@ int conn_reverse(struct connection *conn)
                }
 
                hash = conn_calculate_hash(&hash_params);
-               conn->hash_node->key = hash;
+               conn->hash_node.key = hash;
 
                conn->target = &srv->obj_type;
                srv_use_conn(srv, conn);

diff --git a/src/mux_fcgi.c b/src/mux_fcgi.c
index 0dac901fe614656632f5ba4af08087496c6496e6..eadbdfc607dfb80ed499d5c1c1fc875eba1bec73 100644 (file)
--- a/src/mux_fcgi.c
+++ b/src/mux_fcgi.c
@@ -3812,7 +3812,7 @@ static void fcgi_detach(struct sedesc *sd)
                                TRACE_DEVEL("reusable idle connection", FCGI_EV_STRM_END, fconn->conn);
                                return;
                        }
-                       else if (!ceb_intree(&fconn->conn->hash_node->node) &&
+                       else if (!ceb_intree(&fconn->conn->hash_node.node) &&
                                 fcgi_avail_streams(fconn->conn) > 0 && objt_server(fconn->conn->target) &&
                                 !LIST_INLIST(&fconn->conn->sess_el)) {
                                srv_add_to_avail_list(__objt_server(fconn->conn->target), fconn->conn);

diff --git a/src/mux_h2.c b/src/mux_h2.c
index f126e6a23651510c6e7407e66c8effd6d74a01f2..2635056ebeb6d71a82b039f86883673b9690f582 100644 (file)
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -5630,7 +5630,7 @@ static void h2_detach(struct sedesc *sd)
                                        return;
 
                                }
-                               else if (!ceb_intree(&h2c->conn->hash_node->node) &&
+                               else if (!ceb_intree(&h2c->conn->hash_node.node) &&
                                         h2_avail_streams(h2c->conn) > 0 && objt_server(h2c->conn->target) &&
                                         !LIST_INLIST(&h2c->conn->sess_el)) {
                                        srv_add_to_avail_list(__objt_server(h2c->conn->target), h2c->conn);

diff --git a/src/mux_quic.c b/src/mux_quic.c
index 0e660c0afa628d677331a1385d6743b9ccffa3fc..6baa377a45e5a51b79782f8bf0894d505a1d4e15 100644 (file)
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -3936,7 +3936,7 @@ static void qmux_strm_detach(struct sedesc *sd)
                                conn = NULL;
                                goto end;
                        }
-                       else if (!ceb_intree(&conn->hash_node->node) &&
+                       else if (!ceb_intree(&conn->hash_node.node) &&
                                 qmux_avail_streams(conn) &&
                                 objt_server(conn->target)) {
                                TRACE_DEVEL("mark connection as available for reuse", QMUX_EV_STRM_END, conn);

diff --git a/src/mux_spop.c b/src/mux_spop.c
index b14dbc227bec4a20d7dce58bd1d4648821004b6b..5024078fe2c7480d57cbf90fd3c7d8f0ad567f8a 100644 (file)
--- a/src/mux_spop.c
+++ b/src/mux_spop.c
@@ -3069,7 +3069,7 @@ static void spop_detach(struct sedesc *sd)
                                TRACE_DEVEL("reusable idle connection", SPOP_EV_STRM_END);
                                return;
                        }
-                       else if (!ceb_intree(&spop_conn->conn->hash_node->node) &&
+                       else if (!ceb_intree(&spop_conn->conn->hash_node.node) &&
                                 spop_avail_streams(spop_conn->conn) > 0 && objt_server(spop_conn->conn->target) &&
                                 !LIST_INLIST(&spop_conn->conn->sess_el)) {
                                srv_add_to_avail_list(__objt_server(spop_conn->conn->target), spop_conn->conn);

diff --git a/src/server.c b/src/server.c
index be1ba5549af0138a42d974a10d56d618f9fe3cf1..6db223e9e0fd296857c4ad5edcdab363951a4cf7 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -7262,7 +7262,7 @@ void srv_release_conn(struct server *srv, struct connection *conn)
        }
 
        /* Remove the connection from any tree (safe, idle or available) */
-       if (conn->hash_node) {
+       if (ceb_intree(&conn->hash_node.node)) {
                HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
                conn_delete_from_tree(conn, tid);
                conn->flags &= ~CO_FL_LIST_MASK;
@@ -7275,14 +7275,7 @@ void srv_release_conn(struct server *srv, struct connection *conn)
  */
 struct connection *srv_lookup_conn(struct ceb_root **tree, uint64_t hash)
 {
-       struct conn_hash_node *hash_node;
-       struct connection *conn = NULL;
-
-       hash_node = ceb64_item_lookup(tree, node, key, hash, struct conn_hash_node);
-       if (hash_node)
-               conn = hash_node->conn;
-
-       return conn;
+       return ceb64_item_lookup(tree, hash_node.node, hash_node.key, hash, struct connection);
 }
 
 /* retrieve the next connection sharing the same hash as <conn>
@@ -7290,13 +7283,7 @@ struct connection *srv_lookup_conn(struct ceb_root **tree, uint64_t hash)
  */
 struct connection *srv_lookup_conn_next(struct ceb_root **tree, struct connection *conn)
 {
-       struct conn_hash_node *hash_node;
-
-       hash_node = ceb64_item_next_dup(tree, node, key, conn->hash_node);
-       if (hash_node)
-               conn = hash_node->conn;
-
-       return conn;
+       return ceb64_item_next_dup(tree, hash_node.node, hash_node.key, conn);
 }
 
 /* Add <conn> in <srv> idle trees. Set <is_safe> if connection is deemed safe
@@ -7315,7 +7302,7 @@ void _srv_add_idle(struct server *srv, struct connection *conn, int is_safe)
                                         &srv->per_thr[tid].idle_conns;
 
        /* first insert in idle or safe tree. */
-       ceb64_item_insert(tree, node, key, conn->hash_node);
+       ceb64_item_insert(tree, hash_node.node, hash_node.key, conn);
 
        /* insert in list sorted by connection usage. */
        LIST_APPEND(&srv->per_thr[tid].idle_conn_list, &conn->idle_list);
@@ -7397,7 +7384,7 @@ void srv_add_to_avail_list(struct server *srv, struct connection *conn)
 {
        /* connection cannot be in idle list if used as an avail idle conn. */
        BUG_ON(LIST_INLIST(&conn->idle_list));
-       ceb64_item_insert(&srv->per_thr[tid].avail_conns, node, key, conn->hash_node);
+       ceb64_item_insert(&srv->per_thr[tid].avail_conns, hash_node.node, hash_node.key, conn);
 }
 
 struct task *srv_cleanup_idle_conns(struct task *task, void *context, unsigned int state)
@@ -7499,6 +7486,7 @@ remove:
 static void srv_close_idle_conns(struct server *srv)
 {
        struct ceb_root ***cleaned_tree;
+       struct connection *conn;
        int i;
 
        for (i = 0; i < global.nbthread; ++i) {
@@ -7510,10 +7498,8 @@ static void srv_close_idle_conns(struct server *srv)
                };
 
                for (cleaned_tree = conn_trees; *cleaned_tree; ++cleaned_tree) {
-                       while (!ceb_isempty(*cleaned_tree)) {
-                               struct connection *conn;
-
-                               conn = ceb64_item_first(*cleaned_tree, node, key, struct conn_hash_node)->conn;
+                       while ((conn = ceb64_item_first(*cleaned_tree, hash_node.node,
+                                                       hash_node.key, struct connection))) {
                                if (conn->ctrl->ctrl_close)
                                        conn->ctrl->ctrl_close(conn);
                                conn_delete_from_tree(conn, i);

diff --git a/src/session.c b/src/session.c
index a08e08ac99764d91cf0593c81da844bded8cd542..bcc2afaa421f8d688637f440a3d7d4ca9154d582 100644 (file)
--- a/src/session.c
+++ b/src/session.c
@@ -785,7 +785,7 @@ struct connection *session_get_conn(struct session *sess, void *target, int64_t
 
        /* Search into pconns for a connection with matching params and available streams. */
        list_for_each_entry(srv_conn, &pconns->conn_list, sess_el) {
-               if ((srv_conn->hash_node && srv_conn->hash_node->key == hash) &&
+               if (srv_conn->hash_node.key == hash &&
                    srv_conn->mux &&
                    (srv_conn->mux->avail_streams(srv_conn) > 0) &&
                    !(srv_conn->flags & CO_FL_WAIT_XPRT)) {