[v2,2/5] test/hash: remove duplicated test code

Message ID 20200203194912.4669-3-honnappa.nagarahalli@arm.com
State New
Headers show
Series
  • test/meson: fix hash readwrite timeout failure
Related show

Commit Message

Honnappa Nagarahalli Feb. 3, 2020, 7:49 p.m.
The test case target 'hash_readwrite_autotest' is covered by
'hash_readwrite_func_autotest' and 'hash_readwrite_perf_autotest'.
Hence, it is removed along with its test code.

Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>

---
 app/test/autotest_data.py      |  10 +++-
 app/test/test_hash_readwrite.c | 101 ---------------------------------
 2 files changed, 8 insertions(+), 103 deletions(-)

-- 
2.17.1

Comments

David Marchand Feb. 5, 2020, 8:48 a.m. | #1
On Mon, Feb 3, 2020 at 8:49 PM Honnappa Nagarahalli
<honnappa.nagarahalli@arm.com> wrote:
>

> The test case target 'hash_readwrite_autotest' is covered by

> 'hash_readwrite_func_autotest' and 'hash_readwrite_perf_autotest'.

> Hence, it is removed along with its test code.

>

> Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>

> ---

>  app/test/autotest_data.py      |  10 +++-

>  app/test/test_hash_readwrite.c | 101 ---------------------------------

>  2 files changed, 8 insertions(+), 103 deletions(-)

>

> diff --git a/app/test/autotest_data.py b/app/test/autotest_data.py

> index 6deb97bcc..71db4b3f6 100644

> --- a/app/test/autotest_data.py

> +++ b/app/test/autotest_data.py

> @@ -664,8 +664,14 @@

>          "Report":  None,

>      },

>      {

> -        "Name":    "Hash read-write concurrency autotest",

> -        "Command": "hash_readwrite_autotest",

> +        "Name":    "Hash read-write concurrency functional autotest",

> +        "Command": "hash_readwrite_func_autotest",

> +        "Func":    default_autotest,

> +        "Report":  None,

> +    },

> +    {

> +        "Name":    "Hash read-write concurrency perf autotest",

> +        "Command": "hash_readwrite_perf_autotest",

>          "Func":    default_autotest,

>          "Report":  None,

>      },

> diff --git a/app/test/test_hash_readwrite.c b/app/test/test_hash_readwrite.c

> index aa55db7fe..635ed5a9f 100644

> --- a/app/test/test_hash_readwrite.c

> +++ b/app/test/test_hash_readwrite.c

> @@ -749,106 +749,5 @@ test_hash_rw_func_main(void)

>         return 0;

>  }

>

> -static int

> -test_hash_readwrite_main(void)

> -{

> -       /*

> -        * Variables used to choose different tests.

> -        * use_htm indicates if hardware transactional memory should be used.

> -        * reader_faster indicates if the reader threads should finish earlier

> -        * than writer threads. This is to timing either reader threads or

> -        * writer threads for performance numbers.

> -        */

> -       int use_htm, use_ext,  reader_faster;

> -       unsigned int i = 0, core_id = 0;

> -

> -       if (rte_lcore_count() < 3) {

> -               printf("Not enough cores for hash_readwrite_autotest, expecting at least 3\n");

> -               return TEST_SKIPPED;

> -       }

> -

> -       RTE_LCORE_FOREACH_SLAVE(core_id) {

> -               slave_core_ids[i] = core_id;

> -               i++;

> -       }

> -

> -       setlocale(LC_NUMERIC, "");

> -

> -       if (rte_tm_supported()) {

> -               printf("Hardware transactional memory (lock elision) "

> -                       "is supported\n");

> -

> -               printf("Test read-write with Hardware transactional memory\n");

> -

> -               use_htm = 1;

> -               use_ext = 0;

> -

> -               if (test_hash_readwrite_functional(use_ext, use_htm) < 0)

> -                       return -1;

> -

> -               use_ext = 1;

> -               if (test_hash_readwrite_functional(use_ext, use_htm) < 0)

> -                       return -1;

> -

> -               reader_faster = 1;

> -               if (test_hash_readwrite_perf(&htm_results, use_htm,

> -                                                       reader_faster) < 0)

> -                       return -1;

> -

> -               reader_faster = 0;

> -               if (test_hash_readwrite_perf(&htm_results, use_htm,

> -                                                       reader_faster) < 0)

> -                       return -1;

> -       } else {

> -               printf("Hardware transactional memory (lock elision) "

> -                       "is NOT supported\n");

> -       }

> -

> -       printf("Test read-write without Hardware transactional memory\n");

> -       use_htm = 0;

> -       use_ext = 0;

> -       if (test_hash_readwrite_functional(use_ext, use_htm) < 0)

> -               return -1;

> -

> -       use_ext = 1;

> -       if (test_hash_readwrite_functional(use_ext, use_htm) < 0)

> -               return -1;

> -

> -       reader_faster = 1;

> -       if (test_hash_readwrite_perf(&non_htm_results, use_htm,

> -                                                       reader_faster) < 0)

> -               return -1;

> -       reader_faster = 0;

> -       if (test_hash_readwrite_perf(&non_htm_results, use_htm,

> -                                                       reader_faster) < 0)

> -               return -1;

> -

> -       printf("================\n");

> -       printf("Results summary:\n");

> -       printf("================\n");

> -

> -       printf("single read: %u\n", htm_results.single_read);

> -       printf("single write: %u\n", htm_results.single_write);

> -       for (i = 0; i < NUM_TEST; i++) {

> -               printf("+++ core_cnt: %u +++\n", core_cnt[i]);

> -               printf("HTM:\n");

> -               printf("  read only: %u\n", htm_results.read_only[i]);

> -               printf("  write only: %u\n", htm_results.write_only[i]);

> -               printf("  read-write read: %u\n", htm_results.read_write_r[i]);

> -               printf("  read-write write: %u\n", htm_results.read_write_w[i]);

> -

> -               printf("non HTM:\n");

> -               printf("  read only: %u\n", non_htm_results.read_only[i]);

> -               printf("  write only: %u\n", non_htm_results.write_only[i]);

> -               printf("  read-write read: %u\n",

> -                       non_htm_results.read_write_r[i]);

> -               printf("  read-write write: %u\n",

> -                       non_htm_results.read_write_w[i]);

> -       }

> -

> -       return 0;

> -}

