> From: Stephen Hemminger [mailto:step...@networkplumber.org] > Sent: Tuesday, 26 August 2025 15.42 > > On Tue, 26 Aug 2025 08:55:23 +0200 > Morten Brørup <m...@smartsharesystems.com> wrote: > > > > +static int > > > +rte_hash_k64_cmp_eq(const void *key1, const void *key2, size_t > key_len) > > > +{ > > > + return rte_hash_k32_cmp_eq(key1, key2, key_len) | > > > > Is the "|" instead of "||", to compare in blocks of 64 bytes instead > of 32, intentional? > > The cost of the conditional branch is usually higher than the cost of > doing > a few more instructions on cached data.
I agree the key being looked up is very likely cached. But the key in the hash table might not be, and the 64 byte comparison might cross a cache line. I think using half a cache line as the breakpoint (for using conditional branch instead of unconditional load and compare) seems like a better tradeoff than a full cache line. I have no data to back this up, just a hunch. In reality, it depends on the use case. If a cache hit is more likely than a cache miss, then the unconditional comparison is preferable. No strong opinion. I mainly wanted to ensure this was intentional. While looking into this, I noticed a few instances where your assumption about key1 being aligned is wrong, and the key1/key2 parameters should be swapped: https://elixir.bootlin.com/dpdk/v25.07/source/lib/hash/rte_cuckoo_hash.c#L763 https://elixir.bootlin.com/dpdk/v25.07/source/lib/hash/rte_cuckoo_hash.c#L1319 https://elixir.bootlin.com/dpdk/v25.07/source/lib/hash/rte_cuckoo_hash.c#L1357 E.g. when calling rte_hash_add_key_with_hash(), the key parameter can be unaligned, and it ripples down and becomes key1: https://elixir.bootlin.com/dpdk/v25.07/source/lib/hash/rte_cuckoo_hash.c#L1259
