MEDIUM: pools: be a bit smarter when merging comparable size pools

By default, pools of comparable sizes are merged together. However, the
current algorithm is dumb: it rounds the requested size to the next
multiple of 16 and compares the sizes like this. This results in many
entries which are already multiples of 16 not being merged, for example
1024 and 1032 are separate, 65536 and 65540 are separate, 48 and 56 are
separate (though 56 merges with 64).

This commit changes this to consider not just the entry size but also the
average entry size, that is, it compares the average size of all objects
sharing the pool with the size of the object looking for a pool. If the
object is not more than 1% bigger nor smaller than the current average
size or if it neither 16 bytes smaller nor larger, then it can be merged.
Also, it always respects exact matches in order to avoid merging objects
into larger pools or worse, extending existing ones for no reason, and
when there's a tie, it always avoids extending an existing pool.

Also, we now visit all existing pools in order to spot the best one, we
do not stop anymore at the smallest one large enough. Theoretically this
could cost a bit of CPU but in practice it's O(N^2) with N quite small
(typically in the order of 100) and the cost at each step is very low
(compare a few integer values). But as a side effect, pools are no
longer sorted by size, "show pools bysize" is needed for this.

This causes the objects to be much better grouped together, accepting to
use a little bit more sometimes to avoid fragmentation, without causing
everyone to be merged into the same pool. Thanks to this we're now
seeing 36 pools instead of 48 by default, with some very nice examples
of compact grouping:

  - Pool qc_stream_r (80 bytes) : 13 users
      >  qc_stream_r : size=72 flags=0x1 align=0
      >  quic_cstrea : size=80 flags=0x1 align=0
      >  qc_stream_a : size=64 flags=0x1 align=0
      >  hlua_esub   : size=64 flags=0x1 align=0
      >  stconn      : size=80 flags=0x1 align=0
      >  dns_query   : size=64 flags=0x1 align=0
      >  vars        : size=80 flags=0x1 align=0
      >  filter      : size=64 flags=0x1 align=0
      >  session pri : size=64 flags=0x1 align=0
      >  fcgi_hdr_ru : size=72 flags=0x1 align=0
      >  fcgi_param_ : size=72 flags=0x1 align=0
      >  pendconn    : size=80 flags=0x1 align=0
      >  capture     : size=64 flags=0x1 align=0

  - Pool h3s (56 bytes) : 17 users
      >  h3s         : size=56 flags=0x1 align=0
      >  qf_crypto   : size=48 flags=0x1 align=0
      >  quic_tls_se : size=48 flags=0x1 align=0
      >  quic_arng   : size=56 flags=0x1 align=0
      >  hlua_flt_ct : size=56 flags=0x1 align=0
      >  promex_metr : size=48 flags=0x1 align=0
      >  conn_hash_n : size=56 flags=0x1 align=0
      >  resolv_requ : size=48 flags=0x1 align=0
      >  mux_pt      : size=40 flags=0x1 align=0
      >  comp_state  : size=40 flags=0x1 align=0
      >  notificatio : size=48 flags=0x1 align=0
      >  tasklet     : size=56 flags=0x1 align=0
      >  bwlim_state : size=48 flags=0x1 align=0
      >  xprt_handsh : size=48 flags=0x1 align=0
      >  email_alert : size=56 flags=0x1 align=0
      >  caphdr      : size=41 flags=0x1 align=0
      >  caphdr      : size=41 flags=0x1 align=0

  - Pool quic_cids (32 bytes) : 13 users
      >  quic_cids   : size=16 flags=0x1 align=0
      >  quic_tls_ke : size=32 flags=0x1 align=0
      >  quic_tls_iv : size=12 flags=0x1 align=0
      >  cbuf        : size=32 flags=0x1 align=0
      >  hlua_queuew : size=24 flags=0x1 align=0
      >  hlua_queue  : size=24 flags=0x1 align=0
      >  promex_modu : size=24 flags=0x1 align=0
      >  cache_st    : size=24 flags=0x1 align=0
      >  spoe_appctx : size=32 flags=0x1 align=0
      >  ehdl_sub_tc : size=32 flags=0x1 align=0
      >  fcgi_flt_ct : size=16 flags=0x1 align=0
      >  sig_handler : size=32 flags=0x1 align=0
      >  pipe        : size=24 flags=0x1 align=0

  - Pool quic_crypto (1032 bytes) : 2 users
      >  quic_crypto : size=1032 flags=0x1 align=0
      >  requri      : size=1024 flags=0x1 align=0

  - Pool quic_conn_r (65544 bytes) : 2 users
      >  quic_conn_r : size=65536 flags=0x1 align=0
      >  dns_msg_buf : size=65540 flags=0x1 align=0

