MEDIUM: migrate the patterns reference to cebs_tree

cebs_tree are 24 bytes smaller than ebst_tree (16B vs 40B), and pattern
references are only used during map/acl updates, so their storage is
pure loss between updates (which most of the time never happen). By
switching their indexing to compact trees, we can save 16 to 24 bytes
per entry depending on alightment (here it's 24 per struct but 16
practical as malloc's alignment keeps 8 unused).

Tested on core i7-8650U running at 3.0 GHz, with a file containing
17.7M IP addresses (16.7M different):

   $ time  ./haproxy -c -f acl-ip.cfg

Save 280 MB RAM for 17.7M IP addresses, and slightly speeds up the
startup (5.8%, from 19.2s to 18.2s), a part of which possible being
attributed to having to write less memory. Note that this is on small
strings. On larger ones such as user-agents, ebtree doesn't reread
the whole key and might be more efficient.

Before:
  RAM (VSZ/RSS): 4443912 3912444

  real    0m19.211s
  user    0m18.138s
  sys     0m1.068s

  Overhead  Command         Shared Object      Symbol
    44.79%  haproxy  haproxy            [.] ebst_insert
    25.07%  haproxy  haproxy            [.] ebmb_insert_prefix
     3.44%  haproxy  libc-2.33.so       [.] __libc_calloc
     2.71%  haproxy  libc-2.33.so       [.] _int_malloc
     2.33%  haproxy  haproxy            [.] free_pattern_tree
     1.78%  haproxy  libc-2.33.so       [.] inet_pton4
     1.62%  haproxy  libc-2.33.so       [.] _IO_fgets
     1.58%  haproxy  libc-2.33.so       [.] _int_free
     1.56%  haproxy  haproxy            [.] pat_ref_push
     1.35%  haproxy  libc-2.33.so       [.] malloc_consolidate
     1.16%  haproxy  libc-2.33.so       [.] __strlen_avx2
     0.79%  haproxy  haproxy            [.] pat_idx_tree_ip
     0.76%  haproxy  haproxy            [.] pat_ref_read_from_file
     0.60%  haproxy  libc-2.33.so       [.] __strrchr_avx2
     0.55%  haproxy  libc-2.33.so       [.] unlink_chunk.constprop.0
     0.54%  haproxy  libc-2.33.so       [.] __memchr_avx2
     0.46%  haproxy  haproxy            [.] pat_ref_append

After:
  RAM (VSZ/RSS): 4166108 3634768

  real    0m18.114s
  user    0m17.113s
  sys     0m0.996s

  Overhead  Command  Shared Object       Symbol
    38.99%  haproxy  haproxy             [.] cebs_insert
    27.09%  haproxy  haproxy             [.] ebmb_insert_prefix
     3.63%  haproxy  libc-2.33.so        [.] __libc_calloc
     3.18%  haproxy  libc-2.33.so        [.] _int_malloc
     2.69%  haproxy  haproxy             [.] free_pattern_tree
     1.99%  haproxy  libc-2.33.so        [.] inet_pton4
     1.74%  haproxy  libc-2.33.so        [.] _IO_fgets
     1.73%  haproxy  libc-2.33.so        [.] _int_free
     1.57%  haproxy  haproxy             [.] pat_ref_push
     1.48%  haproxy  libc-2.33.so        [.] malloc_consolidate
     1.22%  haproxy  libc-2.33.so        [.] __strlen_avx2
     1.05%  haproxy  libc-2.33.so        [.] __strcmp_avx2
     0.80%  haproxy  haproxy             [.] pat_idx_tree_ip
     0.74%  haproxy  libc-2.33.so        [.] __memchr_avx2
     0.69%  haproxy  libc-2.33.so        [.] __strrchr_avx2
     0.69%  haproxy  libc-2.33.so        [.] _IO_getline_info
     0.62%  haproxy  haproxy             [.] pat_ref_read_from_file
     0.56%  haproxy  libc-2.33.so        [.] unlink_chunk.constprop.0
     0.56%  haproxy  libc-2.33.so        [.] cfree@GLIBC_2.2.5
     0.46%  haproxy  haproxy             [.] pat_ref_append

If the addresses are totally disordered (via "shuf" on the input file),
we see both implementations reach exactly 68.0s (slower due to much
higher cache miss ratio).

On large strings such as user agents (1 million here), it's now slightly
slower (+9%):

Before:
  real    0m2.475s
  user    0m2.316s
  sys     0m0.155s

After:
  real    0m2.696s
  user    0m2.544s
  sys     0m0.147s

But such patterns are much less common than short ones, and the memory
savings do still count.

Note that while it could be tempting to get rid of the list that chains
all these pat_ref_elt together and only enumerate them by walking along
the tree to save 16 extra bytes per entry, that's not possible due to
the problem that insertion ordering is critical (think overlapping regex
such as /index.* and /index.html). Currently it's not possible to proceed
differently because patterns are first pre-loaded into the pat_ref via
pat_ref_read_from_file_smp() and later indexed by pattern_read_from_file(),
which has to only redo the second part anyway for maps/acls declared
multiple times.

diff --git a/include/haproxy/pattern-t.h b/include/haproxy/pattern-t.h
index 35c4fd0956de95976eac6cde80f98968d76479f0..de1a308e1847727b36edc996ffc772fbbfca7aa0 100644 (file)
--- a/include/haproxy/pattern-t.h
+++ b/include/haproxy/pattern-t.h
@@ -22,6 +22,7 @@
 #ifndef _HAPROXY_PATTERN_T_H
 #define _HAPROXY_PATTERN_T_H
 
+#include <import/cebtree.h>
 #include <import/ebtree-t.h>
 
 #include <haproxy/api-t.h>
@@ -107,7 +108,7 @@ struct pat_ref {
        char *reference; /* The reference name. */
        char *display; /* String displayed to identify the pattern origin. */
        struct list head; /* The head of the list of struct pat_ref_elt. */
-       struct eb_root ebmb_root; /* The tree where pattern reference elements are attached. */
+       struct ceb_root *ceb_root; /* The tree where pattern reference elements are attached. */
        struct list pat; /* The head of the list of struct pattern_expr. */
        unsigned int flags; /* flags PAT_REF_*. */
        unsigned int curr_gen; /* current generation number (anything below can be removed) */
@@ -132,7 +133,7 @@ struct pat_ref_elt {
        char *sample;
        unsigned int gen_id; /* generation of pat_ref this was made for */
        int line;
-       struct ebmb_node node; /* Node to attach this element to its <pat_ref> ebtree. */
+       struct ceb_node node; /* Node to attach this element to its <pat_ref> ebtree. */
        const char pattern[0]; // const only to make sure nobody tries to free it.
 };
 

diff --git a/src/pattern.c b/src/pattern.c
index 3e25b88e79e96a5b610d4aa26ea129f46555d69f..9971841394c46e843aacdf2a67aec86713af94a2 100644 (file)
--- a/src/pattern.c
+++ b/src/pattern.c
@@ -14,6 +14,7 @@
 #include <stdio.h>
 #include <errno.h>
 
+#include <import/cebs_tree.h>
 #include <import/ebistree.h>
 #include <import/ebpttree.h>
 #include <import/ebsttree.h>
@@ -1590,7 +1591,7 @@ void pat_ref_delete_by_ptr(struct pat_ref *ref, struct pat_ref_elt *elt)
                HA_RWLOCK_WRUNLOCK(PATEXP_LOCK, &expr->lock);
 
        LIST_DELETE(&elt->list);
-       ebmb_delete(&elt->node);
+       cebs_item_delete(&ref->ceb_root, node, pattern, elt);
        free(elt->sample);
        free(elt);
        HA_ATOMIC_INC(&patterns_freed);
@@ -1625,20 +1626,18 @@ int pat_ref_delete_by_id(struct pat_ref *ref, struct pat_ref_elt *refelt)
  */
 int pat_ref_gen_delete(struct pat_ref *ref, unsigned int gen_id, const char *key)
 {
-       struct ebmb_node *node;
+       struct pat_ref_elt *elt, *next;
        int found = 0;
 
        /* delete pattern from reference */
-       node = ebst_lookup(&ref->ebmb_root, key);
-       while (node) {
-               struct pat_ref_elt *elt;
-
-               elt = ebmb_entry(node, struct pat_ref_elt, node);
-               node = ebmb_next_dup(node);
+       elt = cebs_item_lookup(&ref->ceb_root, node, pattern, key, struct pat_ref_elt);
+       while (elt) {
                if (elt->gen_id != gen_id)
                        continue;
+               next = cebs_item_next_dup(&ref->ceb_root, node, pattern, elt);
                pat_ref_delete_by_ptr(ref, elt);
                found = 1;
+               elt = next;
        }
 
        if (found)
@@ -1662,20 +1661,15 @@ int pat_ref_delete(struct pat_ref *ref, const char *key)
  */
 struct pat_ref_elt *pat_ref_gen_find_elt(struct pat_ref *ref, unsigned int gen_id, const char *key)
 {
-       struct ebmb_node *node;
        struct pat_ref_elt *elt;
 
-       node = ebst_lookup(&ref->ebmb_root, key);
-       while (node) {
-               elt = ebmb_entry(node, struct pat_ref_elt, node);
+       elt = cebs_item_lookup(&ref->ceb_root, node, pattern, key, struct pat_ref_elt);
+       while (elt) {
                if (elt->gen_id == gen_id)
                        break;
-               node = ebmb_next_dup(node);
+               elt = cebs_item_next_dup(&ref->ceb_root, node, pattern, elt);
        }
-       if (node)
-               return ebmb_entry(node, struct pat_ref_elt, node);
-
-       return NULL;
+       return elt;
 }
 
 /*
@@ -1790,24 +1784,22 @@ int pat_ref_set_by_id(struct pat_ref *ref, struct pat_ref_elt *refelt, const cha
        return 0;
 }
 
-static int pat_ref_set_from_node(struct pat_ref *ref, struct ebmb_node *node, const char *value, char **err)
+static int pat_ref_set_from_elt(struct pat_ref *ref, struct pat_ref_elt *elt, const char *value, char **err)
 {
-       struct pat_ref_elt *elt;
        unsigned int gen;
        int first = 1;
        int found = 0;
 
-       while (node) {
+       while (elt) {
                char *tmp_err = NULL;
 
-               elt = ebmb_entry(node, struct pat_ref_elt, node);
                if (first)
                        gen = elt->gen_id;
                else if (elt->gen_id != gen) {
                        /* only consider duplicate elements from the same gen! */
                        continue;
                }
-               node = ebmb_next_dup(node);
+
                if (!pat_ref_set_elt(ref, elt, value, &tmp_err)) {
                        if (err)
                                *err = tmp_err;
@@ -1817,6 +1809,7 @@ static int pat_ref_set_from_node(struct pat_ref *ref, struct ebmb_node *node, co
                }
                found = 1;
                first = 0;
+               elt = cebs_item_next_dup(&ref->ceb_root, node, pattern, elt);
        }
 
        if (!found) {
@@ -1834,7 +1827,7 @@ static int pat_ref_set_from_node(struct pat_ref *ref, struct ebmb_node *node, co
 int pat_ref_set_elt_duplicate(struct pat_ref *ref, struct pat_ref_elt *elt, const char *value,
                               char **err)
 {
-       return pat_ref_set_from_node(ref, &elt->node, value, err);
+       return pat_ref_set_from_elt(ref, elt, value, err);
 }
 
 /* This function modifies to <value> the sample of all patterns matching <key>
@@ -1843,18 +1836,16 @@ int pat_ref_set_elt_duplicate(struct pat_ref *ref, struct pat_ref_elt *elt, cons
 int pat_ref_gen_set(struct pat_ref *ref, unsigned int gen_id,
                     const char *key, const char *value, char **err)
 {
-       struct ebmb_node *node;
        struct pat_ref_elt *elt;
 
        /* Look for pattern in the reference. */
-       node = ebst_lookup(&ref->ebmb_root, key);
-       while (node) {
-               elt = ebmb_entry(node, struct pat_ref_elt, node);
+       elt = cebs_item_lookup(&ref->ceb_root, node, pattern, key, struct pat_ref_elt);
+       while (elt) {
                if (elt->gen_id == gen_id)
                        break;
-               node = ebmb_next_dup(node);
+               elt = cebs_item_next_dup(&ref->ceb_root, node, pattern, elt);
        }
-       return pat_ref_set_from_node(ref, node, value, err);
+       return pat_ref_set_from_elt(ref, elt, value, err);
 }
 
 /* This function modifies to <value> the sample of all patterns matching <key>
@@ -1896,7 +1887,7 @@ static struct pat_ref *_pat_ref_new(const char *display, unsigned int flags)
        ref->revision = 0;
        ref->entry_cnt = 0;
        LIST_INIT(&ref->head);
-       ref->ebmb_root = EB_ROOT;
+       ref->ceb_root = NULL;
        LIST_INIT(&ref->pat);
        HA_RWLOCK_INIT(&ref->lock);
        event_hdl_sub_list_init(&ref->e_subs);
@@ -2002,9 +1993,7 @@ struct pat_ref_elt *pat_ref_append(struct pat_ref *ref, const char *pattern, con
        elt->list_head = NULL;
        elt->tree_head = NULL;
        LIST_APPEND(&ref->head, &elt->list);
-       /* Even if calloc()'ed, ensure this node is not linked to a tree. */
-       elt->node.node.leaf_p = NULL;
-       ebst_insert(&ref->ebmb_root, &elt->node);
+       cebs_item_insert(&ref->ceb_root, node, pattern, elt);
        HA_ATOMIC_INC(&patterns_added);
        return elt;
  fail:
@@ -2178,7 +2167,7 @@ int pat_ref_purge_range(struct pat_ref *ref, uint from, uint to, int budget)
                pat_delete_gen(ref, elt);
 
                LIST_DELETE(&elt->list);
-               ebmb_delete(&elt->node);
+               cebs_item_delete(&ref->ceb_root, node, pattern, elt);
                free(elt->sample);
                free(elt);
                HA_ATOMIC_INC(&patterns_freed);