DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] test/service: add perf test for service on app lcore
@ 2020-05-01 15:56 ` Harry van Haaren
  2020-05-04 20:50   ` Lukasz Wojciechowski
                     ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Harry van Haaren @ 2020-05-01 15:56 UTC (permalink / raw)
  To: dev; +Cc: honnappa.nagarahalli, phil.yang, Harry van Haaren

Add a performance test to the service run on app lcore auto-
test. This test runs the service in a tight loop, and measures
cycles passed, printing the results. It provides a quick cycle
cost value, enabling measuring performance of the function to
run a service on an application lcore.

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>

---

I'm suggesting to merge this patch before the bugfix/C11 patch series,
(v2 currently here: http://patches.dpdk.org/patch/69199/ )
as this would enable users to benchmark the "before" and "after"
states of the bugfix/C11 patches easier.

---
 app/test/test_service_cores.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/app/test/test_service_cores.c b/app/test/test_service_cores.c
index a922c7ddc..469243314 100644
--- a/app/test/test_service_cores.c
+++ b/app/test/test_service_cores.c
@@ -789,8 +789,18 @@ service_app_lcore_poll_impl(const int mt_safe)
 				"MT Unsafe: App core1 didn't return -EBUSY");
 	}
 
-	unregister_all();
+	/* Performance test: call in a loop, and measure tsc() */
+	const uint32_t perf_iters = (1 << 12);
+	uint64_t start = rte_rdtsc();
+	for (uint32_t i = 0; i < perf_iters; i++) {
+		int err = service_run_on_app_core_func(&id);
+		TEST_ASSERT_EQUAL(0, err, "perf test: returned run failure");
+	}
+	uint64_t end = rte_rdtsc();
+	printf("perf test for %s: %0.1f cycles per call\n", mt_safe ?
+		"MT Safe" : "MT Unsafe", (end - start)/(float)perf_iters);
 
+	unregister_all();
 	return TEST_SUCCESS;
 }
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH] test/service: add perf test for service on app lcore
  2020-05-01 15:56 ` [dpdk-dev] [PATCH] test/service: add perf test for service on app lcore Harry van Haaren
@ 2020-05-04 20:50   ` Lukasz Wojciechowski
  2020-05-05 10:21     ` Van Haaren, Harry
  2020-05-06 14:33     ` Phil Yang
  2020-05-06 15:44   ` David Marchand
  2020-05-06 17:16   ` [dpdk-dev] [PATCH v2] " Harry van Haaren
  2 siblings, 2 replies; 11+ messages in thread
From: Lukasz Wojciechowski @ 2020-05-04 20:50 UTC (permalink / raw)
  To: Harry van Haaren, dev; +Cc: honnappa.nagarahalli, phil.yang


W dniu 01.05.2020 o 17:56, Harry van Haaren pisze:
> Add a performance test to the service run on app lcore auto-
> test. This test runs the service in a tight loop, and measures
> cycles passed, printing the results. It provides a quick cycle
> cost value, enabling measuring performance of the function to
> run a service on an application lcore.
>
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
>
> ---
>
> I'm suggesting to merge this patch before the bugfix/C11 patch series,
> (v2 currently here: https://protect2.fireeye.com/url?k=fda15556-a06d9cd2-fda0de19-0cc47aa8f5ba-177ac65d20682aa8&q=1&u=http%3A%2F%2Fpatches.dpdk.org%2Fpatch%2F69199%2F )
> as this would enable users to benchmark the "before" and "after"
> states of the bugfix/C11 patches easier.
>
> ---
>   app/test/test_service_cores.c | 12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/app/test/test_service_cores.c b/app/test/test_service_cores.c
> index a922c7ddc..469243314 100644
> --- a/app/test/test_service_cores.c
> +++ b/app/test/test_service_cores.c
> @@ -789,8 +789,18 @@ service_app_lcore_poll_impl(const int mt_safe)
>   				"MT Unsafe: App core1 didn't return -EBUSY");
>   	}
>   
> -	unregister_all();
> +	/* Performance test: call in a loop, and measure tsc() */
> +	const uint32_t perf_iters = (1 << 12);
> +	uint64_t start = rte_rdtsc();
> +	for (uint32_t i = 0; i < perf_iters; i++) {
> +		int err = service_run_on_app_core_func(&id);
> +		TEST_ASSERT_EQUAL(0, err, "perf test: returned run failure");
> +	}
> +	uint64_t end = rte_rdtsc();
> +	printf("perf test for %s: %0.1f cycles per call\n", mt_safe ?
> +		"MT Safe" : "MT Unsafe", (end - start)/(float)perf_iters);
>   
> +	unregister_all();
>   	return TEST_SUCCESS;
>   }
>   