On a very unscientific test consisting in sending 1 million H1 requests
and 1 million H2 requests to the stats page, we're seeing an ~6% lower
memory usage with the patch:

  before the patch:
    Total: 48 pools, 4120832 bytes allocated, 4120832 used (~3555680 by thread caches).

  after the patch:
    Total: 36 pools, 3880648 bytes allocated, 3880648 used (~3299064 by thread caches).

This should be taken with care however since pools allocate and release
in batches.

diff --git a/include/haproxy/pool-t.h b/include/haproxy/pool-t.h
index c727614cd29fa8a36e175820062906dec168591b..35bf7a10abfc202d60229d95f550200cf1ff071e 100644 (file)
--- a/include/haproxy/pool-t.h
+++ b/include/haproxy/pool-t.h
@@ -125,6 +125,7 @@ struct pool_head {
        unsigned int flags;     /* MEM_F_* */
        unsigned int users;     /* number of pools sharing this zone */
        unsigned int alloc_sz;  /* allocated size (includes hidden fields) */
+       unsigned int sum_size;  /* sum of all registered users' size */
        struct list list;       /* list of all known pools */
        void *base_addr;        /* allocation address, for free() */
        char name[12];          /* name of the pool */

diff --git a/src/pool.c b/src/pool.c
index da6e389da15de61ed4a283c8935a2e5b4b2ed07a..f734e2a6b72764aa72c682ae94d667029a826348 100644 (file)
--- a/src/pool.c
+++ b/src/pool.c
@@ -298,6 +298,7 @@ struct pool_head *create_pool(char *name, unsigned int size, unsigned int flags)
        struct pool_head *entry;
        struct list *start;
        unsigned int align;
+       unsigned int best_diff;
        int thr __maybe_unused;
 
        pool = NULL;
@@ -338,9 +339,18 @@ struct pool_head *create_pool(char *name, unsigned int size, unsigned int flags)
        /* TODO: thread: we do not lock pool list for now because all pools are
         * created during HAProxy startup (so before threads creation) */
        start = &pools;
+       best_diff = ~0U;
 
        list_for_each_entry(entry, &pools, list) {
-               if (entry->size == size) {
+               if (entry->size == size ||
+                   (!(flags & MEM_F_EXACT) && !pool_allocated(entry) &&
+                    /* size within 1% of avg size */
+                    (((ullong)entry->sum_size * 100ULL < (ullong)size * entry->users * 101ULL &&
+                      (ullong)entry->sum_size * 101ULL > (ullong)size * entry->users * 100ULL) ||
+                     /* or +/- 16 compared to the current avg size */
+                     (entry->sum_size - 16 * entry->users < size * entry->users &&
+                      entry->sum_size + 16 * entry->users > size * entry->users)))) {
+
                        /* either we can share this place and we take it, or
                         * we look for a shareable one or for the next position
                         * before which we will insert a new one.
@@ -349,15 +359,28 @@ struct pool_head *create_pool(char *name, unsigned int size, unsigned int flags)
                            (!(pool_debugging & POOL_DBG_DONT_MERGE) ||
                             strcmp(name, entry->name) == 0)) {
                                /* we can share this one */
-                               pool = entry;
-                               DPRINTF(stderr, "Sharing %s with %s\n", name, pool->name);
-                               break;
+                               uint diff = (abs((int)(size * entry->users - entry->size)) + entry->users / 2) / entry->users;
+
+                               /* the principle here is:
+                                *   - if the best pool is smaller and the current
+                                *     candidate larger, we prefer the larger one
+                                *     so as not to grow an existing pool;
+                                *   - otherwise we go for the smallest distance
+                                *     from the existing one.
+                                */
+                               if (!pool || entry->size == size ||
+                                   (pool->size != size &&
+                                    ((pool->size < size && entry->size >= size) ||
+                                     (diff == best_diff && entry->size >= size) ||
+                                     (diff < best_diff)))) {
+                                       best_diff = diff;
+                                       pool = entry;
+                               }
                        }
                }
                else if (entry->size > size) {
                        /* insert before this one */
                        start = &entry->list;
-                       break;
                }
        }
 
@@ -389,9 +412,19 @@ struct pool_head *create_pool(char *name, unsigned int size, unsigned int flags)
                        }
                }
        }
+       else {
+               /* we found the best one */
+               if (size > pool->size) {
+                       pool->size = size;
+                       pool->alloc_sz = size + extra;
+               }
+               DPRINTF(stderr, "Sharing %s with %s\n", name, pool->name);
+       }
 
        LIST_APPEND(&pool->regs, &reg->list);
        pool->users++;
+       pool->sum_size += size;
+
        return pool;
  fail:
        free(reg);