MEDIUM: guid: switch guid to more compact cebuis_tree

The current guid struct size is 56 bytes. Once reduced using compact
trees, it goes down to 32 (almost half). We're not on a critical path
and size matters here, so better switch to this.

It's worth noting that the name part could also be stored in the
guid_node at the end to save 8 extra byte (no pointer needed anymore),
however the purpose of this struct is to be embedded into other ones,
which is not compatible with having a dynamic size.

Affected struct sizes in bytes:

           Before     After   Diff
  server    4032       4032     0*
  proxy     3184       3160    -24
  listener   752        728    -24

*: struct server is full of holes and padding (176 bytes) and is
64-byte aligned. Moving the guid_node elsewhere such as after sess_conn
reduces it to 3968, or one less cache line. There's no point in moving
anything now because forthcoming patches will arrange other parts.
This commit is contained in:
Willy Tarreau
2025-02-17 09:39:04 +01:00
parent e36b3b60b3
commit 91258fb9d8
5 changed files with 21 additions and 27 deletions

View File

@@ -1,14 +1,15 @@
#ifndef _HAPROXY_GUID_T_H
#define _HAPROXY_GUID_T_H
#include <import/ebtree-t.h>
#include <import/cebtree.h>
#include <haproxy/obj_type-t.h>
/* Maximum GUID size excluding final '\0' */
#define GUID_MAX_LEN 127
struct guid_node {
struct ebpt_node node; /* attach point into GUID global tree */
struct ceb_node node; /* attach point into GUID global tree */
char *key; /* the key itself */
enum obj_type *obj_type; /* pointer to GUID obj owner */
};

View File

@@ -17,7 +17,7 @@ struct guid_node *guid_lookup(const char *uid);
*/
static inline const char *guid_get(const struct guid_node *guid)
{
return guid->node.key;
return guid->key;
}
int guid_is_valid_fmt(const char *uid, char **errmsg);

View File

@@ -83,7 +83,7 @@ static int _counters_shared_prepare(struct counters_shared *shared,
struct be_counters_shared *be_shared;
int it = 0;
if (!guid->node.key || !shm_stats_file_hdr)
if (!guid->key || !shm_stats_file_hdr)
shared->flags |= COUNTERS_SHARED_F_LOCAL;
while (it < global.nbtgroups) {

View File

@@ -1,6 +1,6 @@
#include <haproxy/guid.h>
#include <import/ebistree.h>
#include <import/cebis_tree.h>
#include <haproxy/listener-t.h>
#include <haproxy/obj_type.h>
#include <haproxy/proxy.h>
@@ -9,15 +9,16 @@
#include <haproxy/thread.h>
/* GUID global tree */
struct eb_root guid_tree = EB_ROOT_UNIQUE;
struct ceb_root *guid_tree = NULL;
__decl_thread(HA_RWLOCK_T guid_lock);
/* note: touched under the guid_lock */
static int _guid_count = 0;
/* Initialize <guid> members. */
void guid_init(struct guid_node *guid)
{
guid->node.key = NULL;
guid->node.node.leaf_p = NULL;
memset(guid, 0, sizeof(*guid));
}
/* Insert <objt> into GUID global tree with key <uid>. Must only be called on
@@ -30,7 +31,6 @@ int guid_insert(enum obj_type *objt, const char *uid, char **errmsg)
{
struct guid_node *guid = NULL;
struct guid_node *dup;
struct ebpt_node *node;
char *dup_name = NULL;
if (!guid_is_valid_fmt(uid, errmsg))
@@ -55,16 +55,15 @@ int guid_insert(enum obj_type *objt, const char *uid, char **errmsg)
return 0;
}
guid->node.key = strdup(uid);
if (!guid->node.key) {
guid->key = strdup(uid);
if (!guid->key) {
memprintf(errmsg, "key alloc failure");
goto err;
}
HA_RWLOCK_WRLOCK(GUID_LOCK, &guid_lock);
node = ebis_insert(&guid_tree, &guid->node);
if (node != &guid->node) {
dup = ebpt_entry(node, struct guid_node, node);
dup = cebuis_item_insert(&guid_tree, node, key, guid);
if (dup != guid) {
HA_RWLOCK_WRUNLOCK(GUID_LOCK, &guid_lock);
dup_name = guid_name(dup);
memprintf(errmsg, "duplicate entry with %s", dup_name);
@@ -79,10 +78,8 @@ int guid_insert(enum obj_type *objt, const char *uid, char **errmsg)
err:
if (guid)
ha_free(&guid->node.key);
ha_free(&guid->key);
ha_free(&dup_name);
if (guid)
guid->node.key = NULL; /* so that we can check that guid is not in a tree */
return 1;
}
@@ -92,10 +89,10 @@ int guid_insert(enum obj_type *objt, const char *uid, char **errmsg)
void guid_remove(struct guid_node *guid)
{
HA_RWLOCK_WRLOCK(GUID_LOCK, &guid_lock);
ebpt_delete(&guid->node);
if (guid->node.key)
if (guid->key)
_guid_count--;
ha_free(&guid->node.key);
cebuis_item_delete(&guid_tree, node, key, guid);
ha_free(&guid->key);
HA_RWLOCK_WRUNLOCK(GUID_LOCK, &guid_lock);
}
@@ -105,7 +102,6 @@ void guid_remove(struct guid_node *guid)
*/
struct guid_node *guid_lookup(const char *uid)
{
struct ebpt_node *node = NULL;
struct guid_node *guid = NULL;
/* For now, guid_lookup() is only used during startup in single-thread
@@ -114,10 +110,7 @@ struct guid_node *guid_lookup(const char *uid)
*/
BUG_ON(!(global.mode & MODE_STARTING));
node = ebis_lookup(&guid_tree, uid);
if (node)
guid = ebpt_entry(node, struct guid_node, node);
guid = cebuis_item_lookup(&guid_tree, node, key, uid, struct guid_node);
return guid;
}

View File

@@ -65,10 +65,10 @@ int stats_dump_fields_file(struct buffer *out,
}
/* Skip objects without GUID. */
if (!guid->node.key)
if (!guid->key)
return 1;
chunk_appendf(out, "%s,", (char *)guid->node.key);
chunk_appendf(out, "%s,", (char *)guid->key);
for (i = 0; i < stats_count; ++i) {
/* Empty field for stats-file is used to skip its output,