> > > > diff --git a/lib/mempool/rte_mempool.h
> b/lib/mempool/rte_mempool.h
> > > > index 5a81e53a9f..5fc7cdb79c 100644
> > > > --- a/lib/mempool/rte_mempool.h
> > > > +++ b/lib/mempool/rte_mempool.h
> > > > @@ -105,8 +105,7 @@ struct __rte_cache_aligned rte_mempool_cache
> {
> > > > } stats; /**< Statistics */
> > > > #endif
> > > > /** Cache objects */
> > > > - alignas(RTE_CACHE_LINE_SIZE) void
> > > > *objs[RTE_MEMPOOL_CACHE_MAX_SIZE];
> > > > - RTE_CACHE_GUARD;
> > > > + alignas(RTE_CACHE_LINE_SIZE) void *objs[];
> > >
> > > LGTM to me in general.
> > > Again, just as a thought:
> > > Would it be plausible to have a pointer per lcore instead?
> > > i.e.:
> > > alignas(RTE_CACHE_LINE_SIZE) void *objs[RTE_MAX_LCORE];
> > > That way we can avoid multiply op in rte_mempool_default_cache(),
> > > also it will allow us to allocate lcore cache on demand (only for
> > > enabled lcores).
> > > As a downside - sizeof(rte_mempool_cache) will grow by
> > > (RTE_MAX_LCORE - 1) * sizeof(uintptr_t), but that's probably not a
> big
> > > deal.
> >
> > The multiply op is not expensive.
> >
> > This patch has:
> > struct rte_mempool_cache *cache =
> > (struct rte_mempool_cache *)
> > RTE_PTR_ADD(
> > mp->local_cache,
> > lcore_id * (size_t)mp->sizeof_cache_per_lcore);
> >
> > Which compiles to something like:
> > ptr = LOAD(mp + offsetof(cache_ptr));
> > off = LOAD(mp + offsetof(sizeof_cache_per_lcore));
> > off *= lcore_id;
> > ptr += off;
> >
> >
> > Yes, having an array of pointers to the caches in the mempool would
> be a good
> > alternative:
> >
> > struct __rte_cache_aligned rte_mempool {
> > - struct rte_mempool_cache *local_cache; /**< Per-lcore local cache
> */
> >
> > [...]
> > struct rte_mempool_debug_stats stats[RTE_MAX_LCORE + 1];
> > #endif
> > + /** Per-lcore local cache */
> > + struct rte_mempool_cache *local_cache[RTE_MAX_LCORE];
> > };
>
> Yes, you right, off-course it has to be that way.
>
> >
> >
> > Lookup for that would become simple:
> > struct rte_mempool_cache *cache =
> > mp->local_cache[lcore_id];
> >
> > Which compiles to something like:
> > off = lcore_id * sizeof(void*); // Shift operation
> > off += offsetof(cache_ptr_array);
> > ptr = LOAD(mp + off);
> >
> > Yes, I suppose that would be faster.
> > I'll take a stab at it.
I tried it, but the mempool perf test yields similar results on our build
server (a virtual machine).
There are also a couple of disadvantages:
1. The table has a larger memory (and cache) footprint than the solution with
the base pointer and multiplier. We knew that, and consider it insignificant.
2. If the mempool cache misses, the backend must be involved, and this needs to
load mp->ops_index. With the base/multiplier variant, ops_index is in the same
cache line as the local_cache pointer, and thus already hot in the cache. With
the lcore_id indexed local_cache pointer array, ops_index is not hot in the
cache. Obviously, we want to optimize for mempool cache hits, so this should be
considered insignificant; I only mention it for completeness.
I think we should stick with the patch series as is.
If you want to play around with the lcore_id indexed local_cache pointer array,
a patch is provided at the bottom of this email.
> >
> > Caches for all lcores must be allocated at mempool creation. Or we
> would need
> > special handling to allocate/free the relevant per-lcore cache for
> all registered
> > mempools whenever a new lcore is registered for use at runtime, e.g.
> by
> > rte_thread_register()/unregister().
>
> My initial thought was:
> we can allocate caches for all lcores that are already enabled at
> mempool_init time.
> If later, such core will be disabled - that's probably not a big deal,
> we can simply left the cache for it
> hanging around unused until mempool will be destroyed.
> For lcores that will be enabled after mempool_init() we can use lazy
> allocation (at first access to it).
> But after another thought - that's probably not such good idea, as in
> principle rte_malloc() here can fail.
>
> > >
> > > > };
> > > >
> > > > /**
> > > > @@ -265,6 +264,7 @@ struct __rte_cache_aligned rte_mempool {
> > > > int32_t ops_index;
> > > >
> > > > struct rte_mempool_cache *local_cache; /**< Per-lcore local
> cache
> > > */
> > > > + uint32_t sizeof_cache_per_lcore; /**< Multiplier for
> indexing
> > > into the
> > > > local cache. */
> > > >
> > > > uint32_t populated_size; /**< Number of populated
> > > objects. */
> > > > struct rte_mempool_objhdr_list elt_list; /**< List of
> objects in
> > > pool */
> > > > @@ -1050,8 +1050,7 @@ rte_mempool_free(struct rte_mempool *mp);
> > > > * @param cache_size
> > > > * If cache_size is non-zero, the rte_mempool library will try
> to
> > > > * limit the accesses to the common lockless pool, by
> maintaining
> > > a
> > > > - * per-lcore object cache. This argument must be lower or
> equal to
> > > > - * RTE_MEMPOOL_CACHE_MAX_SIZE and n.
> > > > + * per-lcore object cache. This argument must be lower or
> equal to
> > > n.
> > > > * The access to the per-lcore table is of course
> > > > * faster than the multi-producer/consumer pool. The cache can
> be
> > > > * disabled if the cache_size argument is set to 0; it can be
> > > useful to
> > > > @@ -1371,15 +1370,17 @@ rte_mempool_cache_create(uint32_t size,
> int
> > > > socket_id)
> > > > static __rte_always_inline struct rte_mempool_cache *
> > > > rte_mempool_default_cache(struct rte_mempool *mp, unsigned
> lcore_id)
> > > > {
> > > > - if (unlikely(mp->cache_size == 0))
> > > > + if (unlikely(mp->local_cache == NULL))
> > > > return NULL;
> > > >
> > > > if (unlikely(lcore_id == LCORE_ID_ANY))
> > > > return NULL;
> > > >
> > > > - rte_mempool_trace_default_cache(mp, lcore_id,
> > > > - &mp->local_cache[lcore_id]);
> > > > - return &mp->local_cache[lcore_id];
> > > > + struct rte_mempool_cache *cache = (struct rte_mempool_cache
> > > > *)RTE_PTR_ADD(mp->local_cache,
> > > > + lcore_id * (size_t)mp->sizeof_cache_per_lcore);
> > > > + rte_mempool_trace_default_cache(mp, lcore_id, cache);
> > > > + __rte_assume(cache != NULL);
> > > > + return cache;
> > > > }
> > > >
> > > > /**
> > > > --
> > > > 2.43.0
diff --git a/doc/guides/rel_notes/release_26_11.rst
b/doc/guides/rel_notes/release_26_11.rst
index 64f6802e58..42fc5d2ad2 100644
--- a/doc/guides/rel_notes/release_26_11.rst
+++ b/doc/guides/rel_notes/release_26_11.rst
@@ -105,7 +105,7 @@ API Changes
as it is often used for the ``cache_size`` parameter when creating mempools.
* mempool: Updated the ``rte_mempool`` structure as follows:
- - Added the ``sizeof_cache_per_lcore`` field, for indexing into the
per-lcore local cache.
+ - Changed the ``local_cache`` field to array of pointers to per-lcore local
cache, for improved lookup performance.
* mempool: Updated the ``rte_mempool_cache`` structure as follows:
- Removed the deprecated and obsolete ``flushthresh`` field.
diff --git a/lib/mempool/rte_mempool.c b/lib/mempool/rte_mempool.c
index 069aabaa70..556ecfe150 100644
--- a/lib/mempool/rte_mempool.c
+++ b/lib/mempool/rte_mempool.c
@@ -759,7 +759,7 @@ mempool_cache_init(struct rte_mempool_cache *cache,
uint32_t size)
/*
* Create and initialize a cache for objects that are retrieved from and
* returned to an underlying mempool. This structure is identical to the
- * local_cache entry pointed to by the mempool structure.
+ * local_cache[lcore_id] pointed to by the mempool structure.
*/
RTE_EXPORT_SYMBOL(rte_mempool_cache_create)
struct rte_mempool_cache *
@@ -930,6 +930,7 @@ rte_mempool_create_empty(const char *name, unsigned n,
unsigned elt_size,
mp->elt_size = objsz.elt_size;
mp->header_size = objsz.header_size;
mp->trailer_size = objsz.trailer_size;
+ /* Size of default caches, zero means disabled. */
mp->cache_size = cache_size;
mp->private_data_size = private_data_size;
STAILQ_INIT(&mp->elt_list);
@@ -953,17 +954,16 @@ rte_mempool_create_empty(const char *name, unsigned n,
unsigned elt_size,
goto exit_unlock;
}
- /* local_cache pointer is only set if per-lcore local cache is present
*/
+ /* local_cache pointers are only set if per-lcore local cache is
present */
if (cache_size != 0) {
- mp->local_cache = (struct rte_mempool_cache *)
- RTE_PTR_ADD(mp, sizeof(struct rte_mempool) +
private_data_size);
- mp->sizeof_cache_per_lcore = sizeof_cache_per_lcore;
-
/* Init all default caches. */
- struct rte_mempool_cache *cache = mp->local_cache;
+ struct rte_mempool_cache *cache = (struct rte_mempool_cache *)
+ RTE_PTR_ADD(mp, sizeof(struct rte_mempool) +
private_data_size);
for (unsigned int lcore_id = 0; lcore_id < RTE_MAX_LCORE;
- lcore_id++, cache = RTE_PTR_ADD(cache,
sizeof_cache_per_lcore))
+ lcore_id++, cache = RTE_PTR_ADD(cache,
sizeof_cache_per_lcore)) {
+ mp->local_cache[lcore_id] = cache;
mempool_cache_init(cache, cache_size);
+ }
}
te->data = mp;
@@ -1026,17 +1026,16 @@ RTE_EXPORT_SYMBOL(rte_mempool_avail_count)
unsigned int
rte_mempool_avail_count(const struct rte_mempool *mp)
{
- unsigned int count;
+ unsigned count;
+ unsigned lcore_id;
count = rte_mempool_ops_get_count(mp);
- if (mp->local_cache == NULL)
+ if (mp->cache_size == 0)
return count;
- const struct rte_mempool_cache *cache = mp->local_cache;
- for (unsigned int lcore_id = 0; lcore_id < RTE_MAX_LCORE;
- lcore_id++, cache = RTE_PTR_ADD(cache,
mp->sizeof_cache_per_lcore))
- count += cache->len;
+ for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++)
+ count += mp->local_cache[lcore_id]->len;
/*
* due to race condition (access to len is not locked), the
@@ -1064,11 +1063,11 @@ rte_mempool_stats_reset(struct rte_mempool *mp)
#ifdef RTE_LIBRTE_MEMPOOL_STATS
memset(&mp->stats, 0, sizeof(mp->stats));
- if (mp->local_cache != NULL) {
- struct rte_mempool_cache *cache = mp->local_cache;
- for (unsigned int lcore_id = 0; lcore_id < RTE_MAX_LCORE;
- lcore_id++, cache = RTE_PTR_ADD(cache,
mp->sizeof_cache_per_lcore))
- memset(&cache->stats, 0, sizeof(cache->stats));
+- if (mp->cache_size != 0) {
+- for (unsigned int lcore_id = 0; lcore_id < RTE_MAX_LCORE;
lcore_id++) {
+- memset(&mp->local_cache[lcore_id]->stats, 0,
+-
sizeof(mp->local_cache[lcore_id]->stats));
+- }
}
RTE_MEMPOOL_LOG(DEBUG, "<%s>@%p: statistics reset", mp->name, mp);
@@ -1082,18 +1081,18 @@ rte_mempool_stats_reset(struct rte_mempool *mp)
static unsigned
rte_mempool_dump_cache(FILE *f, const struct rte_mempool *mp)
{
- unsigned int count = 0;
+ unsigned lcore_id;
+ unsigned count = 0;
+ unsigned cache_count;
fprintf(f, " internal cache infos (hide zero value items):\n");
fprintf(f, " cache_size=%"PRIu32"\n", mp->cache_size);
- if (mp->local_cache == NULL)
+ if (mp->cache_size == 0)
return count;
- const struct rte_mempool_cache *cache = mp->local_cache;
- for (unsigned int lcore_id = 0; lcore_id < RTE_MAX_LCORE;
- lcore_id++, cache = RTE_PTR_ADD(cache,
mp->sizeof_cache_per_lcore)) {
- unsigned int cache_count = cache->len;
+ for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
+ cache_count = mp->local_cache[lcore_id]->len;
if (cache_count == 0)
continue;
fprintf(f, " cache_count[%u]=%"PRIu32"\n",
@@ -1234,12 +1233,14 @@ mempool_audit_cookies(struct rte_mempool *mp)
static void
mempool_audit_cache(const struct rte_mempool *mp)
{
- if (mp->local_cache == NULL)
+ unsigned lcore_id;
+
+ if (mp->cache_size == 0)
return;
- const struct rte_mempool_cache *cache = mp->local_cache;
- for (unsigned int lcore_id = 0; lcore_id < RTE_MAX_LCORE;
- lcore_id++, cache = RTE_PTR_ADD(cache,
mp->sizeof_cache_per_lcore)) {
+ for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
+ const struct rte_mempool_cache *cache;
+ cache = mp->local_cache[lcore_id];
if (cache->size > mp->size) {
RTE_MEMPOOL_LOG(CRIT, "badness on cache[%u] size",
lcore_id);
rte_panic("MEMPOOL: invalid cache[%u] size\n",
lcore_id);
@@ -1333,15 +1334,13 @@ rte_mempool_dump(FILE *f, struct rte_mempool *mp)
sum.get_success_blks += mp->stats[lcore_id].get_success_blks;
sum.get_fail_blks += mp->stats[lcore_id].get_fail_blks;
}
- if (mp->local_cache != NULL) {
+ if (mp->cache_size != 0) {
/* Add the statistics stored in the mempool caches. */
- const struct rte_mempool_cache *cache = mp->local_cache;
- for (lcore_id = 0; lcore_id < RTE_MAX_LCORE;
- lcore_id++, cache = RTE_PTR_ADD(cache,
mp->sizeof_cache_per_lcore)) {
- sum.put_bulk += cache->stats.put_bulk;
- sum.put_objs += cache->stats.put_objs;
- sum.get_success_bulk += cache->stats.get_success_bulk;
- sum.get_success_objs += cache->stats.get_success_objs;
+ for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
+ sum.put_bulk +=
mp->local_cache[lcore_id]->stats.put_bulk;
+ sum.put_objs +=
mp->local_cache[lcore_id]->stats.put_objs;
+ sum.get_success_bulk +=
mp->local_cache[lcore_id]->stats.get_success_bulk;
+ sum.get_success_objs +=
mp->local_cache[lcore_id]->stats.get_success_objs;
}
}
fprintf(f, " stats:\n");
@@ -1638,11 +1637,10 @@ mempool_info_cb(struct rte_mempool *mp, void *arg)
mp->populated_size);
cache_count = 0;
- if (mp->local_cache != NULL) {
- const struct rte_mempool_cache *cache = mp->local_cache;
- for (unsigned int lcore_id = 0; lcore_id < RTE_MAX_LCORE;
- lcore_id++, cache = RTE_PTR_ADD(cache,
mp->sizeof_cache_per_lcore))
- cache_count += cache->len;
+ if (mp->cache_size > 0) {
+ int lcore_id;
+ for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++)
+ cache_count += mp->local_cache[lcore_id]->len;
}
rte_tel_data_add_dict_uint(info->d, "total_cache_count", cache_count);
common_count = rte_mempool_ops_get_count(mp);
diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h
index 5fc7cdb79c..cc255063f5 100644
--- a/lib/mempool/rte_mempool.h
+++ b/lib/mempool/rte_mempool.h
@@ -263,14 +263,18 @@ struct __rte_cache_aligned rte_mempool {
*/
int32_t ops_index;
- struct rte_mempool_cache *local_cache; /**< Per-lcore local cache */
- uint32_t sizeof_cache_per_lcore; /**< Multiplier for indexing into the
local cache. */
-
uint32_t populated_size; /**< Number of populated objects. */
struct rte_mempool_objhdr_list elt_list; /**< List of objects in pool */
uint32_t nb_mem_chunks; /**< Number of memory chunks */
struct rte_mempool_memhdr_list mem_list; /**< List of memory chunks */
+ /**
+ * Pointers to per-lcore local cache.
+ * Note: Cache line aligned for higher cache hit rate when only using
the first few lcores.
+ */
+ alignas(RTE_CACHE_LINE_SIZE)
+ struct rte_mempool_cache *local_cache[RTE_MAX_LCORE];
+
#ifdef RTE_LIBRTE_MEMPOOL_STATS
/** Per-lcore statistics.
*
@@ -1370,17 +1374,12 @@ rte_mempool_cache_create(uint32_t size, int socket_id)
static __rte_always_inline struct rte_mempool_cache *
rte_mempool_default_cache(struct rte_mempool *mp, unsigned lcore_id)
{
- if (unlikely(mp->local_cache == NULL))
- return NULL;
-
if (unlikely(lcore_id == LCORE_ID_ANY))
return NULL;
- struct rte_mempool_cache *cache = (struct rte_mempool_cache
*)RTE_PTR_ADD(mp->local_cache,
- lcore_id * (size_t)mp->sizeof_cache_per_lcore);
- rte_mempool_trace_default_cache(mp, lcore_id, cache);
- __rte_assume(cache != NULL);
- return cache;
+ rte_mempool_trace_default_cache(mp, lcore_id,
+ mp->local_cache[lcore_id]);
+ return mp->local_cache[lcore_id];
}
/**
@@ -1449,8 +1448,8 @@ rte_mempool_do_generic_put(struct rte_mempool *mp, void *
const *obj_table,
* are more hot, from the upper half of the cache.
*/
__rte_assume(cache->len > cache->size / 2);
- rte_mempool_ops_enqueue_bulk(mp, &cache->objs[0], cache->size /
2);
- rte_memcpy(&cache->objs[0], &cache->objs[cache->size / 2],
+ rte_mempool_ops_enqueue_bulk(mp, cache->objs, cache->size / 2);
+ rte_memcpy(cache->objs, &cache->objs[cache->size / 2],
sizeof(void *) * (cache->len - cache->size /
2));
cache_objs = &cache->objs[cache->len - cache->size / 2];
cache->len = cache->len - cache->size / 2 + n;
