From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id F246937B4 for ; Thu, 29 Sep 2016 03:33:14 +0200 (CEST) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga104.fm.intel.com with ESMTP; 28 Sep 2016 18:33:14 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,412,1470726000"; d="scan'208";a="14359567" Received: from irsmsx104.ger.corp.intel.com ([163.33.3.159]) by fmsmga005.fm.intel.com with ESMTP; 28 Sep 2016 18:33:13 -0700 Received: from irsmsx108.ger.corp.intel.com ([169.254.11.164]) by IRSMSX104.ger.corp.intel.com ([163.33.3.159]) with mapi id 14.03.0248.002; Thu, 29 Sep 2016 02:33:12 +0100 From: "De Lara Guarch, Pablo" To: "Richardson, Bruce" CC: "dev@dpdk.org" Thread-Topic: [PATCH v3 1/4] hash: reorder hash structure Thread-Index: AQHSCHWUocznqHs650Sb5eURQRPPfqCOrHIAgAEkYaA= Date: Thu, 29 Sep 2016 01:33:12 +0000 Message-ID: References: <1472856999-31028-1-git-send-email-pablo.de.lara.guarch@intel.com> <1473190444-120795-1-git-send-email-pablo.de.lara.guarch@intel.com> <1473190444-120795-2-git-send-email-pablo.de.lara.guarch@intel.com> <20160928090240.GA47356@bricha3-MOBL3> In-Reply-To: <20160928090240.GA47356@bricha3-MOBL3> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMGQ2NTlhYmQtZWNmYy00OGE4LTgzM2EtYTA4NGU5ZmVhMmVlIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6ImNNeWl3ZHBNTG5jOFh4Y2pcL1NPeUxjdTZLQnprZkprOCtiNUVFSUJhRlU0PSJ9 x-ctpclassification: CTP_IC x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v3 1/4] hash: reorder hash structure X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 29 Sep 2016 01:33:15 -0000 > -----Original Message----- > From: Richardson, Bruce > Sent: Wednesday, September 28, 2016 2:03 AM > To: De Lara Guarch, Pablo > Cc: dev@dpdk.org > Subject: Re: [PATCH v3 1/4] hash: reorder hash structure >=20 > On Tue, Sep 06, 2016 at 08:34:01PM +0100, Pablo de Lara wrote: > > In order to optimize lookup performance, hash structure > > is reordered, so all fields used for lookup will be > > in the first cache line. > > > > Signed-off-by: Pablo de Lara > > --- > > lib/librte_hash/rte_cuckoo_hash.h | 12 +++++++----- > > 1 file changed, 7 insertions(+), 5 deletions(-) > > > > diff --git a/lib/librte_hash/rte_cuckoo_hash.h > b/lib/librte_hash/rte_cuckoo_hash.h > > index e290dab..701531a 100644 > > --- a/lib/librte_hash/rte_cuckoo_hash.h > > +++ b/lib/librte_hash/rte_cuckoo_hash.h > > @@ -182,9 +182,7 @@ struct rte_hash_bucket { > > > > /** A hash table structure. */ > > struct rte_hash { > > - char name[RTE_HASH_NAMESIZE]; /**< Name of the hash. */ > > - uint32_t entries; /**< Total table entries. */ > > - uint32_t num_buckets; /**< Number of buckets in table. */ > > + /* first cache line - fields used in lookup */ > > uint32_t key_len; /**< Length of hash key. */ > > rte_hash_function hash_func; /**< Function used to calculate hash. > */ > > uint32_t hash_func_init_val; /**< Init value used by hash_func. */ > > @@ -196,12 +194,13 @@ struct rte_hash { > > from hash signature. */ > > uint32_t key_entry_size; /**< Size of each key entry. */ > > > > - struct rte_ring *free_slots; /**< Ring that stores all indexes > > - of the free slots in the key > table */ > > void *key_store; /**< Table storing all keys and data = */ > > struct rte_hash_bucket *buckets; /**< Table with buckets > storing all the > > hash values and key > indexes > > to the key table*/ > > + > > + struct rte_ring *free_slots; /**< Ring that stores all indexes > > + of the free slots in the key > table */ > > uint8_t hw_trans_mem_support; /**< Hardware transactional > > memory support */ > > struct lcore_cache *local_free_slots; > > @@ -209,6 +208,9 @@ struct rte_hash { > > enum add_key_case add_key; /**< Multi-writer hash add behavior */ > > > > rte_spinlock_t *multiwriter_lock; /**< Multi-writer spinlock for w/o > TM */ > > + char name[RTE_HASH_NAMESIZE]; /**< Name of the hash. */ > > + uint32_t entries; /**< Total table entries. */ > > + uint32_t num_buckets; /**< Number of buckets in table. */ >=20 > Hi Pablo, >=20 > While I've no strong objection to the change, having the name at the star= t > is a common paradigm in DPDK. Rather than place these fields at the end, > can you get the same effect by just marking the key_len function > __rte_cache_aligned? It may use a little more memory per table, but given > that > the size of the hash table is going to be largely governed by the table d= ata, > I don't see an extra 64 bytes in the structure as being an issue. Hi Bruce, Sure, sounds good to me. I was trying a similar approach to the mbuf struct= ure, and actually I saw a small performance boost doing this, so I can push the fields needed for lookup for the second cache line instea= d, it should work the same :) Thanks, Pablo >=20 > Regards, > /Bruce