From e36b3b60b3fcf24e327fe398541aa302802bf521 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Sun, 12 Jan 2025 19:38:28 +0100 Subject: [PATCH] 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. --- include/haproxy/pattern-t.h | 5 ++-- src/pattern.c | 57 +++++++++++++++---------------------- 2 files changed, 26 insertions(+), 36 deletions(-) diff --git a/include/haproxy/pattern-t.h b/include/haproxy/pattern-t.h index 35c4fd095..de1a308e1 100644 --- 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 #include #include @@ -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 ebtree. */ + struct ceb_node node; /* Node to attach this element to its 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 3e25b88e7..997184139 100644 --- a/src/pattern.c +++ b/src/pattern.c @@ -14,6 +14,7 @@ #include #include +#include #include #include #include @@ -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 the sample of all patterns matching @@ -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 the sample of all patterns matching @@ -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);