> -

> -REGISTER_TEST_COMMAND(hash_readwrite_autotest, test_hash_readwrite_main);

>  REGISTER_TEST_COMMAND(hash_readwrite_func_autotest, test_hash_rw_func_main);

>  REGISTER_TEST_COMMAND(hash_readwrite_perf_autotest, test_hash_rw_perf_main);

> --

> 2.17.1

>


I can see that DTS references this test:
https://git.dpdk.org/tools/dts/tree/tests/TestSuite_unit_tests_eal.py#n164

Travis (and UNH) also runs those unit tests (via the list included in meson).

Is there a reason to keep those in DTS?


Thanks.

--
David Marchand
David Marchand Feb. 5, 2020, 4:42 p.m. | #2
On Wed, Feb 5, 2020 at 9:48 AM David Marchand <david.marchand@redhat.com> wrote:
>

> On Mon, Feb 3, 2020 at 8:49 PM Honnappa Nagarahalli

> <honnappa.nagarahalli@arm.com> wrote:

> >

> > The test case target 'hash_readwrite_autotest' is covered by

> > 'hash_readwrite_func_autotest' and 'hash_readwrite_perf_autotest'.

> > Hence, it is removed along with its test code.

> >

> > Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>

> > ---

> >  app/test/autotest_data.py      |  10 +++-

> >  app/test/test_hash_readwrite.c | 101 ---------------------------------

> >  2 files changed, 8 insertions(+), 103 deletions(-)

> >

> > diff --git a/app/test/autotest_data.py b/app/test/autotest_data.py

> > index 6deb97bcc..71db4b3f6 100644

> > --- a/app/test/autotest_data.py

> > +++ b/app/test/autotest_data.py

> > @@ -664,8 +664,14 @@

> >          "Report":  None,

> >      },

> >      {

> > -        "Name":    "Hash read-write concurrency autotest",

> > -        "Command": "hash_readwrite_autotest",

> > +        "Name":    "Hash read-write concurrency functional autotest",

> > +        "Command": "hash_readwrite_func_autotest",

> > +        "Func":    default_autotest,

> > +        "Report":  None,

> > +    },

> > +    {

> > +        "Name":    "Hash read-write concurrency perf autotest",

> > +        "Command": "hash_readwrite_perf_autotest",

> >          "Func":    default_autotest,

> >          "Report":  None,

> >      },

> > diff --git a/app/test/test_hash_readwrite.c b/app/test/test_hash_readwrite.c

> > index aa55db7fe..635ed5a9f 100644

> > --- a/app/test/test_hash_readwrite.c

> > +++ b/app/test/test_hash_readwrite.c

> > @@ -749,106 +749,5 @@ test_hash_rw_func_main(void)

> >         return 0;

> >  }

> >

> > -static int

> > -test_hash_readwrite_main(void)

