MEDIUM: connection: move idle connection trees to ceb64

Idle connection trees currently require a 56-byte conn_hash_node per
connection, which can be reduced to 32 bytes by moving to ceb64. While
ceb64 is theoretically slower, in practice here we're essentially
dealing with trees that almost always contain a single key and many
duplicates. In this case, ceb64 insert and lookup functions become
faster than eb64 ones because all duplicates are a list accessed in
O(1) while it's a subtree for eb64. In tests it is impossible to tell
the difference between the two, so it's worth reducing the memory
usage.

This commit brings the following memory savings to conn_hash_node
(one per backend connection), and to srv_per_thread (one per thread
and per server):

     struct       before  after  delta
  conn_hash_nodea   56     32     -24
  srv_per_thread    96     72     -24

The delicate part is conn_delete_from_tree(), because we need to
know the tree root the connection is attached to. But thanks to
recent cleanups, it's now clear enough (i.e. idle/safe/avail vs
session are easy to distinguish).

diff --git a/include/haproxy/connection-t.h b/include/haproxy/connection-t.h
index d216e49a846ed7104a2bd44ce87a5e22bf578c2a..d02c777c0e635c34231dcf8aed822e1203feabfb 100644 (file)
--- a/include/haproxy/connection-t.h
+++ b/include/haproxy/connection-t.h
@@ -28,7 +28,7 @@
 #include <netinet/ip.h>
 #include <netinet/ip6.h>
 
-#include <import/ebtree-t.h>
+#include <import/cebtree.h>
 #include <import/ist.h>
 
 #include <haproxy/api-t.h>
@@ -566,7 +566,7 @@ enum conn_hash_params_t {
 #define CONN_HASH_PARAMS_TYPE_COUNT 7
 
 #define CONN_HASH_PAYLOAD_LEN \
-       (((sizeof(((struct conn_hash_node *)0)->node.key)) * 8) - CONN_HASH_PARAMS_TYPE_COUNT)
+       (((sizeof(((struct conn_hash_node *)0)->key)) * 8) - CONN_HASH_PARAMS_TYPE_COUNT)
 
 #define CONN_HASH_GET_PAYLOAD(hash) \
        (((hash) << CONN_HASH_PARAMS_TYPE_COUNT) >> CONN_HASH_PARAMS_TYPE_COUNT)
@@ -660,7 +660,8 @@ struct connection {
  * A connection is identified by a hash generated from its specific parameters
  */
 struct conn_hash_node {
-       struct eb64_node node;   /* contains the hashing key */
+       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 */
 };
 

diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h
index a401bae0654a7716e730fa8f732bad68450aded6..3609d10aaec301f59e26e1e975578f2e94abdcab 100644 (file)
--- a/include/haproxy/server-t.h
+++ b/include/haproxy/server-t.h
@@ -265,15 +265,17 @@ struct tree_occ {
 /* Each server will have one occurrence of this structure per thread */
 struct srv_per_thread {
        struct mt_list streams;                 /* streams using this server (used by "shutdown server sessions") */
-       struct eb_root idle_conns;              /* Shareable idle connections */
-       struct eb_root safe_conns;              /* Safe idle connections */
-       struct eb_root avail_conns;             /* Connections in use, but with still new streams available */
        struct mt_list sess_conns;              /* Connections attached to a session which cannot be shared across clients */
 
        /* Secondary idle conn storage used in parallel to idle/safe trees.
         * Used to sort them by last usage and purge them in reverse order.
         */
        struct list idle_conn_list;
+
+       /* connection trees to look them up by name */
+       struct ceb_root *idle_conns;            /* Shareable idle connections */
+       struct ceb_root *safe_conns;            /* Safe idle connections */
+       struct ceb_root *avail_conns;           /* Connections in use, but with still new streams available */
 };
 
 /* Each server will have one occurrence of this structure per thread group */

diff --git a/include/haproxy/server.h b/include/haproxy/server.h
index 1b311e4a137c0fadd6d4973b00efc2e46a37f27a..968e28599bf8ad4e63bdf906f3b54f8702374abc 100644 (file)
--- a/include/haproxy/server.h
+++ b/include/haproxy/server.h
@@ -96,8 +96,8 @@ int snr_resolution_error_cb(struct resolv_requester *requester, int error_code);
 struct server *snr_check_ip_callback(struct server *srv, void *ip, unsigned char *ip_family);
 struct task *srv_cleanup_idle_conns(struct task *task, void *ctx, unsigned int state);
 void srv_release_conn(struct server *srv, struct connection *conn);
-struct connection *srv_lookup_conn(struct eb_root *tree, uint64_t hash);
-struct connection *srv_lookup_conn_next(struct connection *conn);
+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);
 
 void _srv_add_idle(struct server *srv, struct connection *conn, int is_safe);
 int srv_add_to_idle_list(struct server *srv, struct connection *conn, int is_safe);

diff --git a/src/backend.c b/src/backend.c
index ea3440c37db648461d5998c776cada93e3c86e5c..7b21c85b6645e1f39498d73f0b22aba8170e8422 100644 (file)
--- a/src/backend.c
+++ b/src/backend.c
@@ -18,7 +18,7 @@
 #include <ctype.h>
 #include <sys/types.h>
 
-#include <import/ebmbtree.h>
+#include <import/ceb64_tree.h>
 
 #include <haproxy/api.h>
 #include <haproxy/acl.h>
@@ -1365,7 +1365,7 @@ check_tgid:
                 * tree; unsafe requests are looked up in the safe conns tree.
                 */
                int search_tree = is_safe ? 1 : 0; // 0 = idle, 1 = safe
-               struct eb_root *tree;
+               struct ceb_root **tree;
 
                if (!srv->curr_idle_thr[i] || i == tid)
                        continue;
@@ -1387,7 +1387,7 @@ check_tgid:
                                        found = 1;
                                        break;
                                }
-                               conn = srv_lookup_conn_next(conn);
+                               conn = srv_lookup_conn_next(tree, conn);
                        }
                } while (!found && ++search_tree <= 1);
 
