diff mbox series

[2/5] app/testpmd: print fractional part in CPU cycles

Message ID 20200506215847.7628-2-dharmik.thakkar@arm.com (mailing list archive)
State Superseded, archived
Headers
Series [1/5] app/testpmd: print clock with CPU cycles per pkt |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Dharmik Thakkar May 6, 2020, 9:58 p.m. UTC 
  Change printing of CPU cycles/packet to include fractional part for
accurateness.

Example:

Without patch:
CPU cycles/packet=14
(total cycles=4899533541 / total RX packets=343031966)

With patch:
CPU cycles/packet=14.28
(total cycles=4899533541 / total RX packets=343031966)

Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>
---
 app/test-pmd/testpmd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Ananyev, Konstantin May 7, 2020, 9:50 a.m. UTC | #1 
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Dharmik Thakkar
> Sent: Wednesday, May 6, 2020 10:59 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Iremonger, Bernard <bernard.iremonger@intel.com>
> Cc: dev@dpdk.org; nd@arm.com; Dharmik Thakkar <dharmik.thakkar@arm.com>
> Subject: [dpdk-dev] [PATCH 2/5] app/testpmd: print fractional part in CPU cycles
> 
> Change printing of CPU cycles/packet to include fractional part for
> accurateness.
> 
> Example:
> 
> Without patch:
> CPU cycles/packet=14
> (total cycles=4899533541 / total RX packets=343031966)
> 
> With patch:
> CPU cycles/packet=14.28
> (total cycles=4899533541 / total RX packets=343031966)
> 
> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>
> ---
>  app/test-pmd/testpmd.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 9a8cbbd6fc7c..9444a730a153 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -1955,9 +1955,9 @@ fwd_stats_display(void)
>  #ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
>  #define CYC_PER_MHZ 1E6
>  	if (total_recv > 0)
> -		printf("\n  CPU cycles/packet=%u (total cycles="
> +		printf("\n  CPU cycles/packet=%.2f (total cycles="
>  		       "%"PRIu64" / total RX packets=%"PRIu64") at %lu MHz Clock\n",
> -		       (unsigned int)(fwd_cycles / total_recv),
> +		       (double)(fwd_cycles / (double)total_recv),

Probably safer long double - to avoid overflow. 

>  		       fwd_cycles, total_recv, (uint64_t)(rte_get_tsc_hz() / CYC_PER_MHZ));
>  #endif
>  }
> --
> 2.20.1
Dharmik Thakkar May 7, 2020, 10:16 p.m. UTC | #2 
Hi Konstantin,

