]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
OPTIM: vars: use a cebtree instead of a list for variable names
authorWilly Tarreau <w@1wt.eu>
Sun, 15 Sep 2024 08:51:14 +0000 (10:51 +0200)
committerWilly Tarreau <w@1wt.eu>
Sun, 15 Sep 2024 21:49:01 +0000 (23:49 +0200)
Configs involving many variables can start to eat a lot of CPU in name
lookups. The reason is that the names themselves are dynamic in that
they are relative to dynamic objects (sessions, streams, etc), so
there's no fixed index for example. The current implementation relies
on a standard linked list, and in order to speed up lookups and avoid
comparing strings, only a 64-bit hash of the variable's name is stored
and compared everywhere.

But with just 100 variables and 1000 accesses in a config, it's clearly
visible that variable name lookup can reach 56% CPU with a config
generated this way:

  for i in {0..100}; do
    printf "\thttp-request set-var(txn.var%04d) int(%d)" $i $i;
    for j in {1..10}; do [ $i -lt $j ] || printf ",add(txn.var%04d)" $((i-j)); done;
    echo;
  done

The performance and a 4-core skylake 4.4 GHz reaches 85k RPS with a perf
profile showing:

  Samples: 170K of event 'cycles', Event count (approx.): 142378815419
  Overhead  Shared Object            Symbol
    56.39%  haproxy                  [.] var_to_smp
     6.65%  haproxy                  [.] var_set.part.0
     5.76%  haproxy                  [.] sample_process_cnv
     3.23%  haproxy                  [.] sample_conv_var2smp
     2.88%  haproxy                  [.] sample_conv_arith_add
     2.33%  haproxy                  [.] __pool_alloc
     2.19%  haproxy                  [.] action_store
     2.13%  haproxy                  [.] vars_get_by_desc
     1.87%  haproxy                  [.] smp_dup

[above, var_to_smp() calls var_get() under the read lock].

By switching to a binary tree, the cost is significantly lower, the
performance reaches 117k RPS (+37%) with this profile:

  Samples: 170K of event 'cycles', Event count (approx.): 142323631229
  Overhead  Shared Object            Symbol
    40.22%  haproxy                  [.] cebu64_lookup
     7.12%  haproxy                  [.] sample_process_cnv
     6.15%  haproxy                  [.] var_to_smp
     4.75%  haproxy                  [.] cebu64_insert
     3.79%  haproxy                  [.] sample_conv_var2smp
     3.40%  haproxy                  [.] cebu64_delete
     3.10%  haproxy                  [.] sample_conv_arith_add
     2.36%  haproxy                  [.] action_store
     2.32%  haproxy                  [.] __pool_alloc
     2.08%  haproxy                  [.] vars_get_by_desc
     1.96%  haproxy                  [.] smp_dup
     1.75%  haproxy                  [.] var_set.part.0
     1.74%  haproxy                  [.] cebu64_first
     1.07%  [kernel]                 [k] aq_hw_read_reg
     1.03%  haproxy                  [.] pool_put_to_cache
     1.00%  haproxy                  [.] sample_process

The performance lowers a bit earlier than with the list however. What
can be seen is that the performance maintains a plateau till 25 vars,
starts degrading a little bit for the tree while it remains stable till
28 vars for the list. Then both cross at 42 vars and the list continues
to degrade doing a hyperbole while the tree resists better. The biggest
loss is at around 32 variables where the list stays 10% higher.

Regardless, given the extremely narrow band where the list is better, it
looks relevant to switch to this in order to preserve the almost linear
performance of large setups. For example at 1000 variables and 10k
lookups, the tree is 18 times faster than the list.

In addition this reduces the size of the struct vars by 8 bytes since
there's a single pointer, though it could make sense to re-invest them
into a secondary head for example.

include/haproxy/vars-t.h
include/haproxy/vars.h
src/vars.c

index 320c7d6831ed33b6cc031b3f98dd4734be8bd477..3ad8c73ac6ae8ab1be4ac5ef0eb99c3213bfe508 100644 (file)
@@ -24,6 +24,7 @@
 
 #include <haproxy/sample_data-t.h>
 #include <haproxy/thread-t.h>
+#include <import/cebtree.h>
 
 /* flags used when setting/clearing variables */
 #define VF_CREATEONLY       0x00000001   // do nothing if the variable already exists