Hi Harry,


I like the idea of adding this test. I checked it and it works fine.
However have you considered adding it as a separate testcase or even 
better as "service_perf_autotest" command ?

With your changes the: service_app_lcore_mt_safe and 
service_app_lcore_mt_unsafe unit tests cases have multiple 
functionalities: they test simultaneous execution of service and they do 
performance checks.


Best regards

Lukasz

-- 

Lukasz Wojciechowski
Principal Software Engineer

Samsung R&D Institute Poland
Samsung Electronics
Office +48 22 377 88 25
l.wojciechow@partner.samsung.com


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH] test/service: add perf test for service on app lcore
  2020-05-04 20:50   ` Lukasz Wojciechowski
@ 2020-05-05 10:21     ` Van Haaren, Harry
  2020-05-05 12:38       ` Lukasz Wojciechowski
  2020-05-06 14:33     ` Phil Yang
  1 sibling, 1 reply; 11+ messages in thread
From: Van Haaren, Harry @ 2020-05-05 10:21 UTC (permalink / raw)
  To: Lukasz Wojciechowski, dev; +Cc: honnappa.nagarahalli, phil.yang

> -----Original Message-----
> From: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
> Sent: Monday, May 4, 2020 9:50 PM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>; dev@dpdk.org
> Cc: honnappa.nagarahalli@arm.com; phil.yang@arm.com
> Subject: Re: [dpdk-dev] [PATCH] test/service: add perf test for service on app
> lcore

<snip code>

> > +	unregister_all();
> >   	return TEST_SUCCESS;
> >   }
> >
> 
> Hi Harry,

Hey Lukasz,

> I like the idea of adding this test. I checked it and it works fine.

Thanks for testing - please send a "Tested-by: .. <email>" and we can track
your input into the git logs :)

> However have you considered adding it as a separate testcase or even
> better as "service_perf_autotest" command ?

Actually I started implementing a new function with a very similar name,
but realized that the configuration/mapping would be duplicated, only to
have a different named test... so opted for this smaller diff solution.

> With your changes the: service_app_lcore_mt_safe and
> service_app_lcore_mt_unsafe unit tests cases have multiple
> functionalities: they test simultaneous execution of service and they do
> performance checks.

Correct - if you feel strongly that this isn't right I can refactor to a v2,
however I expect there would be more duplicated code to manage, and
hence I decided the above approach was simpler and easier.

> Best regards
> Lukasz

Regards, -Harry

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH] test/service: add perf test for service on app lcore
  2020-05-05 10:21     ` Van Haaren, Harry
@ 2020-05-05 12:38       ` Lukasz Wojciechowski
  0 siblings, 0 replies; 11+ messages in thread
From: Lukasz Wojciechowski @ 2020-05-05 12:38 UTC (permalink / raw)
  To: Van Haaren, Harry, dev; +Cc: honnappa.nagarahalli, phil.yang


W dniu 05.05.2020 o 12:21, Van Haaren, Harry pisze:
>> -----Original Message-----
>> From: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
>> Sent: Monday, May 4, 2020 9:50 PM
>> To: Van Haaren, Harry <harry.van.haaren@intel.com>; dev@dpdk.org
>> Cc: honnappa.nagarahalli@arm.com; phil.yang@arm.com
>> Subject: Re: [dpdk-dev] [PATCH] test/service: add perf test for service on app
>> lcore
> <snip code>
>
>>> +	unregister_all();
>>>    	return TEST_SUCCESS;
>>>    }
>>>
>> Hi Harry,
> Hey Lukasz,
>
>> I like the idea of adding this test. I checked it and it works fine.
> Thanks for testing - please send a "Tested-by: .. <email>" and we can track
> your input into the git logs :)

