<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". > > > > Sounds good, will add it in v2. > > > 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. > > > > I see, however I can't find inside the rte_hash_cuckoo_move_insert_mw() > 'struct rte_hash_bucket' as local variables, there are only pointers: Agree, it is all pointers.
> > struct rte_hash_bucket *cur_bkt; > struct rte_hash_bucket *prev_bkt, *curr_bkt = leaf->bkt; > > Yipeng, Sameh, Bruce, what do you think about removing __rte_cache_aligned > attribute? > > >> #define RTE_HASH_BUCKET_ENTRIES 8 > >> > >> #if !RTE_IS_POWER_OF_2(RTE_HASH_BUCKET_ENTRIES) > >> -- > >> 2.25.1 > > > > -- > Regards, > Vladimir