On Wed, Jul 08, 2015 at 02:12:06PM +0100, Pablo de Lara wrote:
> Add new unit test for calculating the average table utilization,
> using random keys, based on number of entries that can be added
> until we encounter one that cannot be added (bucket if full)
>
> Also, replace current hash_perf unit test to see performance more clear.
> The current hash_perf unit test takes too long and add keys that
> may or may not fit in the table and look up/delete that may not be
> in the table. This new unit test gets a set of keys that we know
> that fits in the table, and then measure the time to add/look up/delete
> them.
>
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch at intel.com>
Few more comments on the change to test_hash.c
/Bruce
> ---
> app/test/test_hash.c | 61 ++++
> app/test/test_hash_perf.c | 906
> +++++++++++++++++++---------------------------
> 2 files changed, 439 insertions(+), 528 deletions(-)
>
> diff --git a/app/test/test_hash.c b/app/test/test_hash.c
> index 4300de9..4d538b2 100644
> --- a/app/test/test_hash.c
> +++ b/app/test/test_hash.c
> @@ -1147,6 +1147,65 @@ test_hash_creation_with_good_parameters(void)
> return 0;
> }
>
> +#define ITERATIONS 50
> +/*
> + * Test to see the average table utilization (entries added/max entries)
> + * before hitting a random entry that cannot be added
> + */
> +static int test_average_table_utilization(void)
> +{
> + struct rte_hash *handle;
> + void *simple_key;
> + unsigned i, j, no_space = 0;
> + double added_keys_until_no_space = 0;
> + int ret;
> +
> + ut_params.entries = 1 << 20;
> + ut_params.name = "test_average_utilization";
> + ut_params.hash_func = rte_jhash;
> + handle = rte_hash_create(&ut_params);
> + RETURN_IF_ERROR(handle == NULL, "hash creation failed");
> +
> + simple_key = rte_zmalloc(NULL, ut_params.key_len, 0);
> +
> + for (j = 0; j < ITERATIONS; j++) {
> + while (!no_space) {
> + for (i = 0; i < ut_params.key_len; i++)
> + ((uint8_t *) simple_key)[i] = rte_rand() % 255;
> +
> + ret = rte_hash_add_key(handle, simple_key);
> + print_key_info("Add", simple_key, ret);
> +
> + if (ret == -ENOSPC) {
> + if (rte_hash_lookup(handle, simple_key) !=
> -ENOENT)
> + printf("Found key that should not be
> present\n");
Should this not be an immediate test failure?
In fact, is it really worth testing, for this condition. Why not just have
the loop and test as:
do {
/*set up simple key */
} while ((ret = rte_hash_add_key(...)) >= 0);
if (ret != -ENOSPC) {
/* print error */
return -1;
}
> + no_space = 1;
> + } else {
> + if (ret < 0)
> + rte_free(simple_key);
Rather than using malloc free, why not just make simple_key a local array of
size MAX_KEY_SIZE.
> + RETURN_IF_ERROR(ret < 0,
> + "failed to add key (ret=%d)",
> ret);
> + added_keys_until_no_space++;
> + }
> + }
> + no_space = 0;
> +
> + /* Reset the table */
> + rte_hash_free(handle);
> + handle = rte_hash_create(&ut_params);
> + RETURN_IF_ERROR(handle == NULL, "hash creation failed");
Would a reset call work better than a free/recreate?
> + }
> +
> + const unsigned average_keys_added = added_keys_until_no_space /
> ITERATIONS;
> +
> + printf("Average table utilization = %.2f%% (%u/%u)\n",
> + ((double) average_keys_added / ut_params.entries * 100),
> + average_keys_added, ut_params.entries);
> + rte_hash_free(handle);
> +
> + return 0;
> +}
> +
> static uint8_t key[16] = {0x00, 0x01, 0x02, 0x03,
> 0x04, 0x05, 0x06, 0x07,
> 0x08, 0x09, 0x0a, 0x0b,
> @@ -1405,6 +1464,8 @@ test_hash(void)
> return -1;
> if (test_hash_creation_with_good_parameters() < 0)
> return -1;
> + if (test_average_table_utilization() < 0)
> + return -1;
>
> run_hash_func_tests();
>
