RE: [PATCH] mempool: split statistics from debug

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Morten Brørup [mailto:mb@smartsharesystems.com]
> Sent: Sunday, 30 October 2022 12.55
> 
> Split statistics from debug, to make mempool statistics available
> without
> the performance cost of continuously validating the cookies in the
> mempool
> elements.

mempool_perf_autotest shows that the rate_persec drops to a third (-66 %) when enabling full mempool debug - quite prohibitive!

With this patch, the performance cost is much lower. I don't have detailed test results, but the initial tests without mempool cache show a performance drop of -11 %. Significant, but not prohibitive.

> 
> Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> ---
>  config/rte_config.h       |  2 ++
>  lib/mempool/rte_mempool.c | 10 +++++-----
>  lib/mempool/rte_mempool.h | 14 +++++++-------
>  3 files changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/config/rte_config.h b/config/rte_config.h
> index ae56a86394..1b12d30557 100644
> --- a/config/rte_config.h
> +++ b/config/rte_config.h
> @@ -47,6 +47,8 @@
> 
>  /* mempool defines */
>  #define RTE_MEMPOOL_CACHE_MAX_SIZE 512
> +// RTE_LIBRTE_MEMPOOL_STATS is not set
> +// RTE_LIBRTE_MEMPOOL_DEBUG is not set
> 
>  /* mbuf defines */
>  #define RTE_MBUF_DEFAULT_MEMPOOL_OPS "ring_mp_mc"
> diff --git a/lib/mempool/rte_mempool.c b/lib/mempool/rte_mempool.c
> index 21c94a2b9f..f180257a54 100644
> --- a/lib/mempool/rte_mempool.c
> +++ b/lib/mempool/rte_mempool.c
> @@ -818,8 +818,8 @@ rte_mempool_create_empty(const char *name, unsigned
> n, unsigned elt_size,
>  			  RTE_CACHE_LINE_MASK) != 0);
>  	RTE_BUILD_BUG_ON((sizeof(struct rte_mempool_cache) &
>  			  RTE_CACHE_LINE_MASK) != 0);
> -#ifdef RTE_LIBRTE_MEMPOOL_DEBUG
> -	RTE_BUILD_BUG_ON((sizeof(struct rte_mempool_debug_stats) &
> +#ifdef RTE_LIBRTE_MEMPOOL_STATS
> +	RTE_BUILD_BUG_ON((sizeof(struct rte_mempool_stats) &
>  			  RTE_CACHE_LINE_MASK) != 0);
>  	RTE_BUILD_BUG_ON((offsetof(struct rte_mempool, stats) &
>  			  RTE_CACHE_LINE_MASK) != 0);
> @@ -1221,9 +1221,9 @@ rte_mempool_audit(struct rte_mempool *mp)
>  void
>  rte_mempool_dump(FILE *f, struct rte_mempool *mp)
>  {
> -#ifdef RTE_LIBRTE_MEMPOOL_DEBUG
> +#ifdef RTE_LIBRTE_MEMPOOL_STATS
>  	struct rte_mempool_info info;
> -	struct rte_mempool_debug_stats sum;
> +	struct rte_mempool_stats sum;
>  	unsigned lcore_id;
>  #endif
>  	struct rte_mempool_memhdr *memhdr;
> @@ -1269,7 +1269,7 @@ rte_mempool_dump(FILE *f, struct rte_mempool *mp)
>  	fprintf(f, "  common_pool_count=%u\n", common_count);
> 
>  	/* sum and dump statistics */
> -#ifdef RTE_LIBRTE_MEMPOOL_DEBUG
> +#ifdef RTE_LIBRTE_MEMPOOL_STATS
>  	rte_mempool_ops_get_info(mp, &info);
>  	memset(&sum, 0, sizeof(sum));
>  	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
> diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h
> index 3725a72951..89640e48e5 100644
> --- a/lib/mempool/rte_mempool.h
> +++ b/lib/mempool/rte_mempool.h
> @@ -56,14 +56,14 @@ extern "C" {
>  #define RTE_MEMPOOL_HEADER_COOKIE2  0xf2eef2eedadd2e55ULL /**< Header
> cookie. */
>  #define RTE_MEMPOOL_TRAILER_COOKIE  0xadd2e55badbadbadULL /**< Trailer
> cookie.*/
> 
> -#ifdef RTE_LIBRTE_MEMPOOL_DEBUG
> +#ifdef RTE_LIBRTE_MEMPOOL_STATS
>  /**
>   * A structure that stores the mempool statistics (per-lcore).
>   * Note: Cache stats (put_cache_bulk/objs, get_cache_bulk/objs) are
> not
>   * captured since they can be calculated from other stats.
>   * For example: put_cache_objs = put_objs - put_common_pool_objs.
>   */
> -struct rte_mempool_debug_stats {
> +struct rte_mempool_stats {
>  	uint64_t put_bulk;             /**< Number of puts. */
>  	uint64_t put_objs;             /**< Number of objects
> successfully put. */
>  	uint64_t put_common_pool_bulk; /**< Number of bulks enqueued in
> common pool. */
> @@ -237,9 +237,9 @@ struct rte_mempool {
>  	uint32_t nb_mem_chunks;          /**< Number of memory chunks */
>  	struct rte_mempool_memhdr_list mem_list; /**< List of memory
> chunks */
> 
> -#ifdef RTE_LIBRTE_MEMPOOL_DEBUG
> +#ifdef RTE_LIBRTE_MEMPOOL_STATS
>  	/** Per-lcore statistics. */
> -	struct rte_mempool_debug_stats stats[RTE_MAX_LCORE];
> +	struct rte_mempool_stats stats[RTE_MAX_LCORE];
>  #endif
>  }  __rte_cache_aligned;
> 
> @@ -293,16 +293,16 @@ struct rte_mempool {
>  	| RTE_MEMPOOL_F_NO_IOVA_CONTIG \
>  	)
>  /**
> - * @internal When debug is enabled, store some statistics.
> + * @internal When stats are enabled, store some statistics.
>   *
>   * @param mp
>   *   Pointer to the memory pool.
>   * @param name
>   *   Name of the statistics field to increment in the memory pool.
>   * @param n
> - *   Number to add to the object-oriented statistics.
> + *   Number to add to the statistics.
>   */
> -#ifdef RTE_LIBRTE_MEMPOOL_DEBUG
> +#ifdef RTE_LIBRTE_MEMPOOL_STATS
>  #define RTE_MEMPOOL_STAT_ADD(mp, name, n) do {                  \
>  		unsigned __lcore_id = rte_lcore_id();           \
>  		if (__lcore_id < RTE_MAX_LCORE) {               \
> --
> 2.17.1