> -----Original Message----- > From: Lukasz Wojciechowski <l.wojciec...@partner.samsung.com> > Sent: Tuesday, May 5, 2020 4:50 AM > To: Harry van Haaren <harry.van.haa...@intel.com>; dev@dpdk.org > Cc: Honnappa Nagarahalli <honnappa.nagaraha...@arm.com>; Phil Yang > <phil.y...@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.haa...@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