No problem, I just wanted to get you opinion first, but here you go:

Tested-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>

>
>> However have you considered adding it as a separate testcase or even
>> better as "service_perf_autotest" command ?
> Actually I started implementing a new function with a very similar name,
> but realized that the configuration/mapping would be duplicated, only to
> have a different named test... so opted for this smaller diff solution.
>
>> With your changes the: service_app_lcore_mt_safe and
>> service_app_lcore_mt_unsafe unit tests cases have multiple
>> functionalities: they test simultaneous execution of service and they do
>> performance checks.
> Correct - if you feel strongly that this isn't right I can refactor to a v2,
> however I expect there would be more duplicated code to manage, and
> hence I decided the above approach was simpler and easier.
It's up to you and some more dpdk-experienced guys. Isn't there a way 
not to duplicate the code but wrap it in a function?
You don't really need that much, because you run the performance test 
after the former version of test is completed. You don't need 2 lcores 
also as you measure direct start of service function, which also is 
almost empty as it immediately returns because the loop breaking 
conditions are already set.
>
>> Best regards
>> Lukasz
> Regards, -Harry

-- 

Lukasz Wojciechowski
Principal Software Engineer

Samsung R&D Institute Poland
Samsung Electronics
Office +48 22 377 88 25
l.wojciechow@partner.samsung.com


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH] test/service: add perf test for service on app lcore
  2020-05-04 20:50   ` Lukasz Wojciechowski
  2020-05-05 10:21     ` Van Haaren, Harry
@ 2020-05-06 14:33     ` Phil Yang
  1 sibling, 0 replies; 11+ messages in thread
From: Phil Yang @ 2020-05-06 14:33 UTC (permalink / raw)
  To: Lukasz Wojciechowski, Harry van Haaren, dev; +Cc: Honnappa Nagarahalli, nd

> -----Original Message-----
> From: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
> Sent: Tuesday, May 5, 2020 4:50 AM
> To: Harry van Haaren <harry.van.haaren@intel.com>; dev@dpdk.org
> Cc: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; Phil Yang
> <Phil.Yang@arm.com>
> Subject: Re: [dpdk-dev] [PATCH] test/service: add perf test for service on
> app lcore
> 
> 
> W dniu 01.05.2020 o 17:56, Harry van Haaren pisze:
> > Add a performance test to the service run on app lcore auto-
> > test. This test runs the service in a tight loop, and measures
> > cycles passed, printing the results. It provides a quick cycle
> > cost value, enabling measuring performance of the function to
> > run a service on an application lcore.
> >
> > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> >
> > ---
> >
> > I'm suggesting to merge this patch before the bugfix/C11 patch series,
> > (v2 currently here: https://protect2.fireeye.com/url?k=fda15556-
> a06d9cd2-fda0de19-0cc47aa8f5ba-
> 177ac65d20682aa8&q=1&u=http%3A%2F%2Fpatches.dpdk.org%2Fpatch%2F
> 69199%2F )
> > as this would enable users to benchmark the "before" and "after"
> > states of the bugfix/C11 patches easier.
> >
> > ---
> >   app/test/test_service_cores.c | 12 +++++++++++-
> >   1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/test/test_service_cores.c b/app/test/test_service_cores.c
> > index a922c7ddc..469243314 100644
> > --- a/app/test/test_service_cores.c
> > +++ b/app/test/test_service_cores.c
> > @@ -789,8 +789,18 @@ service_app_lcore_poll_impl(const int mt_safe)
> >   				"MT Unsafe: App core1 didn't return -
> EBUSY");
> >   	}
> >
> > -	unregister_all();
> > +	/* Performance test: call in a loop, and measure tsc() */
> > +	const uint32_t perf_iters = (1 << 12);
> > +	uint64_t start = rte_rdtsc();
> > +	for (uint32_t i = 0; i < perf_iters; i++) {
> > +		int err = service_run_on_app_core_func(&id);
> > +		TEST_ASSERT_EQUAL(0, err, "perf test: returned run failure");
> > +	}
> > +	uint64_t end = rte_rdtsc();
> > +	printf("perf test for %s: %0.1f cycles per call\n", mt_safe ?
> > +		"MT Safe" : "MT Unsafe", (end - start)/(float)perf_iters);
> >
> > +	unregister_all();
> >   	return TEST_SUCCESS;
> >   }
> >
> 
> Hi Harry,
> 
> 
> I like the idea of adding this test. I checked it and it works fine.
> However have you considered adding it as a separate testcase or even
> better as "service_perf_autotest" command ?
> 
> With your changes the: service_app_lcore_mt_safe and
> service_app_lcore_mt_unsafe unit tests cases have multiple
> functionalities: they test simultaneous execution of service and they do
> performance checks.

+1 for this.

This patch will skip MT safe UT, but it will continue the MT safe performance test.   It's a defect. E.g:
-------
+ TestCase [12] : service_mt_safe_poll skipped
perf test for MT Safe: 40.2 cycles per call
 + TestCase [13] : service_app_lcore_mt_safe succeeded
perf test for MT Unsafe: 53.7 cycles per call
--------

If you want to put the performance test and functional test in the same test, I think it is better to add some indents before print the performance test output to align with the functional test output format.  Such as:
------
+ TestCase [13] : service_app_lcore_mt_safe succeeded
+		  perf test for MT Unsafe: 53.7 cycles per call
------


According to this performance case, the C11 version patches got 20% performance improvement on aarch64 and 8.5% on x86 for the MT unsafe case. In MT safe case, it got 10% performance improvement on aarch64 and 17% on x86. These are preliminary test results, only covered one testbed for each platform.

Thanks,
Phil

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH] test/service: add perf test for service on app lcore
  2020-05-01 15:56 ` [dpdk-dev] [PATCH] test/service: add perf test for service on app lcore Harry van Haaren
  2020-05-04 20:50   ` Lukasz Wojciechowski
@ 2020-05-06 15:44   ` David Marchand
  2020-05-06 17:00     ` Van Haaren, Harry
  2020-05-06 17:16   ` [dpdk-dev] [PATCH v2] " Harry van Haaren
  2 siblings, 1 reply; 11+ messages in thread