> > -{

> > -       /*

> > -        * Variables used to choose different tests.

> > -        * use_htm indicates if hardware transactional memory should be used.

> > -        * reader_faster indicates if the reader threads should finish earlier

> > -        * than writer threads. This is to timing either reader threads or

> > -        * writer threads for performance numbers.

> > -        */

> > -       int use_htm, use_ext,  reader_faster;

> > -       unsigned int i = 0, core_id = 0;

> > -

> > -       if (rte_lcore_count() < 3) {

> > -               printf("Not enough cores for hash_readwrite_autotest, expecting at least 3\n");

> > -               return TEST_SKIPPED;

> > -       }

> > -

> > -       RTE_LCORE_FOREACH_SLAVE(core_id) {

> > -               slave_core_ids[i] = core_id;

> > -               i++;

> > -       }

> > -

> > -       setlocale(LC_NUMERIC, "");

> > -

> > -       if (rte_tm_supported()) {

> > -               printf("Hardware transactional memory (lock elision) "

> > -                       "is supported\n");

> > -

> > -               printf("Test read-write with Hardware transactional memory\n");

> > -

> > -               use_htm = 1;

> > -               use_ext = 0;

> > -

> > -               if (test_hash_readwrite_functional(use_ext, use_htm) < 0)

> > -                       return -1;

> > -

> > -               use_ext = 1;

> > -               if (test_hash_readwrite_functional(use_ext, use_htm) < 0)

> > -                       return -1;

> > -

> > -               reader_faster = 1;

> > -               if (test_hash_readwrite_perf(&htm_results, use_htm,

> > -                                                       reader_faster) < 0)

> > -                       return -1;

> > -

> > -               reader_faster = 0;

> > -               if (test_hash_readwrite_perf(&htm_results, use_htm,

> > -                                                       reader_faster) < 0)

> > -                       return -1;

> > -       } else {

> > -               printf("Hardware transactional memory (lock elision) "

> > -                       "is NOT supported\n");

> > -       }

> > -

> > -       printf("Test read-write without Hardware transactional memory\n");

> > -       use_htm = 0;

> > -       use_ext = 0;

> > -       if (test_hash_readwrite_functional(use_ext, use_htm) < 0)

> > -               return -1;

> > -

> > -       use_ext = 1;

> > -       if (test_hash_readwrite_functional(use_ext, use_htm) < 0)

> > -               return -1;

> > -

> > -       reader_faster = 1;

> > -       if (test_hash_readwrite_perf(&non_htm_results, use_htm,

> > -                                                       reader_faster) < 0)

> > -               return -1;

> > -       reader_faster = 0;

> > -       if (test_hash_readwrite_perf(&non_htm_results, use_htm,

> > -                                                       reader_faster) < 0)

> > -               return -1;

> > -

> > -       printf("================\n");

> > -       printf("Results summary:\n");

> > -       printf("================\n");

> > -

> > -       printf("single read: %u\n", htm_results.single_read);

> > -       printf("single write: %u\n", htm_results.single_write);

> > -       for (i = 0; i < NUM_TEST; i++) {

> > -               printf("+++ core_cnt: %u +++\n", core_cnt[i]);

> > -               printf("HTM:\n");

> > -               printf("  read only: %u\n", htm_results.read_only[i]);

> > -               printf("  write only: %u\n", htm_results.write_only[i]);

> > -               printf("  read-write read: %u\n", htm_results.read_write_r[i]);

> > -               printf("  read-write write: %u\n", htm_results.read_write_w[i]);

> > -

> > -               printf("non HTM:\n");

> > -               printf("  read only: %u\n", non_htm_results.read_only[i]);

> > -               printf("  write only: %u\n", non_htm_results.write_only[i]);

> > -               printf("  read-write read: %u\n",

> > -                       non_htm_results.read_write_r[i]);

> > -               printf("  read-write write: %u\n",

> > -                       non_htm_results.read_write_w[i]);

> > -       }

> > -

> > -       return 0;

> > -}

> > -

> > -REGISTER_TEST_COMMAND(hash_readwrite_autotest, test_hash_readwrite_main);

> >  REGISTER_TEST_COMMAND(hash_readwrite_func_autotest, test_hash_rw_func_main);

> >  REGISTER_TEST_COMMAND(hash_readwrite_perf_autotest, test_hash_rw_perf_main);

> > --

> > 2.17.1

> >

>

> I can see that DTS references this test:

> https://git.dpdk.org/tools/dts/tree/tests/TestSuite_unit_tests_eal.py#n164

>

> Travis (and UNH) also runs those unit tests (via the list included in meson).

>

> Is there a reason to keep those in DTS?



I will take this as a no and go ahead with this patch.


-- 
David Marchand

Patch

