-----Original Message-----
From: Richardson, Bruce
Sent: Wednesday, September 28, 2016 2:05 AM
To: De Lara Guarch, Pablo
Cc: dev@dpdk.org; Marohn, Byron; Edupuganti, Saikrishna
Subject: Re: [PATCH v3 2/4] hash: reorganize bucket structure

On Tue, Sep 06, 2016 at 08:34:02PM +0100, Pablo de Lara wrote:

quoted

From: Byron Marohn <redacted>

Move current signatures of all entries together in the bucket
and same with all alternative signatures, instead of having
current and alternative signatures together per entry in the bucket.
This will be benefitial in the next commits, where a vectorized
comparison will be performed, achieving better performance.

Signed-off-by: Byron Marohn <redacted>
Signed-off-by: Saikrishna Edupuganti <redacted>
---
 lib/librte_hash/rte_cuckoo_hash.c     | 43 ++++++++++++++++++----------------

-

quoted

 lib/librte_hash/rte_cuckoo_hash.h     | 17 ++++----------
 lib/librte_hash/rte_cuckoo_hash_x86.h | 20 ++++++++--------
 3 files changed, 37 insertions(+), 43 deletions(-)

<snip>

quoted

--- a/lib/librte_hash/rte_cuckoo_hash.h
+++ b/lib/librte_hash/rte_cuckoo_hash.h

@@ -151,17 +151,6 @@ struct lcore_cache {
 	void *objs[LCORE_CACHE_SIZE]; /**< Cache objects */
 } __rte_cache_aligned;

-/* Structure storing both primary and secondary hashes */
-struct rte_hash_signatures {
-	union {
-		struct {
-			hash_sig_t current;
-			hash_sig_t alt;
-		};
-		uint64_t sig;
-	};
-};
-
 /* Structure that stores key-value pair */
 struct rte_hash_key {
 	union {

@@ -174,10 +163,14 @@ struct rte_hash_key {

 /** Bucket structure */
 struct rte_hash_bucket {
-	struct rte_hash_signatures signatures[RTE_HASH_BUCKET_ENTRIES];
+	hash_sig_t sig_current[RTE_HASH_BUCKET_ENTRIES];
+
 	/* Includes dummy key index that always contains index 0 */
 	uint32_t key_idx[RTE_HASH_BUCKET_ENTRIES + 1];
+
 	uint8_t flag[RTE_HASH_BUCKET_ENTRIES];
+
+	hash_sig_t sig_alt[RTE_HASH_BUCKET_ENTRIES];
 } __rte_cache_aligned;

Is there a reason why sig_current and sig_alt fields cannot be beside each
other in the structure. It looks strange having them separate by other fields?

Bucket entries has increased to 8 now, so sig_current and key_idx take 64 bytes
(key_idx will be reduced to 8 entries in the fourth patch).

Therefore, the idea was to push sig_alt to the next cache line
(assuming a 64 byte cacheline, if it is bigger, then either way is ok),
as it is not used in lookup (like sig_current and key_idx).

Anyway, I think I will move sig_alt before flag, so it is cache aligned,
in case vectorization is used in the future.

Thanks,
Pablo

/Bruce