From: David Marchand @ 2020-05-06 15:44 UTC (permalink / raw)
  To: Harry van Haaren; +Cc: dev, Honnappa Nagarahalli, Phil Yang

On Fri, May 1, 2020 at 5:56 PM Harry van Haaren
<harry.van.haaren@intel.com> wrote:
>
> Add a performance test to the service run on app lcore auto-
> test. This test runs the service in a tight loop, and measures
> cycles passed, printing the results. It provides a quick cycle
> cost value, enabling measuring performance of the function to
> run a service on an application lcore.
>
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
>
> ---
>
> I'm suggesting to merge this patch before the bugfix/C11 patch series,
> (v2 currently here: http://patches.dpdk.org/patch/69199/ )
> as this would enable users to benchmark the "before" and "after"
> states of the bugfix/C11 patches easier.
>
> ---
>  app/test/test_service_cores.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/app/test/test_service_cores.c b/app/test/test_service_cores.c
> index a922c7ddc..469243314 100644
> --- a/app/test/test_service_cores.c
> +++ b/app/test/test_service_cores.c
> @@ -789,8 +789,18 @@ service_app_lcore_poll_impl(const int mt_safe)
>                                 "MT Unsafe: App core1 didn't return -EBUSY");
>         }
>
> -       unregister_all();
> +       /* Performance test: call in a loop, and measure tsc() */
> +       const uint32_t perf_iters = (1 << 12);
> +       uint64_t start = rte_rdtsc();
> +       for (uint32_t i = 0; i < perf_iters; i++) {

- How long does this test take now?
We tend to put performance tests in dedicated tests to avoid issues in Travis.
I suppose this is quick, but still want a confirmation.


- Centos7/RHEL7 gcc is not happy with this.

http://mails.dpdk.org/archives/test-report/2020-May/129993.html

../app/test/test_service_cores.c: In function ‘service_app_lcore_poll_impl’:
../app/test/test_service_cores.c:795:2: error: ‘for’ loop initial
declarations are only allowed in C99 mode
  for (uint32_t i = 0; i < perf_iters; i++) {
  ^
../app/test/test_service_cores.c:795:2: note: use option -std=c99 or
-std=gnu99 to compile your code


> +               int err = service_run_on_app_core_func(&id);
> +               TEST_ASSERT_EQUAL(0, err, "perf test: returned run failure");
> +       }
> +       uint64_t end = rte_rdtsc();
> +       printf("perf test for %s: %0.1f cycles per call\n", mt_safe ?
> +               "MT Safe" : "MT Unsafe", (end - start)/(float)perf_iters);
>
> +       unregister_all();
>         return TEST_SUCCESS;
>  }
>