diff --git a/app/test/autotest_data.py b/app/test/autotest_data.py
index 6deb97bcc..71db4b3f6 100644
--- a/app/test/autotest_data.py
+++ b/app/test/autotest_data.py
@@ -664,8 +664,14 @@ 
         "Report":  None,
     },
     {
-        "Name":    "Hash read-write concurrency autotest",
-        "Command": "hash_readwrite_autotest",
+        "Name":    "Hash read-write concurrency functional autotest",
+        "Command": "hash_readwrite_func_autotest",
+        "Func":    default_autotest,
+        "Report":  None,
+    },
+    {
+        "Name":    "Hash read-write concurrency perf autotest",
+        "Command": "hash_readwrite_perf_autotest",
         "Func":    default_autotest,
         "Report":  None,
     },
diff --git a/app/test/test_hash_readwrite.c b/app/test/test_hash_readwrite.c
index aa55db7fe..635ed5a9f 100644
--- a/app/test/test_hash_readwrite.c
+++ b/app/test/test_hash_readwrite.c
@@ -749,106 +749,5 @@  test_hash_rw_func_main(void)
 	return 0;
 }
 
-static int
-test_hash_readwrite_main(void)
-{
-	/*
-	 * Variables used to choose different tests.
-	 * use_htm indicates if hardware transactional memory should be used.
-	 * reader_faster indicates if the reader threads should finish earlier
-	 * than writer threads. This is to timing either reader threads or
-	 * writer threads for performance numbers.
-	 */
-	int use_htm, use_ext,  reader_faster;
-	unsigned int i = 0, core_id = 0;
-
-	if (rte_lcore_count() < 3) {
-		printf("Not enough cores for hash_readwrite_autotest, expecting at least 3\n");
-		return TEST_SKIPPED;
-	}
-
-	RTE_LCORE_FOREACH_SLAVE(core_id) {
-		slave_core_ids[i] = core_id;
-		i++;
-	}
-
-	setlocale(LC_NUMERIC, "");
-
-	if (rte_tm_supported()) {
-		printf("Hardware transactional memory (lock elision) "
-			"is supported\n");
-
-		printf("Test read-write with Hardware transactional memory\n");
-
-		use_htm = 1;
-		use_ext = 0;
-
-		if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
-			return -1;
-
-		use_ext = 1;
-		if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
-			return -1;
-
-		reader_faster = 1;
-		if (test_hash_readwrite_perf(&htm_results, use_htm,
-							reader_faster) < 0)
-			return -1;
-
-		reader_faster = 0;
-		if (test_hash_readwrite_perf(&htm_results, use_htm,
-							reader_faster) < 0)
-			return -1;
-	} else {
-		printf("Hardware transactional memory (lock elision) "
-			"is NOT supported\n");
-	}
-
-	printf("Test read-write without Hardware transactional memory\n");
-	use_htm = 0;
-	use_ext = 0;
-	if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
-		return -1;
-
-	use_ext = 1;
-	if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
-		return -1;
-
-	reader_faster = 1;
-	if (test_hash_readwrite_perf(&non_htm_results, use_htm,
-							reader_faster) < 0)
-		return -1;
-	reader_faster = 0;
-	if (test_hash_readwrite_perf(&non_htm_results, use_htm,
-							reader_faster) < 0)
-		return -1;
-
-	printf("================\n");
-	printf("Results summary:\n");
-	printf("================\n");
-
-	printf("single read: %u\n", htm_results.single_read);
-	printf("single write: %u\n", htm_results.single_write);
-	for (i = 0; i < NUM_TEST; i++) {
-		printf("+++ core_cnt: %u +++\n", core_cnt[i]);
-		printf("HTM:\n");
-		printf("  read only: %u\n", htm_results.read_only[i]);
-		printf("  write only: %u\n", htm_results.write_only[i]);
-		printf("  read-write read: %u\n", htm_results.read_write_r[i]);
-		printf("  read-write write: %u\n", htm_results.read_write_w[i]);
-
-		printf("non HTM:\n");
-		printf("  read only: %u\n", non_htm_results.read_only[i]);
-		printf("  write only: %u\n", non_htm_results.write_only[i]);
-		printf("  read-write read: %u\n",
-			non_htm_results.read_write_r[i]);
-		printf("  read-write write: %u\n",
-			non_htm_results.read_write_w[i]);
-	}
-
-	return 0;
-}
-
-REGISTER_TEST_COMMAND(hash_readwrite_autotest, test_hash_readwrite_main);
 REGISTER_TEST_COMMAND(hash_readwrite_func_autotest, test_hash_rw_func_main);
 REGISTER_TEST_COMMAND(hash_readwrite_perf_autotest, test_hash_rw_perf_main);