@@ -48,7 +49,7 @@ enum vars_scope {
 };
 
 struct vars {
-       struct list head;
+       struct ceb_node *name_root;
        enum vars_scope scope;
        unsigned int size;
        __decl_thread(HA_RWLOCK_T rwlock);
@@ -64,8 +65,8 @@ struct var_desc {
 };
 
 struct var {
-       struct list l; /* Used for chaining vars. */
-       uint64_t name_hash;      /* XXH3() of the variable's name */
+       struct ceb_node node; /* Used for chaining vars. */
+       uint64_t name_hash;      /* XXH3() of the variable's name, must be just after node */
        uint flags;       // VF_*
        /* 32-bit hole here */
        struct sample_data data; /* data storage. */
index 95bb4dcd80c1db7b837c9ca3537bca05d0658c33..86dcf2d46ef1375d4e9a9bd9d7243614b53a3e97 100644 (file)
@@ -22,6 +22,8 @@
 #ifndef _HAPROXY_VARS_H
 #define _HAPROXY_VARS_H
 
+#include <import/cebu64_tree.h>
+
 #include <haproxy/api-t.h>
 #include <haproxy/session-t.h>
 #include <haproxy/stream-t.h>
@@ -34,7 +36,7 @@ struct arg;
 
 void vars_init_head(struct vars *vars, enum vars_scope scope);
 void var_accounting_diff(struct vars *vars, struct session *sess, struct stream *strm, int size);
-unsigned int var_clear(struct var *var, int force);
+unsigned int var_clear(struct vars *vars, struct var *var, int force);
 void vars_prune_per_sess(struct vars *vars);
 int var_set(const struct var_desc *desc, struct sample *smp, uint flags);
 int var_unset(const struct var_desc *desc, struct sample *smp);
@@ -78,11 +80,13 @@ static inline void vars_rdunlock(struct vars *vars)
  */
 static inline void vars_prune(struct vars *vars, struct session *sess, struct stream *strm)
 {
-       struct var *var, *tmp;
+       struct ceb_node *node;
+       struct var *var;
        unsigned int size = 0;
 
-       list_for_each_entry_safe(var, tmp, &vars->head, l) {
-               size += var_clear(var, 1);
+       while ((node = cebu64_first(&vars->name_root))) {
+               var = container_of(node, struct var, node);
+               size += var_clear(vars, var, 1);
        }
 
        if (!size)
index 0c34202e6fc06d9b7d955c8cc6635d0a5d19dd08..6ba82b7e05c0deb55939d464376c203b37986cc0 100644 (file)
@@ -20,6 +20,8 @@
 #include <haproxy/vars.h>
 #include <haproxy/xxhash.h>
 
+#include <import/cebu64_tree.h>
+
 
 /* This contains a pool of struct vars */
 DECLARE_STATIC_POOL(var_pool, "vars", sizeof(struct var));
@@ -172,7 +174,7 @@ scope_sess:
  * using. If the variable is marked "VF_PERMANENT", the sample_data is only
  * reset to SMP_T_ANY unless <force> is non nul. Returns the freed size.
  */
-unsigned int var_clear(struct var *var, int force)
+unsigned int var_clear(struct vars *vars, struct var *var, int force)
 {
        unsigned int size = 0;
 
@@ -188,7 +190,7 @@ unsigned int var_clear(struct var *var, int force)
        var->data.type = SMP_T_ANY;
 
        if (!(var->flags & VF_PERMANENT) || force) {
-               LIST_DELETE(&var->l);
+               cebu64_delete(&vars->name_root, &var->node);
                pool_free(var_pool, var);
                size += sizeof(struct var);
        }
@@ -200,11 +202,13 @@ unsigned int var_clear(struct var *var, int force)
  */
 void vars_prune_per_sess(struct vars *vars)
 {
-       struct var *var, *tmp;
+       struct ceb_node *node;
+       struct var *var;
        unsigned int size = 0;
 
-       list_for_each_entry_safe(var, tmp, &vars->head, l) {
-               size += var_clear(var, 1);
+       while ((node = cebu64_first(&vars->name_root))) {
+               var = container_of(node, struct var, node);
+               size += var_clear(vars, var, 1);
        }
 
        if (!size)
@@ -219,7 +223,7 @@ void vars_prune_per_sess(struct vars *vars)
 /* This function initializes a variables list head */
 void vars_init_head(struct vars *vars, enum vars_scope scope)
 {
-       LIST_INIT(&vars->head);
+       vars->name_root = NULL;
        vars->scope = scope;
        vars->size = 0;
        HA_RWLOCK_INIT(&vars->rwlock);
@@ -327,11 +331,12 @@ static int vars_fill_desc(const char *name, int len, struct var_desc *desc, char
  */
 static struct var *var_get(struct vars *vars, uint64_t name_hash)
 {
-       struct var *var;
+       struct ceb_node *node;
+
+       node = cebu64_lookup(&vars->name_root, name_hash);
+       if (node)
+               return container_of(node, struct var, node);
 
-       list_for_each_entry(var, &vars->head, l)
-               if (var->name_hash == name_hash)
-                       return var;
        return NULL;
 }
 
@@ -428,10 +433,10 @@ int var_set(const struct var_desc *desc, struct sample *smp, uint flags)
                var = pool_alloc(var_pool);
                if (!var)
                        goto unlock;
-               LIST_APPEND(&vars->head, &var->l);
                var->name_hash = desc->name_hash;
                var->flags = flags & VF_PERMANENT;
                var->data.type = SMP_T_ANY;
+               cebu64_insert(&vars->name_root, &var->node);
        }
 
        /* A variable of type SMP_T_ANY is considered as unset (either created
@@ -561,7 +566,7 @@ int var_unset(const struct var_desc *desc, struct sample *smp)
        vars_wrlock(vars);
        var = var_get(vars, desc->name_hash);
        if (var) {
-               size = var_clear(var, 0);
+               size = var_clear(vars, var, 0);
                var_accounting_diff(vars, smp->sess, smp->strm, -size);
        }
        vars_wrunlock(vars);