- Can you look at Phil comments too?


-- 
David Marchand


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH] test/service: add perf test for service on app lcore
  2020-05-06 15:44   ` David Marchand
@ 2020-05-06 17:00     ` Van Haaren, Harry
  0 siblings, 0 replies; 11+ messages in thread
From: Van Haaren, Harry @ 2020-05-06 17:00 UTC (permalink / raw)
  To: David Marchand; +Cc: dev, Honnappa Nagarahalli, Phil Yang

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Wednesday, May 6, 2020 4:44 PM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>
> Cc: dev <dev@dpdk.org>; Honnappa Nagarahalli
> <honnappa.nagarahalli@arm.com>; Phil Yang <phil.yang@arm.com>
> Subject: Re: [dpdk-dev] [PATCH] test/service: add perf test for service on app
<snip>
> > +       /* Performance test: call in a loop, and measure tsc() */
> > +       const uint32_t perf_iters = (1 << 12);
> > +       uint64_t start = rte_rdtsc();
> > +       for (uint32_t i = 0; i < perf_iters; i++) {
> 
> - How long does this test take now?
> We tend to put performance tests in dedicated tests to avoid issues in Travis.
> I suppose this is quick, but still want a confirmation.

Can confirm this is quick, blink of an eye quick, maybe ~250k cycles.


> - Centos7/RHEL7 gcc is not happy with this.
> 
> http://mails.dpdk.org/archives/test-report/2020-May/129993.html
> 
> ../app/test/test_service_cores.c: In function ‘service_app_lcore_poll_impl’:
> ../app/test/test_service_cores.c:795:2: error: ‘for’ loop initial
> declarations are only allowed in C99 mode
>   for (uint32_t i = 0; i < perf_iters; i++) {
>   ^
> ../app/test/test_service_cores.c:795:2: note: use option -std=c99 or
> -std=gnu99 to compile your code

Will fix in v2.


> > +               int err = service_run_on_app_core_func(&id);
> > +               TEST_ASSERT_EQUAL(0, err, "perf test: returned run failure");
> > +       }
> > +       uint64_t end = rte_rdtsc();
> > +       printf("perf test for %s: %0.1f cycles per call\n", mt_safe ?
> > +               "MT Safe" : "MT Unsafe", (end - start)/(float)perf_iters);
> >
> > +       unregister_all();
> >         return TEST_SUCCESS;
> >  }
> >
> 
> - Can you look at Phil comments too?

Yes - will update to use padding at start of line in output.

V2 on the way.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [dpdk-dev] [PATCH v2] test/service: add perf test for service on app lcore
  2020-05-01 15:56 ` [dpdk-dev] [PATCH] test/service: add perf test for service on app lcore Harry van Haaren
  2020-05-04 20:50   ` Lukasz Wojciechowski
  2020-05-06 15:44   ` David Marchand
@ 2020-05-06 17:16   ` Harry van Haaren
  2020-05-07  6:28     ` Phil Yang
  2020-05-11 11:20     ` David Marchand
  2 siblings, 2 replies; 11+ messages in thread
From: Harry van Haaren @ 2020-05-06 17:16 UTC (permalink / raw)
  To: dev; +Cc: david.marchand, phil.yang, honnappa.nagarahalli, Harry van Haaren

This commit adds a basic test to check the cycle cost
of related to calling into a service.

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>

---

v2:
- Add space at start of output (Phil Yang)
- Fix compile error on older GCCs (David Marchand)

---
 app/test/test_service_cores.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/app/test/test_service_cores.c b/app/test/test_service_cores.c
