<snip>

> 
> This patch adds a comment for RTE_HASH_BUCKET_ENTRIES explaining why a
> particular value was chosen.
> 
> Signed-off-by: Vladimir Medvedkin <vladimir.medved...@intel.com>
> ---
>  lib/hash/rte_cuckoo_hash.h | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/hash/rte_cuckoo_hash.h b/lib/hash/rte_cuckoo_hash.h index
> 85be49d3bb..84dc55d86e 100644
> --- a/lib/hash/rte_cuckoo_hash.h
> +++ b/lib/hash/rte_cuckoo_hash.h
> @@ -101,7 +101,13 @@ const rte_hash_cmp_eq_t
> cmp_jump_table[NUM_KEY_CMP_CASES] = {  #endif
> 
> 
> -/** Number of items per bucket. */
> +/**
> + * Number of items per bucket.
> + * 8 is a tradeoff between performance and memory consumption.
> + * When it is equal to 8, the sizeof(struct rte_hash_bucket) equal to
> + * RTE_CACHE_LINE_SIZE, thus, there are no gaps in memory between the
> +hash
> + * buckets due to their alignment.
> + */
I think this should consider cache lines which are 128B. How about the 
following:
"when it is equal to 8, multiple 'struct rte_hash_bucket' can be fit on a 
single cache line without any gaps in memory between them".

On the other hand, I am wondering if 'struct rte_hash_bucket' needs to have 
__rte_cache_aligned attribute. When the memory is allocated for the buckets we 
are requesting that it is aligned on the cache line boundary. That should be 
sufficient. Removing the attribute will help for local variables. Some 
functions (for ex: rte_hash_cuckoo_move_insert_mw) have 2 local variables of 
this type and they can be placed on the same cache line if this attribute is 
removed.

>  #define RTE_HASH_BUCKET_ENTRIES              8
> 
>  #if !RTE_IS_POWER_OF_2(RTE_HASH_BUCKET_ENTRIES)
> --
> 2.25.1

Reply via email to