> On May 7, 2020, at 4:50 AM, Ananyev, Konstantin <konstantin.ananyev@intel.com> wrote:
> 
> 
> 
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Dharmik Thakkar
>> Sent: Wednesday, May 6, 2020 10:59 PM
>> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Iremonger, Bernard <bernard.iremonger@intel.com>
>> Cc: dev@dpdk.org; nd@arm.com; Dharmik Thakkar <dharmik.thakkar@arm.com>
>> Subject: [dpdk-dev] [PATCH 2/5] app/testpmd: print fractional part in CPU cycles
>> 
>> Change printing of CPU cycles/packet to include fractional part for
>> accurateness.
>> 
>> Example:
>> 
>> Without patch:
>> CPU cycles/packet=14
>> (total cycles=4899533541 / total RX packets=343031966)
>> 
>> With patch:
>> CPU cycles/packet=14.28
>> (total cycles=4899533541 / total RX packets=343031966)
>> 
>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>> Reviewed-by: Phil Yang <phil.yang@arm.com>
>> ---
>> app/test-pmd/testpmd.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
>> index 9a8cbbd6fc7c..9444a730a153 100644
>> --- a/app/test-pmd/testpmd.c
>> +++ b/app/test-pmd/testpmd.c
>> @@ -1955,9 +1955,9 @@ fwd_stats_display(void)
>> #ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
>> #define CYC_PER_MHZ 1E6
>> 	if (total_recv > 0)
>> -		printf("\n  CPU cycles/packet=%u (total cycles="
>> +		printf("\n  CPU cycles/packet=%.2f (total cycles="
>> 		       "%"PRIu64" / total RX packets=%"PRIu64") at %lu MHz Clock\n",
>> -		       (unsigned int)(fwd_cycles / total_recv),
>> +		       (double)(fwd_cycles / (double)total_recv),
> 
> Probably safer long double - to avoid overflow. 

Is it possible for a ‘double' to be less than 8 bytes?

> 
>> 		       fwd_cycles, total_recv, (uint64_t)(rte_get_tsc_hz() / CYC_PER_MHZ));
>> #endif
>> }
>> --
>> 2.20.1
Ananyev, Konstantin May 8, 2020, 5:17 p.m. UTC | #3 
> Hi Konstantin,
> 
> > On May 7, 2020, at 4:50 AM, Ananyev, Konstantin <konstantin.ananyev@intel.com> wrote:
> >
> >
> >
> >> -----Original Message-----
> >> From: dev <dev-bounces@dpdk.org> On Behalf Of Dharmik Thakkar
> >> Sent: Wednesday, May 6, 2020 10:59 PM
> >> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Iremonger, Bernard <bernard.iremonger@intel.com>
> >> Cc: dev@dpdk.org; nd@arm.com; Dharmik Thakkar <dharmik.thakkar@arm.com>
> >> Subject: [dpdk-dev] [PATCH 2/5] app/testpmd: print fractional part in CPU cycles
> >>
> >> Change printing of CPU cycles/packet to include fractional part for
> >> accurateness.
> >>
> >> Example:
> >>
> >> Without patch:
> >> CPU cycles/packet=14
> >> (total cycles=4899533541 / total RX packets=343031966)
> >>
> >> With patch:
> >> CPU cycles/packet=14.28
> >> (total cycles=4899533541 / total RX packets=343031966)
> >>
> >> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> >> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> >> Reviewed-by: Phil Yang <phil.yang@arm.com>
> >> ---
> >> app/test-pmd/testpmd.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> >> index 9a8cbbd6fc7c..9444a730a153 100644
> >> --- a/app/test-pmd/testpmd.c
> >> +++ b/app/test-pmd/testpmd.c
> >> @@ -1955,9 +1955,9 @@ fwd_stats_display(void)
> >> #ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
> >> #define CYC_PER_MHZ 1E6
> >> 	if (total_recv > 0)
> >> -		printf("\n  CPU cycles/packet=%u (total cycles="
> >> +		printf("\n  CPU cycles/packet=%.2f (total cycles="
> >> 		       "%"PRIu64" / total RX packets=%"PRIu64") at %lu MHz Clock\n",
> >> -		       (unsigned int)(fwd_cycles / total_recv),
> >> +		       (double)(fwd_cycles / (double)total_recv),
> >
> > Probably safer long double - to avoid overflow.
> 
> Is it possible for a ‘double' to be less than 8 bytes?

That was my initial thought - that on some 32 bit systems it could be 4B.
Though it seems I was wrong, so feel free to ignore.
BTW, what for double conversion, why not just:
double)(fwd_cycles /total_recv
?



> 
> >
> >> 		       fwd_cycles, total_recv, (uint64_t)(rte_get_tsc_hz() / CYC_PER_MHZ));
> >> #endif
> >> }
> >> --
> >> 2.20.1
Dharmik Thakkar May 8, 2020, 5:36 p.m. UTC | #4 
> On May 8, 2020, at 12:17 PM, Ananyev, Konstantin <konstantin.ananyev@intel.com> wrote:
> 
>> Hi Konstantin,
>> 
>>> On May 7, 2020, at 4:50 AM, Ananyev, Konstantin <konstantin.ananyev@intel.com> wrote:
>>> 
>>> 
>>> 
>>>> -----Original Message-----
>>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Dharmik Thakkar
>>>> Sent: Wednesday, May 6, 2020 10:59 PM
>>>> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Iremonger, Bernard <bernard.iremonger@intel.com>
>>>> Cc: dev@dpdk.org; nd@arm.com; Dharmik Thakkar <dharmik.thakkar@arm.com>
>>>> Subject: [dpdk-dev] [PATCH 2/5] app/testpmd: print fractional part in CPU cycles
>>>> 
>>>> Change printing of CPU cycles/packet to include fractional part for
>>>> accurateness.
>>>> 
>>>> Example:
>>>> 
>>>> Without patch:
>>>> CPU cycles/packet=14
>>>> (total cycles=4899533541 / total RX packets=343031966)
>>>> 
>>>> With patch:
>>>> CPU cycles/packet=14.28
>>>> (total cycles=4899533541 / total RX packets=343031966)
>>>> 
>>>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>>>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>>>> Reviewed-by: Phil Yang <phil.yang@arm.com>
>>>> ---
>>>> app/test-pmd/testpmd.c | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>> 
>>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
>>>> index 9a8cbbd6fc7c..9444a730a153 100644
>>>> --- a/app/test-pmd/testpmd.c
>>>> +++ b/app/test-pmd/testpmd.c
>>>> @@ -1955,9 +1955,9 @@ fwd_stats_display(void)
>>>> #ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
>>>> #define CYC_PER_MHZ 1E6
>>>> 	if (total_recv > 0)
>>>> -		printf("\n  CPU cycles/packet=%u (total cycles="
>>>> +		printf("\n  CPU cycles/packet=%.2f (total cycles="
>>>> 		       "%"PRIu64" / total RX packets=%"PRIu64") at %lu MHz Clock\n",
>>>> -		       (unsigned int)(fwd_cycles / total_recv),
>>>> +		       (double)(fwd_cycles / (double)total_recv),
>>> 
>>> Probably safer long double - to avoid overflow.
>> 
>> Is it possible for a ‘double' to be less than 8 bytes?
> 
> That was my initial thought - that on some 32 bit systems it could be 4B.
> Though it seems I was wrong, so feel free to ignore.
> BTW, what for double conversion, why not just:
> double)(fwd_cycles /total_recv
> ?

Without (double) total recv, I will always get the fractional part as .00.
For the above example, with (double)(fwd_cycles / total_recv), I see 14.00 instead of 14.28

> 
> 
> 
>> 
>>> 
>>>> 		       fwd_cycles, total_recv, (uint64_t)(rte_get_tsc_hz() / CYC_PER_MHZ));
>>>> #endif
>>>> }
>>>> --
>>>> 2.20.1
Ananyev, Konstantin May 8, 2020, 6:08 p.m. UTC | #5 
> -----Original Message-----
> From: Dharmik Thakkar <Dharmik.Thakkar@arm.com>
> Sent: Friday, May 8, 2020 6:37 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Iremonger, Bernard <bernard.iremonger@intel.com>;
> dev@dpdk.org; nd <nd@arm.com>
> Subject: Re: [dpdk-dev] [PATCH 2/5] app/testpmd: print fractional part in CPU cycles
> 
> 
> 
> > On May 8, 2020, at 12:17 PM, Ananyev, Konstantin <konstantin.ananyev@intel.com> wrote:
> >
> >> Hi Konstantin,
> >>
> >>> On May 7, 2020, at 4:50 AM, Ananyev, Konstantin <konstantin.ananyev@intel.com> wrote:
> >>>
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Dharmik Thakkar
> >>>> Sent: Wednesday, May 6, 2020 10:59 PM
> >>>> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Iremonger, Bernard
> <bernard.iremonger@intel.com>
> >>>> Cc: dev@dpdk.org; nd@arm.com; Dharmik Thakkar <dharmik.thakkar@arm.com>
> >>>> Subject: [dpdk-dev] [PATCH 2/5] app/testpmd: print fractional part in CPU cycles
> >>>>
> >>>> Change printing of CPU cycles/packet to include fractional part for
> >>>> accurateness.
> >>>>
> >>>> Example:
> >>>>
> >>>> Without patch:
> >>>> CPU cycles/packet=14
> >>>> (total cycles=4899533541 / total RX packets=343031966)
> >>>>
> >>>> With patch:
> >>>> CPU cycles/packet=14.28
> >>>> (total cycles=4899533541 / total RX packets=343031966)
> >>>>
> >>>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> >>>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> >>>> Reviewed-by: Phil Yang <phil.yang@arm.com>
> >>>> ---
> >>>> app/test-pmd/testpmd.c | 4 ++--
> >>>> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> >>>> index 9a8cbbd6fc7c..9444a730a153 100644
> >>>> --- a/app/test-pmd/testpmd.c
> >>>> +++ b/app/test-pmd/testpmd.c
> >>>> @@ -1955,9 +1955,9 @@ fwd_stats_display(void)
> >>>> #ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
> >>>> #define CYC_PER_MHZ 1E6
> >>>> 	if (total_recv > 0)
> >>>> -		printf("\n  CPU cycles/packet=%u (total cycles="
> >>>> +		printf("\n  CPU cycles/packet=%.2f (total cycles="
> >>>> 		       "%"PRIu64" / total RX packets=%"PRIu64") at %lu MHz Clock\n",
> >>>> -		       (unsigned int)(fwd_cycles / total_recv),
> >>>> +		       (double)(fwd_cycles / (double)total_recv),
> >>>
> >>> Probably safer long double - to avoid overflow.
> >>
> >> Is it possible for a ‘double' to be less than 8 bytes?
> >
> > That was my initial thought - that on some 32 bit systems it could be 4B.
> > Though it seems I was wrong, so feel free to ignore.
> > BTW, what for double conversion, why not just:
> > double)(fwd_cycles /total_recv
> > ?
> 
> Without (double) total recv, I will always get the fractional part as .00.
> For the above example, with (double)(fwd_cycles / total_recv), I see 14.00 instead of 14.28

AFAIK, that's because that way you told compiler to do integer division and then convert result to double.
While (double)fwd_cycles /total_recv, will tell compiler to explicitly convert fwd_cycles to double
and compiler will implicitly do the same for total_recv too, and result will be double too.


> 
> >
> >
> >
> >>
> >>>
> >>>> 		       fwd_cycles, total_recv, (uint64_t)(rte_get_tsc_hz() / CYC_PER_MHZ));
> >>>> #endif
> >>>> }
> >>>> --
> >>>> 2.20.1
diff mbox series

Patch

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 9a8cbbd6fc7c..9444a730a153 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1955,9 +1955,9 @@  fwd_stats_display(void)
 #ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
 #define CYC_PER_MHZ 1E6
 	if (total_recv > 0)
-		printf("\n  CPU cycles/packet=%u (total cycles="
+		printf("\n  CPU cycles/packet=%.2f (total cycles="
 		       "%"PRIu64" / total RX packets=%"PRIu64") at %lu MHz Clock\n",
-		       (unsigned int)(fwd_cycles / total_recv),
+		       (double)(fwd_cycles / (double)total_recv),
 		       fwd_cycles, total_recv, (uint64_t)(rte_get_tsc_hz() / CYC_PER_MHZ));
 #endif
 }