diff mbox series

[v3,2/2] test: update hash performance tests

Message ID 1586370739-61729-2-git-send-email-vladimir.medvedkin@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [v3,1/2] hash: add hash bulk lookup with hash signatures array |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/travis-robot success Travis build: passed
ci/Intel-compilation success Compilation OK

Commit Message

Vladimir Medvedkin April 8, 2020, 6:32 p.m. UTC 
  Add preformance test for rte_hash_lookup_with_hash_bulk_data()

Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
---
 app/test/test_hash_perf.c | 60 +++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 55 insertions(+), 5 deletions(-)

Comments

Thomas Monjalon April 16, 2020, 9:44 a.m. UTC | #1 
08/04/2020 20:32, Vladimir Medvedkin:
> Add preformance test for rte_hash_lookup_with_hash_bulk_data()

Why is it a separate patch?
To me, it is natural to add such test when adding a new hash API.
So they should be in the same patch.
Vladimir Medvedkin April 16, 2020, 2:47 p.m. UTC | #2 
Hi Thomas,

On 16/04/2020 10:44, Thomas Monjalon wrote:
> 08/04/2020 20:32, Vladimir Medvedkin:
>> Add preformance test for rte_hash_lookup_with_hash_bulk_data()
> Why is it a separate patch?
> To me, it is natural to add such test when adding a new hash API.
> So they should be in the same patch.
>

Here I split the patch to help review it - first lib part, second - test 
part. I can merge it if you think it should be.

>
Thomas Monjalon April 16, 2020, 2:50 p.m. UTC | #3 
16/04/2020 16:47, Medvedkin, Vladimir:
> Hi Thomas,
> 
> On 16/04/2020 10:44, Thomas Monjalon wrote:
> > 08/04/2020 20:32, Vladimir Medvedkin:
> >> Add preformance test for rte_hash_lookup_with_hash_bulk_data()
> > Why is it a separate patch?
> > To me, it is natural to add such test when adding a new hash API.
> > So they should be in the same patch.
> >
> 
> Here I split the patch to help review it - first lib part, second - test 
> part. I can merge it if you think it should be.

Yes please.

I don't think it helps reviewing because they are in different files anyway.
diff mbox series

Patch

diff --git a/app/test/test_hash_perf.c b/app/test/test_hash_perf.c
index a438eae..d88bfa9 100644
--- a/app/test/test_hash_perf.c
+++ b/app/test/test_hash_perf.c
@@ -391,8 +391,8 @@  timed_lookups(unsigned int with_hash, unsigned int with_data,
 }
 
 static int
-timed_lookups_multi(unsigned int with_data, unsigned int table_index,
-							unsigned int ext)
+timed_lookups_multi(unsigned int with_hash, unsigned int with_data,
+		unsigned int table_index, unsigned int ext)
 {
 	unsigned i, j, k;
 	int32_t positions_burst[BURST_SIZE];
@@ -417,7 +417,7 @@  timed_lookups_multi(unsigned int with_data, unsigned int table_index,
 		for (j = 0; j < keys_to_add/BURST_SIZE; j++) {
 			for (k = 0; k < BURST_SIZE; k++)
 				keys_burst[k] = keys[j * BURST_SIZE + k];
-			if (with_data) {
+			if (!with_hash && with_data) {
 				ret = rte_hash_lookup_bulk_data(h[table_index],
 					(const void **) keys_burst,
 					BURST_SIZE,
@@ -442,6 +442,54 @@  timed_lookups_multi(unsigned int with_data, unsigned int table_index,
 						return -1;
 					}
 				}
+			} else if (with_hash && with_data) {
+				ret = rte_hash_lookup_with_hash_bulk_data(
+					h[table_index],
+					(const void **)keys_burst,
+					&signatures[j * BURST_SIZE],
+					BURST_SIZE, &hit_mask, ret_data);
+				if (ret != BURST_SIZE) {
+					printf("Expect to find %u keys,"
+					       " but found %d\n",
+						BURST_SIZE, ret);
+					return -1;
+				}
+				for (k = 0; k < BURST_SIZE; k++) {
+					if ((hit_mask & (1ULL << k))  == 0) {
+						printf("Key number %u"
+							" not found\n",
+							j * BURST_SIZE + k);
+						return -1;
+					}
+					expected_data[k] =
+						(void *)((uintptr_t)signatures[
+						j * BURST_SIZE + k]);
+					if (ret_data[k] != expected_data[k]) {
+						printf("Data returned for key"
+							" number %u is %p,"
+							" but should be %p\n",
+							j * BURST_SIZE + k,
+							ret_data[k],
+							expected_data[k]);
+						return -1;
+					}
+				}
+			} else if (with_hash && !with_data) {
+				ret = rte_hash_lookup_with_hash_bulk(
+					h[table_index],
+					(const void **)keys_burst,
+					&signatures[j * BURST_SIZE],
+					BURST_SIZE, positions_burst);
+				for (k = 0; k < BURST_SIZE; k++) {
+					if (positions_burst[k] !=
+							positions[j *
+							BURST_SIZE + k]) {
+						printf("Key looked up in %d, should be in %d\n",
+							positions_burst[k],
+							positions[j * BURST_SIZE + k]);
+						return -1;
+					}
+				}
 			} else {
 				rte_hash_lookup_bulk(h[table_index],
 						(const void **) keys_burst,
@@ -462,7 +510,8 @@  timed_lookups_multi(unsigned int with_data, unsigned int table_index,
 	const uint64_t end_tsc = rte_rdtsc();
 	const uint64_t time_taken = end_tsc - start_tsc;
 
-	cycles[table_index][LOOKUP_MULTI][0][with_data] = time_taken/num_lookups;
+	cycles[table_index][LOOKUP_MULTI][with_hash][with_data] =
+		time_taken/num_lookups;
 
 	return 0;
 }
@@ -543,7 +592,8 @@  run_all_tbl_perf_tests(unsigned int with_pushes, unsigned int with_locks,
 				if (timed_lookups(with_hash, with_data, i, ext) < 0)
 					return -1;
 
-				if (timed_lookups_multi(with_data, i, ext) < 0)
+				if (timed_lookups_multi(with_hash, with_data,
+						i, ext) < 0)
 					return -1;
 
 				if (timed_deletes(with_hash, with_data, i, ext) < 0)