Hi Morten,

On Wed, Oct 26, 2022 at 04:44:36PM +0200, Morten Brørup wrote:
> Add __rte_cache_aligned to the objs array.
> 
> It makes no difference in the general case, but if get/put operations are
> always 32 objects, it will reduce the number of memory (or last level
> cache) accesses from five to four 64 B cache lines for every get/put
> operation.
> 
> For readability reasons, an example using 16 objects follows:
> 
> Currently, with 16 objects (128B), we access to 3
> cache lines:
> 
>       ┌────────┐
>       │len     │
> cache │********│---
> line0 │********│ ^
>       │********│ |
>       ├────────┤ | 16 objects
>       │********│ | 128B
> cache │********│ |
> line1 │********│ |
>       │********│ |
>       ├────────┤ |
>       │********│_v_
> cache │        │
> line2 │        │
>       │        │
>       └────────┘
> 
> With the alignment, it is also 3 cache lines:
> 
>       ┌────────┐
>       │len     │
> cache │        │
> line0 │        │
>       │        │
>       ├────────┤---
>       │********│ ^
> cache │********│ |
> line1 │********│ |
>       │********│ |
>       ├────────┤ | 16 objects
>       │********│ | 128B
> cache │********│ |
> line2 │********│ |
>       │********│ v
>       └────────┘---
> 
> However, accessing the objects at the bottom of the mempool cache is a
> special case, where cache line0 is also used for objects.
> 
> Consider the next burst (and any following bursts):
> 
> Current:
>       ┌────────┐
>       │len     │
> cache │        │
> line0 │        │
>       │        │
>       ├────────┤
>       │        │
> cache │        │
> line1 │        │
>       │        │
>       ├────────┤
>       │        │
> cache │********│---
> line2 │********│ ^
>       │********│ |
>       ├────────┤ | 16 objects
>       │********│ | 128B
> cache │********│ |
> line3 │********│ |
>       │********│ |
>       ├────────┤ |
>       │********│_v_
> cache │        │
> line4 │        │
>       │        │
>       └────────┘
> 4 cache lines touched, incl. line0 for len.
> 
> With the proposed alignment:
>       ┌────────┐
>       │len     │
> cache │        │
> line0 │        │
>       │        │
>       ├────────┤
>       │        │
> cache │        │
> line1 │        │
>       │        │
>       ├────────┤
>       │        │
> cache │        │
> line2 │        │
>       │        │
>       ├────────┤
>       │********│---
> cache │********│ ^
> line3 │********│ |
>       │********│ | 16 objects
>       ├────────┤ | 128B
>       │********│ |
> cache │********│ |
> line4 │********│ |
>       │********│_v_
>       └────────┘
> Only 3 cache lines touched, incl. line0 for len.

I understand your logic, but are we sure that having an application that
works with bulks of 32 means that the cache will stay aligned to 32
elements for the whole life of the application?

In an application, the alignment of the cache can change if you have
any of:
- software queues (reassembly for instance)
- packet duplication (bridge, multicast)
- locally generated packets (keepalive, control protocol)
- pipeline to other cores

Even with testpmd, which work by bulk of 32, I can see that the size
of the cache filling is not aligned to 32. Right after starting the
application, we already have this:

  internal cache infos:
    cache_size=250
    cache_count[0]=231

This is probably related to the hw rx rings size, number of queues,
number of ports.

The "250" default value for cache size in testpmd is questionable, but
with --mbcache=256, the behavior is similar.

Also, when we transmit to a NIC, the mbufs are not returned immediatly
to the pool, they may stay in the hw tx ring during some time, which is
a driver decision.

After processing traffic on cores 8 and 24 with this testpmd, I get:
    cache_count[0]=231
    cache_count[8]=123
    cache_count[24]=122

In my opinion, it is not realistic to think that the mempool cache will
remain aligned to cachelines. In these conditions, it looks better to
keep the structure packed to avoid wasting memory.

Olivier


> 
> Credits go to Olivier Matz for the nice ASCII graphics.
> 
> Signed-off-by: Morten Brørup <m...@smartsharesystems.com>
> ---
>  lib/mempool/rte_mempool.h | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h
> index 1f5707f46a..3725a72951 100644
> --- a/lib/mempool/rte_mempool.h
> +++ b/lib/mempool/rte_mempool.h
> @@ -86,11 +86,13 @@ struct rte_mempool_cache {
>       uint32_t size;        /**< Size of the cache */
>       uint32_t flushthresh; /**< Threshold before we flush excess elements */
>       uint32_t len;         /**< Current cache count */
> -     /*
> +     /**
> +      * Cache objects
> +      *
>        * Cache is allocated to this size to allow it to overflow in certain
>        * cases to avoid needless emptying of cache.
>        */
> -     void *objs[RTE_MEMPOOL_CACHE_MAX_SIZE * 2]; /**< Cache objects */
> +     void *objs[RTE_MEMPOOL_CACHE_MAX_SIZE * 2] __rte_cache_aligned;
>  } __rte_cache_aligned;
>  
>  /**
> -- 
> 2.17.1
> 

Reply via email to