@@ -1477,20 +1477,19 @@ static int do_connect_server(struct stream *s, struct connection *conn)
  * if any.
  */
 static struct connection *
-takeover_random_idle_conn(struct eb_root *root, int curtid)
+takeover_random_idle_conn(struct ceb_root **root, int curtid)
 {
        struct conn_hash_node *hash_node;
        struct connection *conn = NULL;
-       struct eb64_node *node = eb64_first(root);
 
-       while (node) {
-               hash_node = eb64_entry(node, struct conn_hash_node, node);
+       hash_node = ceb64_item_first(root, node, key, struct conn_hash_node);
+       while (hash_node) {
                conn = hash_node->conn;
-               if (conn && conn->mux->takeover && conn->mux->takeover(conn, curtid, 1) == 0) {
+               if (conn->mux->takeover && conn->mux->takeover(conn, curtid, 1) == 0) {
                        conn_delete_from_tree(conn, curtid);
                        return conn;
                }
-               node = eb64_next(node);
+               hash_node = ceb64_item_next(root, node, key, hash_node);
        }
 
        return NULL;
@@ -1696,7 +1695,7 @@ int be_reuse_connection(int64_t hash, struct session *sess,
                 * Idle conns are necessarily looked up on the same thread so
                 * that there is no concurrency issues.
                 */
-               if (!eb_is_empty(&srv->per_thr[tid].avail_conns)) {
+               if (!ceb_isempty(&srv->per_thr[tid].avail_conns)) {
                        srv_conn = srv_lookup_conn(&srv->per_thr[tid].avail_conns, hash);
                        if (srv_conn) {
                                /* connection cannot be in idle list if used as an avail idle conn. */
@@ -1975,7 +1974,7 @@ int connect_server(struct stream *s)
                                srv_conn->flags |= CO_FL_OPT_TOS;
                        }
 
-                       srv_conn->hash_node->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 896dbc1a74f422d0ee47c1762f0ebef0d8bfd3ce..238a595e43d88c615f55560af04b66ba5d0325fe 100644 (file)
--- a/src/connection.c
+++ b/src/connection.c
@@ -12,7 +12,7 @@
 
 #include <errno.h>
 
-#include <import/ebmbtree.h>
+#include <import/ceb64_tree.h>
 
 #include <haproxy/api.h>
 #include <haproxy/arg.h>
@@ -81,8 +81,26 @@ struct conn_tlv_list *conn_get_tlv(struct connection *conn, int type)
  */
 void conn_delete_from_tree(struct connection *conn, int thr)
 {
-       LIST_DEL_INIT(&conn->idle_list);
-       eb64_delete(&conn->hash_node->node);
+       struct ceb_root **conn_tree;
+       struct server *srv = __objt_server(conn->target);
+
+       /* We need to determine where the connection is attached, if at all.
+        *  - if it's in a tree and not in the idle_list, it's the avail_tree
+        *  - if it's in a tree and has CO_FL_SAFE_LIST, it's the safe_tree
+        *  - if it's in a tree and has CO_FL_IDLE_LIST, it's the idle_tree
+        *  - if it's not in a tree and has CO_FL_SESS_IDLE, it's in the
+        *    session's list (but we don't care here).
+        */
+       if (LIST_INLIST(&conn->idle_list)) {
+               LIST_DEL_INIT(&conn->idle_list);
+               conn_tree = (conn->flags & CO_FL_SAFE_LIST) ?
+                       &srv->per_thr[thr].safe_conns :
+                       &srv->per_thr[thr].idle_conns;
+       } else {
+               conn_tree = &srv->per_thr[thr].avail_conns;
+       }
+
+       ceb64_item_delete(conn_tree, node, key, conn->hash_node);
 }
 
 int conn_create_mux(struct connection *conn, int *closed_connection)
@@ -556,7 +574,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(conn->hash_node->node.node.leaf_p != NULL);
+               BUG_ON(ceb_intree(&conn->hash_node->node));
 
        pool_free(pool_head_conn_hash_node, conn->hash_node);
        conn->hash_node = NULL;
@@ -2992,7 +3010,7 @@ int conn_reverse(struct connection *conn)
                }
 
                hash = conn_calculate_hash(&hash_params);
-               conn->hash_node->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 fc714d5a88d68accc3cf825a4e7a032a4fd2cdfd..0dac901fe614656632f5ba4af08087496c6496e6 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 (!fconn->conn->hash_node->node.node.leaf_p &&
+                       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 309e06e63ead7ee073d45df4cbf1f0a2a8f7386c..f126e6a23651510c6e7407e66c8effd6d74a01f2 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 (!h2c->conn->hash_node->node.node.leaf_p &&
+                               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 e51c99fb6bf4874d493beaaa1f0b5b6d33d14eb7..0e660c0afa628d677331a1385d6743b9ccffa3fc 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 (!conn->hash_node->node.node.leaf_p &&
+                       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 2583b3c9eb4d94d8c7f1d1748ff00bc15f517ad0..b14dbc227bec4a20d7dce58bd1d4648821004b6b 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 (!spop_conn->conn->hash_node->node.node.leaf_p &&
+                       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 2ce1151be162d6e076c8c00d7ddb4058077bd8c3..be1ba5549af0138a42d974a10d56d618f9fe3cf1 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -16,6 +16,7 @@
 #include <ctype.h>
 #include <errno.h>
 
+#include <import/ceb64_tree.h>
 #include <import/cebis_tree.h>
 #include <import/eb64tree.h>
 
@@ -5976,9 +5977,9 @@ static int srv_init_per_thr(struct server *srv)
                return -1;
 
        for (i = 0; i < global.nbthread; i++) {
-               srv->per_thr[i].idle_conns = EB_ROOT;
-               srv->per_thr[i].safe_conns = EB_ROOT;
-               srv->per_thr[i].avail_conns = EB_ROOT;
+               srv->per_thr[i].idle_conns = NULL;
+               srv->per_thr[i].safe_conns = NULL;
+               srv->per_thr[i].avail_conns = NULL;
                MT_LIST_INIT(&srv->per_thr[i].sess_conns);
                MT_LIST_INIT(&srv->per_thr[i].streams);
 
@@ -7272,17 +7273,14 @@ void srv_release_conn(struct server *srv, struct connection *conn)
 /* retrieve a connection from its <hash> in <tree>
  * returns NULL if no connection found
  */
-struct connection *srv_lookup_conn(struct eb_root *tree, uint64_t hash)
+struct connection *srv_lookup_conn(struct ceb_root **tree, uint64_t hash)
 {
-       struct eb64_node *node = NULL;
+       struct conn_hash_node *hash_node;
        struct connection *conn = NULL;
-       struct conn_hash_node *hash_node = NULL;
 
-       node = eb64_lookup(tree, hash);
-       if (node) {
-               hash_node = eb64_entry(node, struct conn_hash_node, node);
+       hash_node = ceb64_item_lookup(tree, node, key, hash, struct conn_hash_node);
+       if (hash_node)
                conn = hash_node->conn;
-       }
 
        return conn;
 }
@@ -7290,19 +7288,15 @@ struct connection *srv_lookup_conn(struct eb_root *tree, uint64_t hash)
 /* retrieve the next connection sharing the same hash as <conn>
  * returns NULL if no connection found
  */
-struct connection *srv_lookup_conn_next(struct connection *conn)
+struct connection *srv_lookup_conn_next(struct ceb_root **tree, struct connection *conn)
 {
-       struct eb64_node *node = NULL;
-       struct connection *next_conn = NULL;
-       struct conn_hash_node *hash_node = NULL;
+       struct conn_hash_node *hash_node;
 
-       node = eb64_next_dup(&conn->hash_node->node);
-       if (node) {
-               hash_node = eb64_entry(node, struct conn_hash_node, node);
-               next_conn = hash_node->conn;
-       }
+       hash_node = ceb64_item_next_dup(tree, node, key, conn->hash_node);
+       if (hash_node)
+               conn = hash_node->conn;
 
-       return next_conn;
+       return conn;
 }
 
 /* Add <conn> in <srv> idle trees. Set <is_safe> if connection is deemed safe
@@ -7317,11 +7311,11 @@ struct connection *srv_lookup_conn_next(struct connection *conn)
  */
 void _srv_add_idle(struct server *srv, struct connection *conn, int is_safe)
 {
-       struct eb_root *tree = is_safe ? &srv->per_thr[tid].safe_conns :
+       struct ceb_root **tree = is_safe ? &srv->per_thr[tid].safe_conns :
                                         &srv->per_thr[tid].idle_conns;
 
        /* first insert in idle or safe tree. */
-       eb64_insert(tree, &conn->hash_node->node);
+       ceb64_item_insert(tree, node, key, conn->hash_node);
 
        /* insert in list sorted by connection usage. */
        LIST_APPEND(&srv->per_thr[tid].idle_conn_list, &conn->idle_list);
@@ -7344,8 +7338,8 @@ int srv_add_to_idle_list(struct server *srv, struct connection *conn, int is_saf
            ((srv->proxy->options & PR_O_REUSE_MASK) != PR_O_REUSE_NEVR) &&
            ha_used_fds < global.tune.pool_high_count &&
            (srv->max_idle_conns == -1 || srv->max_idle_conns > srv->curr_idle_conns) &&
-           ((eb_is_empty(&srv->per_thr[tid].safe_conns) &&
-             (is_safe || eb_is_empty(&srv->per_thr[tid].idle_conns))) ||
+           ((ceb_isempty(&srv->per_thr[tid].safe_conns) &&
+             (is_safe || ceb_isempty(&srv->per_thr[tid].idle_conns))) ||
             (ha_used_fds < global.tune.pool_low_count &&
              (srv->curr_used_conns + srv->curr_idle_conns <=
               MAX(srv->curr_used_conns, srv->est_need_conns) + srv->low_idle_conns ||
@@ -7403,7 +7397,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));
-       eb64_insert(&srv->per_thr[tid].avail_conns, &conn->hash_node->node);
+       ceb64_item_insert(&srv->per_thr[tid].avail_conns, node, key, conn->hash_node);
 }
 
 struct task *srv_cleanup_idle_conns(struct task *task, void *context, unsigned int state)
@@ -7504,11 +7498,11 @@ remove:
  */
 static void srv_close_idle_conns(struct server *srv)
 {
-       struct eb_root **cleaned_tree;
+       struct ceb_root ***cleaned_tree;
        int i;
 
        for (i = 0; i < global.nbthread; ++i) {
-               struct eb_root *conn_trees[] = {
+               struct ceb_root **conn_trees[] = {
                        &srv->per_thr[i].idle_conns,
                        &srv->per_thr[i].safe_conns,
                        &srv->per_thr[i].avail_conns,
@@ -7516,11 +7510,10 @@ static void srv_close_idle_conns(struct server *srv)
                };
 
                for (cleaned_tree = conn_trees; *cleaned_tree; ++cleaned_tree) {
-                       while (!eb_is_empty(*cleaned_tree)) {
-                               struct eb64_node *node = eb64_first(*cleaned_tree);
-                               struct conn_hash_node *conn_hash_node = eb64_entry(node, struct conn_hash_node, node);
-                               struct connection *conn = conn_hash_node->conn;
+                       while (!ceb_isempty(*cleaned_tree)) {
+                               struct connection *conn;
 
+                               conn = ceb64_item_first(*cleaned_tree, node, key, struct conn_hash_node)->conn;
                                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 103fb159ae59420fb78e206a6b71de5068570c29..a08e08ac99764d91cf0593c81da844bded8cd542 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->node.key == hash) &&
+               if ((srv_conn->hash_node && srv_conn->hash_node->key == hash) &&
                    srv_conn->mux &&
                    (srv_conn->mux->avail_streams(srv_conn) > 0) &&
                    !(srv_conn->flags & CO_FL_WAIT_XPRT)) {