[v3,1/5] test/hash: fix bucket size in hash perf test

Message ID 1537966465-248530-2-git-send-email-yipeng1.wang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series hash: fix multiple issues |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Wang, Yipeng1 Sept. 26, 2018, 12:54 p.m. UTC
  The bucket size was changed from 4 to 8 but the corresponding
perf test was not changed accordingly.

In the test, the bucket size and number of buckets are used
to map to the underneath rte_hash structure. They are used
to test performance of two conditions: keys in primary
buckets only and keys in both primary and secondary buckets.

Although there is no functional issue with bucket size set
to 4, it mismatches the underneath rte_hash structure,
which may affect code readability and future extension.

Fixes: 58017c98ed53 ("hash: add vectorized comparison")
Cc: stable@dpdk.org

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
---
 test/test/test_hash_perf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Bruce Richardson Sept. 27, 2018, 11:17 a.m. UTC | #1
On Wed, Sep 26, 2018 at 05:54:21AM -0700, Yipeng Wang wrote:
> The bucket size was changed from 4 to 8 but the corresponding
> perf test was not changed accordingly.
> 
> In the test, the bucket size and number of buckets are used
> to map to the underneath rte_hash structure. They are used
> to test performance of two conditions: keys in primary
> buckets only and keys in both primary and secondary buckets.
> 
> Although there is no functional issue with bucket size set
> to 4, it mismatches the underneath rte_hash structure,
> which may affect code readability and future extension.
> 
> Fixes: 58017c98ed53 ("hash: add vectorized comparison")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
> ---
>  test/test/test_hash_perf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/test/test/test_hash_perf.c b/test/test/test_hash_perf.c
> index 33dcb9f..9ed7125 100644
> --- a/test/test/test_hash_perf.c
> +++ b/test/test/test_hash_perf.c
> @@ -20,7 +20,7 @@
>  #define MAX_ENTRIES (1 << 19)
>  #define KEYS_TO_ADD (MAX_ENTRIES * 3 / 4) /* 75% table utilization */
>  #define NUM_LOOKUPS (KEYS_TO_ADD * 5) /* Loop among keys added, several times */
> -#define BUCKET_SIZE 4
> +#define BUCKET_SIZE 8
>  #define NUM_BUCKETS (MAX_ENTRIES / BUCKET_SIZE)
>  #define MAX_KEYSIZE 64
>  #define NUM_KEYSIZES 10

Honnappa's suggestion that a comment be added here to indicate that the
value should be kept in sync with the rte_hash one is a good one, and
should be done.

Otherwise:

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> -- 
> 2.7.4
>
  

Patch

diff --git a/test/test/test_hash_perf.c b/test/test/test_hash_perf.c
index 33dcb9f..9ed7125 100644
--- a/test/test/test_hash_perf.c
+++ b/test/test/test_hash_perf.c
@@ -20,7 +20,7 @@ 
 #define MAX_ENTRIES (1 << 19)
 #define KEYS_TO_ADD (MAX_ENTRIES * 3 / 4) /* 75% table utilization */
 #define NUM_LOOKUPS (KEYS_TO_ADD * 5) /* Loop among keys added, several times */
-#define BUCKET_SIZE 4
+#define BUCKET_SIZE 8
 #define NUM_BUCKETS (MAX_ENTRIES / BUCKET_SIZE)
 #define MAX_KEYSIZE 64
 #define NUM_KEYSIZES 10