[PATCH] mempool: try to get objects from cache when the mempool is single consumer and multiple producer

[PATCH] mempool: try to get objects from cache when the mempool is single consumer and multiple producer

[Wenfeng Liu]

We put objects to cache when the mempool is multiple producer, however the cache will not be used when it is single consumer.
With this patch we can get objects from cache when the single consumer is happen to be one of the producers, and this improves performance.

Signed-off-by: Wenfeng Liu <liuwf@arraynetworks.com.cn>
---
 lib/librte_mempool/rte_mempool.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h
index d315d42..4ab5a95 100644
--- a/lib/librte_mempool/rte_mempool.h
+++ b/lib/librte_mempool/rte_mempool.h
@@ -1250,8 +1250,9 @@ __mempool_generic_get(struct rte_mempool *mp, void **obj_table,
 	uint32_t index, len;
 	void **cache_objs;
 
-	/* No cache provided or single consumer */
-	if (unlikely(cache == NULL || flags & MEMPOOL_F_SC_GET ||
+	/* No cache provided or single consumer and single producer */
+	if (unlikely(cache == NULL ||
+		     (flags & MEMPOOL_F_SC_GET) && (flags & MEMPOOL_F_SP_PUT) ||
 		     n >= cache->size))
 		goto ring_dequeue;
 
-- 
2.7.4

Re: [PATCH] mempool: try to get objects from cache when the mempool is single consumer and multiple producer

[Ananyev, Konstantin]

Hi,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wenfeng Liu
> Sent: Monday, January 9, 2017 10:25 AM
> To: olivier.matz@6wind.com
> Cc: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] mempool: try to get objects from cache when the mempool is single consumer and multiple producer
> 
> We put objects to cache when the mempool is multiple producer, however the cache will not be used when it is single consumer.
> With this patch we can get objects from cache when the single consumer is happen to be one of the producers, and this improves
> performance.
> 
> Signed-off-by: Wenfeng Liu <liuwf@arraynetworks.com.cn>
> ---
>  lib/librte_mempool/rte_mempool.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h
> index d315d42..4ab5a95 100644
> --- a/lib/librte_mempool/rte_mempool.h
> +++ b/lib/librte_mempool/rte_mempool.h
> @@ -1250,8 +1250,9 @@ __mempool_generic_get(struct rte_mempool *mp, void **obj_table,
>  	uint32_t index, len;
>  	void **cache_objs;
> 
> -	/* No cache provided or single consumer */
> -	if (unlikely(cache == NULL || flags & MEMPOOL_F_SC_GET ||
> +	/* No cache provided or single consumer and single producer */
> +	if (unlikely(cache == NULL ||
> +		     (flags & MEMPOOL_F_SC_GET) && (flags & MEMPOOL_F_SP_PUT) ||


I suppose that's a good thing to do...
Might be go one step further and don't check flags at all?
if (unlikely(cache == NULL || n >= cache->size)
   goto ring_dequeue;
If people don't want to have a mempool with cache,
they can just specify it at mempool creation time.
Again cache might improve performance even for SC|SP case.
Konstantin   

>  		     n >= cache->size))
>  		goto ring_dequeue;
> 
> --
> 2.7.4

[PATCH v2] mempool: don't check mempool flags when cache is enabled

[Wenfeng Liu]

Currently we will check mempool flags when we put/get objects from
mempool. However, this makes cache useless when mempool is SC|SP,
SC|MP, MC|SP cases.
This patch makes cache available in above cases and improves performance.
---
 lib/librte_mempool/rte_mempool.h | 25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h
index d315d42..aca2f1b 100644
--- a/lib/librte_mempool/rte_mempool.h
+++ b/lib/librte_mempool/rte_mempool.h
@@ -1038,19 +1038,15 @@ static inline struct rte_mempool_cache *__attribute__((always_inline))
  */
 static inline void __attribute__((always_inline))
 __mempool_generic_put(struct rte_mempool *mp, void * const *obj_table,
-		      unsigned n, struct rte_mempool_cache *cache, int flags)
+		      unsigned n, struct rte_mempool_cache *cache)
 {
 	void **cache_objs;
 
 	/* increment stat now, adding in mempool always success */
 	__MEMPOOL_STAT_ADD(mp, put, n);
 
-	/* No cache provided or single producer */
-	if (unlikely(cache == NULL || flags & MEMPOOL_F_SP_PUT))
-		goto ring_enqueue;
-
-	/* Go straight to ring if put would overflow mem allocated for cache */
-	if (unlikely(n > RTE_MEMPOOL_CACHE_MAX_SIZE))
+	/* No cache provided or if put would overflow mem allocated for cache */
+	if (unlikely(cache == NULL || n > RTE_MEMPOOL_CACHE_MAX_SIZE))
 		goto ring_enqueue;
 
 	cache_objs = &cache->objs[cache->len];
@@ -1104,10 +1100,10 @@ static inline void __attribute__((always_inline))
  */
 static inline void __attribute__((always_inline))
 rte_mempool_generic_put(struct rte_mempool *mp, void * const *obj_table,
-			unsigned n, struct rte_mempool_cache *cache, int flags)
+			unsigned n, struct rte_mempool_cache *cache, __rte_unused int flags)
 {
 	__mempool_check_cookies(mp, obj_table, n, 0);
-	__mempool_generic_put(mp, obj_table, n, cache, flags);
+	__mempool_generic_put(mp, obj_table, n, cache);
 }
 
 /**
@@ -1244,15 +1240,14 @@ static inline void __attribute__((always_inline))
  */
 static inline int __attribute__((always_inline))
 __mempool_generic_get(struct rte_mempool *mp, void **obj_table,
-		      unsigned n, struct rte_mempool_cache *cache, int flags)
+		      unsigned n, struct rte_mempool_cache *cache)
 {
 	int ret;
 	uint32_t index, len;
 	void **cache_objs;
 
-	/* No cache provided or single consumer */
-	if (unlikely(cache == NULL || flags & MEMPOOL_F_SC_GET ||
-		     n >= cache->size))
+	/* No cache provided or cannot be satisfied from cache */
+	if (unlikely(cache == NULL || n >= cache->size))
 		goto ring_dequeue;
 
 	cache_objs = cache->objs;
@@ -1326,10 +1321,10 @@ static inline int __attribute__((always_inline))
  */
 static inline int __attribute__((always_inline))
 rte_mempool_generic_get(struct rte_mempool *mp, void **obj_table, unsigned n,
-			struct rte_mempool_cache *cache, int flags)
+			struct rte_mempool_cache *cache, __rte_unused int flags)
 {
 	int ret;
-	ret = __mempool_generic_get(mp, obj_table, n, cache, flags);
+	ret = __mempool_generic_get(mp, obj_table, n, cache);
 	if (ret == 0)
 		__mempool_check_cookies(mp, obj_table, n, 1);
 	return ret;
-- 
1.8.3.1

[PATCH v3] mempool: don't check mempool flags when cache is enabled

[Wenfeng Liu]

Currently we will check mempool flags when we put/get objects from
mempool. However, this makes cache useless when mempool is SC|SP,
SC|MP, MC|SP cases.
This patch makes cache available in above cases and improves performance.

Signed-off-by: Wenfeng Liu <liuwf@arraynetworks.com.cn>
---
 lib/librte_mempool/rte_mempool.h | 25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h
index d315d42..aca2f1b 100644
--- a/lib/librte_mempool/rte_mempool.h
+++ b/lib/librte_mempool/rte_mempool.h
@@ -1038,19 +1038,15 @@ static inline struct rte_mempool_cache *__attribute__((always_inline))
  */
 static inline void __attribute__((always_inline))
 __mempool_generic_put(struct rte_mempool *mp, void * const *obj_table,
-		      unsigned n, struct rte_mempool_cache *cache, int flags)
+		      unsigned n, struct rte_mempool_cache *cache)
 {
 	void **cache_objs;
 
 	/* increment stat now, adding in mempool always success */
 	__MEMPOOL_STAT_ADD(mp, put, n);
 
-	/* No cache provided or single producer */
-	if (unlikely(cache == NULL || flags & MEMPOOL_F_SP_PUT))
-		goto ring_enqueue;
-
-	/* Go straight to ring if put would overflow mem allocated for cache */
-	if (unlikely(n > RTE_MEMPOOL_CACHE_MAX_SIZE))
+	/* No cache provided or if put would overflow mem allocated for cache */
+	if (unlikely(cache == NULL || n > RTE_MEMPOOL_CACHE_MAX_SIZE))
 		goto ring_enqueue;
 
 	cache_objs = &cache->objs[cache->len];
@@ -1104,10 +1100,10 @@ static inline void __attribute__((always_inline))
  */
 static inline void __attribute__((always_inline))
 rte_mempool_generic_put(struct rte_mempool *mp, void * const *obj_table,
-			unsigned n, struct rte_mempool_cache *cache, int flags)
+			unsigned n, struct rte_mempool_cache *cache, __rte_unused int flags)
 {
 	__mempool_check_cookies(mp, obj_table, n, 0);
-	__mempool_generic_put(mp, obj_table, n, cache, flags);
+	__mempool_generic_put(mp, obj_table, n, cache);
 }
 
 /**
@@ -1244,15 +1240,14 @@ static inline void __attribute__((always_inline))
  */
 static inline int __attribute__((always_inline))
 __mempool_generic_get(struct rte_mempool *mp, void **obj_table,
-		      unsigned n, struct rte_mempool_cache *cache, int flags)
+		      unsigned n, struct rte_mempool_cache *cache)
 {
 	int ret;
 	uint32_t index, len;
 	void **cache_objs;
 
-	/* No cache provided or single consumer */
-	if (unlikely(cache == NULL || flags & MEMPOOL_F_SC_GET ||
-		     n >= cache->size))
+	/* No cache provided or cannot be satisfied from cache */
+	if (unlikely(cache == NULL || n >= cache->size))
 		goto ring_dequeue;
 
 	cache_objs = cache->objs;
@@ -1326,10 +1321,10 @@ static inline int __attribute__((always_inline))
  */
 static inline int __attribute__((always_inline))
 rte_mempool_generic_get(struct rte_mempool *mp, void **obj_table, unsigned n,
-			struct rte_mempool_cache *cache, int flags)
+			struct rte_mempool_cache *cache, __rte_unused int flags)
 {
 	int ret;
-	ret = __mempool_generic_get(mp, obj_table, n, cache, flags);
+	ret = __mempool_generic_get(mp, obj_table, n, cache);
 	if (ret == 0)
 		__mempool_check_cookies(mp, obj_table, n, 1);
 	return ret;
-- 
1.8.3.1

Re: [PATCH v3] mempool: don't check mempool flags when cache is enabled

[Olivier MATZ]

Hi Wengfeng,

On Tue, 10 Jan 2017 08:26:42 +0000, Wenfeng Liu
<liuwf@arraynetworks.com.cn> wrote:
> Currently we will check mempool flags when we put/get objects from
> mempool. However, this makes cache useless when mempool is SC|SP,
> SC|MP, MC|SP cases.
> This patch makes cache available in above cases and improves
> performance.
> 
> Signed-off-by: Wenfeng Liu <liuwf@arraynetworks.com.cn>

I agree with you and Konstantin. This should enhance performance in
single consumer/producer mode.

> @@ -1104,10 +1100,10 @@ static inline void
> __attribute__((always_inline)) */
>  static inline void __attribute__((always_inline))
>  rte_mempool_generic_put(struct rte_mempool *mp, void * const
> *obj_table,
> -			unsigned n, struct rte_mempool_cache *cache,
> int flags)
> +			unsigned n, struct rte_mempool_cache *cache,
> __rte_unused int flags) {

Small nit, seen with checkpatch:

WARNING:LONG_LINE: line over 80 characters
#43: FILE: lib/librte_mempool/rte_mempool.h:1103:
+                       unsigned n, struct rte_mempool_cache *cache,
  __rte_unused int flags)


The other warnings (Prefer 'unsigned int' to bare use of 'unsigned')
can be ignored, since it's not coming from your patch.


While there, I suggest another title that better reflects what is done:
"mempool: use cache in single producer or consumer mode"


Thanks,
Olivier

[PATCH v4] mempool: use cache in single producer or consumer mode

[Wenfeng Liu]

Currently we will check mempool flags when we put/get objects from
mempool. However, this makes cache useless when mempool is SC|SP,
SC|MP, MC|SP cases.
This patch makes cache available in above cases and improves performance.

Signed-off-by: Wenfeng Liu <liuwf@arraynetworks.com.cn>
---
 lib/librte_mempool/rte_mempool.h | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h
index d315d42..d0f5b27 100644
--- a/lib/librte_mempool/rte_mempool.h
+++ b/lib/librte_mempool/rte_mempool.h
@@ -1038,19 +1038,15 @@ static inline struct rte_mempool_cache *__attribute__((always_inline))
  */
 static inline void __attribute__((always_inline))
 __mempool_generic_put(struct rte_mempool *mp, void * const *obj_table,
-		      unsigned n, struct rte_mempool_cache *cache, int flags)
+		      unsigned n, struct rte_mempool_cache *cache)
 {
 	void **cache_objs;
 
 	/* increment stat now, adding in mempool always success */
 	__MEMPOOL_STAT_ADD(mp, put, n);
 
-	/* No cache provided or single producer */
-	if (unlikely(cache == NULL || flags & MEMPOOL_F_SP_PUT))
-		goto ring_enqueue;
-
-	/* Go straight to ring if put would overflow mem allocated for cache */
-	if (unlikely(n > RTE_MEMPOOL_CACHE_MAX_SIZE))
+	/* No cache provided or if put would overflow mem allocated for cache */
+	if (unlikely(cache == NULL || n > RTE_MEMPOOL_CACHE_MAX_SIZE))
 		goto ring_enqueue;
 
 	cache_objs = &cache->objs[cache->len];
@@ -1104,10 +1100,11 @@ static inline void __attribute__((always_inline))
  */
 static inline void __attribute__((always_inline))
 rte_mempool_generic_put(struct rte_mempool *mp, void * const *obj_table,
-			unsigned n, struct rte_mempool_cache *cache, int flags)
+			unsigned n, struct rte_mempool_cache *cache,
+			__rte_unused int flags)
 {
 	__mempool_check_cookies(mp, obj_table, n, 0);
-	__mempool_generic_put(mp, obj_table, n, cache, flags);
+	__mempool_generic_put(mp, obj_table, n, cache);
 }
 
 /**
@@ -1244,15 +1241,14 @@ static inline void __attribute__((always_inline))
  */
 static inline int __attribute__((always_inline))
 __mempool_generic_get(struct rte_mempool *mp, void **obj_table,
-		      unsigned n, struct rte_mempool_cache *cache, int flags)
+		      unsigned n, struct rte_mempool_cache *cache)
 {
 	int ret;
 	uint32_t index, len;
 	void **cache_objs;
 
-	/* No cache provided or single consumer */
-	if (unlikely(cache == NULL || flags & MEMPOOL_F_SC_GET ||
-		     n >= cache->size))
+	/* No cache provided or cannot be satisfied from cache */
+	if (unlikely(cache == NULL || n >= cache->size))
 		goto ring_dequeue;
 
 	cache_objs = cache->objs;
@@ -1326,10 +1322,10 @@ static inline int __attribute__((always_inline))
  */
 static inline int __attribute__((always_inline))
 rte_mempool_generic_get(struct rte_mempool *mp, void **obj_table, unsigned n,
-			struct rte_mempool_cache *cache, int flags)
+			struct rte_mempool_cache *cache, __rte_unused int flags)
 {
 	int ret;
-	ret = __mempool_generic_get(mp, obj_table, n, cache, flags);
+	ret = __mempool_generic_get(mp, obj_table, n, cache);
 	if (ret == 0)
 		__mempool_check_cookies(mp, obj_table, n, 1);
 	return ret;
-- 
1.8.3.1

Re: [PATCH v4] mempool: use cache in single producer or consumer mode

[Olivier Matz]

On Wed, 11 Jan 2017 02:25:28 +0000, Wenfeng Liu
<liuwf@arraynetworks.com.cn> wrote:
> Currently we will check mempool flags when we put/get objects from
> mempool. However, this makes cache useless when mempool is SC|SP,
> SC|MP, MC|SP cases.
> This patch makes cache available in above cases and improves
> performance.
> 
> Signed-off-by: Wenfeng Liu <liuwf@arraynetworks.com.cn>

Acked-by: Olivier Matz <olivier.matz@6wind.com>

Re: [PATCH v4] mempool: use cache in single producer or consumer mode

[Thomas Monjalon]

2017-01-13 16:23, Olivier Matz:
> On Wed, 11 Jan 2017 02:25:28 +0000, Wenfeng Liu
> <liuwf@arraynetworks.com.cn> wrote:
> > Currently we will check mempool flags when we put/get objects from
> > mempool. However, this makes cache useless when mempool is SC|SP,
> > SC|MP, MC|SP cases.
> > This patch makes cache available in above cases and improves
> > performance.
> > 
> > Signed-off-by: Wenfeng Liu <liuwf@arraynetworks.com.cn>
> 
> Acked-by: Olivier Matz <olivier.matz@6wind.com>

Applied, thanks