From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 8F4CB5A52 for ; Wed, 8 Jul 2015 17:05:16 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga102.fm.intel.com with ESMTP; 08 Jul 2015 08:04:19 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.15,431,1432623600"; d="scan'208";a="743024617" Received: from bricha3-mobl3.ger.corp.intel.com ([10.237.208.63]) by fmsmga001.fm.intel.com with SMTP; 08 Jul 2015 08:04:18 -0700 Received: by (sSMTP sendmail emulation); Wed, 08 Jul 2015 16:04:17 +0025 Date: Wed, 8 Jul 2015 16:04:17 +0100 From: Bruce Richardson To: Pablo de Lara Message-ID: <20150708150417.GB2676@bricha3-MOBL3> References: <1436361126-22178-1-git-send-email-pablo.de.lara.guarch@intel.com> <1436361126-22178-2-git-send-email-pablo.de.lara.guarch@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1436361126-22178-2-git-send-email-pablo.de.lara.guarch@intel.com> Organization: Intel Shannon Ltd. User-Agent: Mutt/1.5.23 (2014-03-12) Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH] test/hash: improve hash unit tests 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: Wed, 08 Jul 2015 15:05:17 -0000 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 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(); >