pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/41865?usp=email )

Change subject: gmstap_log: optimization: Add talloc_pool for transmitted messages ......................................................................

gmstap_log: optimization: Add talloc_pool for transmitted messages

In synchronous modes (blocking and non-blocking), set a pool size of 1 msgb. This way we avoid re-allocating the msgb with malloc/free everytime a gsmtap_log msg is transmitted over the target, and instead we allocate the msgb during creation of the target.

In asynchronous mode (workqueue), create a pool of 100 msgbs, which should in general be enough to speedup more usual needs. If more than 100 msgbs are to be enqueued into the gsmtap_inst, then they will be allocated as normal talloc contexts.

Since the struct log_target is public, it is currently not possible to add new fields to it under "tgt_gsmtap" without breaking the ABI. Hence, the ->output field, which is unused in this target, is reused to store the pool object internally.

Change-Id: I19cdf09f21d856e8e3646de495ce7ae040195268 --- M configure.ac M src/core/logging_gsmtap.c 2 files changed, 34 insertions(+), 2 deletions(-)

Approvals: Jenkins Builder: Verified laforge: Looks good to me, but someone else must approve pespin: Looks good to me, approved osmith: Looks good to me, but someone else must approve

diff --git a/configure.ac b/configure.ac index 565d392..b0914f7 100644 --- a/configure.ac +++ b/configure.ac @@ -455,6 +455,9 @@ )], [enable_pseudotalloc=$enableval], [enable_pseudotalloc=$ENABLE_PSEUDOTALLOC_DEFAULT]) AM_CONDITIONAL(ENABLE_PSEUDOTALLOC, test x"$enable_pseudotalloc" = x"yes") +AS_IF([test "x$enable_pseudotalloc" = "xyes"], [ + AC_DEFINE([ENABLE_PSEUDOTALLOC], [1], [Enable building pseudotalloc library]) +])

AC_ARG_ENABLE(log_macros, [AS_HELP_STRING( diff --git a/src/core/logging_gsmtap.c b/src/core/logging_gsmtap.c index 7775c27..062efd9 100644 --- a/src/core/logging_gsmtap.c +++ b/src/core/logging_gsmtap.c @@ -50,6 +50,9 @@ #include <osmocom/core/thread.h>

#define GSMTAP_LOG_MAX_SIZE 4096 +#define GSMTAP_MSG_MAX_SIZE (sizeof(struct gsmtap_hdr) + \ + sizeof(struct gsmtap_osmocore_log_hdr) + \ + GSMTAP_LOG_MAX_SIZE)

static __thread uint32_t logging_gsmtap_tid;

@@ -70,8 +73,8 @@ /* get timestamp ASAP */ osmo_gettimeofday(&tv, NULL);

- msg = msgb_alloc(sizeof(*gh)+sizeof(*golh)+GSMTAP_LOG_MAX_SIZE, - "GSMTAP logging"); + + msg = msgb_alloc_c((void *)target->output, GSMTAP_MSG_MAX_SIZE, "GSMTAP logging");

/* GSMTAP header */ gh = (struct gsmtap_hdr *) msgb_put(msg, sizeof(*gh)); @@ -150,6 +153,32 @@ return NULL; }

+#ifndef ENABLE_PSEUDOTALLOC + size_t num_pool_objects; + if (ofd_wq_mode) { + /* Allocate a talloc pool to avoid malloc() on the first 100 + * concurrently queued msgbs (~400KB per gsmtap_log target). + * Once the talloc_pool is full, new normal talloc chunks will be used. */ + num_pool_objects = 100; + } else { + /* When in synchronous mode (blocking & non-blocking), there's + * no queueing in gsmtap_sendmsg() so there's no need to have a + * pool with multiple msgbs. */ + num_pool_objects = 1; + } + /* XXX: Abuse "output" field to store the talloc_pool, since it's not + * used in gsmtap log target. This can be moved to its own field once we + * make "struct log_target" private. + */ + target->output = _talloc_pooled_object(target, 0, "gsmtap_log_msgb_pool", + num_pool_objects, + (sizeof(struct msgb) + GSMTAP_MSG_MAX_SIZE) * num_pool_objects); +#else + /* talloc pools not supported by pseudotalloc, allocate on usual msgb ctx instead: */ + extern void *tall_msgb_ctx; + target->output = tall_msgb_ctx; +#endif /* ifndef ENABLE_PSEUDOTALLOC */ + if (add_sink) gsmtap_source_add_sink(gti);