index a922c7ddc..75a3f8b45 100644
--- a/app/test/test_service_cores.c
+++ b/app/test/test_service_cores.c
@@ -789,8 +789,19 @@ service_app_lcore_poll_impl(const int mt_safe)
 				"MT Unsafe: App core1 didn't return -EBUSY");
 	}
 
-	unregister_all();
+	/* Performance test: call in a loop, and measure tsc() */
+	uint32_t i;
+	const uint32_t perf_iters = (1 << 12);
+	uint64_t start = rte_rdtsc();
+	for (i = 0; i < perf_iters; i++) {
+		int err = service_run_on_app_core_func(&id);
+		TEST_ASSERT_EQUAL(0, err, "perf test: returned run failure");
+	}
+	uint64_t end = rte_rdtsc();
+	printf("\t\tperf test for %s: %0.1f cycles per call\n", mt_safe ?
+		"MT Safe" : "MT Unsafe", (end - start)/(float)perf_iters);
 
+	unregister_all();
 	return TEST_SUCCESS;
 }
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v2] test/service: add perf test for service on app lcore
  2020-05-06 17:16   ` [dpdk-dev] [PATCH v2] " Harry van Haaren
@ 2020-05-07  6:28     ` Phil Yang
  2020-05-07 12:11       ` David Marchand
  2020-05-11 11:20     ` David Marchand
  1 sibling, 1 reply; 11+ messages in thread
From: Phil Yang @ 2020-05-07  6:28 UTC (permalink / raw)
  To: Harry van Haaren, dev; +Cc: david.marchand, Honnappa Nagarahalli, nd

> -----Original Message-----
> From: Harry van Haaren <harry.van.haaren@intel.com>
> Sent: Thursday, May 7, 2020 1:17 AM
> To: dev@dpdk.org
> Cc: david.marchand@redhat.com; Phil Yang <Phil.Yang@arm.com>;
> Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; Harry van Haaren
> <harry.van.haaren@intel.com>
> Subject: [PATCH v2] test/service: add perf test for service on app lcore
> 
> This commit adds a basic test to check the cycle cost
> of related to calling into a service.
> 
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> 
> ---
> 
> v2:
> - Add space at start of output (Phil Yang)
> - Fix compile error on older GCCs (David Marchand)
> 
> ---
>  app/test/test_service_cores.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test/test_service_cores.c b/app/test/test_service_cores.c
> index a922c7ddc..75a3f8b45 100644
> --- a/app/test/test_service_cores.c
> +++ b/app/test/test_service_cores.c
> @@ -789,8 +789,19 @@ service_app_lcore_poll_impl(const int mt_safe)
>  				"MT Unsafe: App core1 didn't return -
> EBUSY");
>  	}
> 
> -	unregister_all();
> +	/* Performance test: call in a loop, and measure tsc() */
> +	uint32_t i;
> +	const uint32_t perf_iters = (1 << 12);
> +	uint64_t start = rte_rdtsc();
> +	for (i = 0; i < perf_iters; i++) {
> +		int err = service_run_on_app_core_func(&id);
> +		TEST_ASSERT_EQUAL(0, err, "perf test: returned run failure");
> +	}
> +	uint64_t end = rte_rdtsc();
> +	printf("\t\tperf test for %s: %0.1f cycles per call\n", mt_safe ?

This print will be printed before the function test result.
Apologize for my previous comments. Please ignore it. 

A nitpick comment on formatting the output.

-       printf("\t\tperf test for %s: %0.1f cycles per call\n", mt_safe ?
+       printf("Perf test for %-10s: %0.1f cycles per call\n", mt_safe ?


Tested-by: Phil Yang <phil.yang@arm.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>

Thanks,
Phil

> +		"MT Safe" : "MT Unsafe", (end - start)/(float)perf_iters);
> 
> +	unregister_all();
>  	return TEST_SUCCESS;
>  }
> 
> --
> 2.17.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v2] test/service: add perf test for service on app lcore
  2020-05-07  6:28     ` Phil Yang
@ 2020-05-07 12:11       ` David Marchand
  0 siblings, 0 replies; 11+ messages in thread
From: David Marchand @ 2020-05-07 12:11 UTC (permalink / raw)
  To: Phil Yang, Harry van Haaren; +Cc: dev, Honnappa Nagarahalli, nd

