[dpdk-dev] [PATCH 3/3] lib/hash: adjust tbl_chng_cnt position

Wang, Yipeng1 yipeng1.wang at intel.com
Thu Jul 4 02:52:35 CEST 2019


>-----Original Message-----
>From: Honnappa Nagarahalli [mailto:Honnappa.Nagarahalli at arm.com]
>Sent: Tuesday, July 2, 2019 10:23 AM
>To: Wang, Yipeng1 <yipeng1.wang at intel.com>; Gobriel, Sameh <sameh.gobriel at intel.com>; Richardson, Bruce
><bruce.richardson at intel.com>; De Lara Guarch, Pablo <pablo.de.lara.guarch at intel.com>
>Cc: Gavin Hu (Arm Technology China) <Gavin.Hu at arm.com>; Ruifeng Wang (Arm Technology China) <Ruifeng.Wang at arm.com>;
>dev at dpdk.org; Honnappa Nagarahalli <Honnappa.Nagarahalli at arm.com>; nd <nd at arm.com>; stable at dpdk.org; nd <nd at arm.com>
>Subject: RE: [PATCH 3/3] lib/hash: adjust tbl_chng_cnt position
>
><snip>
>
>> >
>> >tbl_chng_cnt is one of the first elements of the structure used in the
>> >lookup. Move it to the beginning of the cache line to gain performance.
>> >
>> >Fixes: e605a1d36 ("hash: add lock-free r/w concurrency")
>> >Cc: stable at dpdk.org
>> >
>> >Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli at arm.com>
>> >Reviewed-by: Gavin Hu <gavin.hu at arm.com>
>> >Tested-by: Ruifeng Wang <ruifeng.wang at arm.com>
>> >---
>> > lib/librte_hash/rte_cuckoo_hash.h | 6 +++---
>> > 1 file changed, 3 insertions(+), 3 deletions(-)
>> >
>> >diff --git a/lib/librte_hash/rte_cuckoo_hash.h
>> >b/lib/librte_hash/rte_cuckoo_hash.h
>> >index fb19bb27d..af6451b5c 100644
>> >--- a/lib/librte_hash/rte_cuckoo_hash.h
>> >+++ b/lib/librte_hash/rte_cuckoo_hash.h
>> >@@ -170,7 +170,9 @@ struct rte_hash {
>> >
>> > 	/* Fields used in lookup */
>> >
>> >-	uint32_t key_len __rte_cache_aligned;
>> >+	uint32_t *tbl_chng_cnt __rte_cache_aligned;
>> >+	/**< Indicates if the hash table changed from last read. */
>> >+	uint32_t key_len;
>> > 	/**< Length of hash key. */
>> > 	uint8_t hw_trans_mem_support;
>> > 	/**< If hardware transactional memory is used. */ @@ -218,8 +220,6
>> @@
>> >struct rte_hash {
>> > 	 * is piggy-backed to freeing of the key index.
>> > 	 */
>> > 	uint32_t *ext_bkt_to_free;
>> >-	uint32_t *tbl_chng_cnt;
>> >-	/**< Indicates if the hash table changed from last read. */
>> > } __rte_cache_aligned;
>> >
>> > struct queue_node {
>> >--
>> >2.17.1
>>
>> [Wang, Yipeng]
>> I am not sure about this change. By moving counter to front, I think you
>> seems push key_store out of the cache line. And key_store Is also used in
>> lookup (and more commonly).
>> My tests also show perf drop in many cases.
>I ran hash_readwrite_lf tests and L3 fwd application. Both of them showed improvements for both lock-free and using locks for Arm
>platforms (L3 fwd was not run on x86). Which tests are resulting in performance drops for you?
>
>But, I do agree that this work is not complete. We can drop this patch and take this up separately.
[Wang, Yipeng] 
I run the LF test on x86. For 1 reader case it seems a clear improvement for lookup without key-shifts. Other results are a mixed bag even
for lock-free. For most HTM it is a drop.

My opinion is to delay similar fine tuning effort tied to architecture specs. 


More information about the dev mailing list