On Thu, May 7, 2020 at 8:29 AM Phil Yang <Phil.Yang@arm.com> wrote:
>
> > -----Original Message-----
> > From: Harry van Haaren <harry.van.haaren@intel.com>
> > Sent: Thursday, May 7, 2020 1:17 AM
> > To: dev@dpdk.org
> > Cc: david.marchand@redhat.com; Phil Yang <Phil.Yang@arm.com>;
> > Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; Harry van Haaren
> > <harry.van.haaren@intel.com>
> > Subject: [PATCH v2] test/service: add perf test for service on app lcore
> >
> > This commit adds a basic test to check the cycle cost
> > of related to calling into a service.
> >
> > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> >
> > ---
> >
> > v2:
> > - Add space at start of output (Phil Yang)
> > - Fix compile error on older GCCs (David Marchand)
> >
> > ---
> >  app/test/test_service_cores.c | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/test/test_service_cores.c b/app/test/test_service_cores.c
> > index a922c7ddc..75a3f8b45 100644
> > --- a/app/test/test_service_cores.c
> > +++ b/app/test/test_service_cores.c
> > @@ -789,8 +789,19 @@ service_app_lcore_poll_impl(const int mt_safe)
> >                               "MT Unsafe: App core1 didn't return -
> > EBUSY");
> >       }
> >
> > -     unregister_all();
> > +     /* Performance test: call in a loop, and measure tsc() */
> > +     uint32_t i;
> > +     const uint32_t perf_iters = (1 << 12);
> > +     uint64_t start = rte_rdtsc();
> > +     for (i = 0; i < perf_iters; i++) {
> > +             int err = service_run_on_app_core_func(&id);
> > +             TEST_ASSERT_EQUAL(0, err, "perf test: returned run failure");
> > +     }
> > +     uint64_t end = rte_rdtsc();
> > +     printf("\t\tperf test for %s: %0.1f cycles per call\n", mt_safe ?
>
> This print will be printed before the function test result.
> Apologize for my previous comments. Please ignore it.
>
> A nitpick comment on formatting the output.
>
> -       printf("\t\tperf test for %s: %0.1f cycles per call\n", mt_safe ?
> +       printf("Perf test for %-10s: %0.1f cycles per call\n", mt_safe ?
>
>
> Tested-by: Phil Yang <phil.yang@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>

Ok, so I will restore this part from the v1 when applying.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v2] test/service: add perf test for service on app lcore
  2020-05-06 17:16   ` [dpdk-dev] [PATCH v2] " Harry van Haaren
  2020-05-07  6:28     ` Phil Yang
@ 2020-05-11 11:20     ` David Marchand
  1 sibling, 0 replies; 11+ messages in thread
From: David Marchand @ 2020-05-11 11:20 UTC (permalink / raw)
  To: Harry van Haaren; +Cc: dev, Phil Yang, Honnappa Nagarahalli

On Wed, May 6, 2020 at 7:15 PM Harry van Haaren
<harry.van.haaren@intel.com> wrote:
>
> This commit adds a basic test to check the cycle cost
> of related to calling into a service.
>
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>

Tested-by: Phil Yang <phil.yang@arm.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>

Restored output to v1 format following Phil comment.

Applied, thanks.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-05-11 11:21 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20200501155607eucas1p2509870b6b5f4cc520dca11888e221b1b@eucas1p2.samsung.com>
2020-05-01 15:56 ` [dpdk-dev] [PATCH] test/service: add perf test for service on app lcore Harry van Haaren
2020-05-04 20:50   ` Lukasz Wojciechowski
2020-05-05 10:21     ` Van Haaren, Harry
2020-05-05 12:38       ` Lukasz Wojciechowski
2020-05-06 14:33     ` Phil Yang
2020-05-06 15:44   ` David Marchand
2020-05-06 17:00     ` Van Haaren, Harry
2020-05-06 17:16   ` [dpdk-dev] [PATCH v2] " Harry van Haaren
2020-05-07  6:28     ` Phil Yang
2020-05-07 12:11       ` David Marchand
2020-05-11 11:20     ` David Marchand

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).