[dpdk-dev] [PATCH v2 3/3] app/testpmd: qualify profiling statistics on burst size
Ferruh Yigit
ferruh.yigit at intel.com
Thu Apr 9 13:46:09 CEST 2020
On 3/20/2020 6:13 AM, Jerin Jacob wrote:
> On Thu, Mar 19, 2020 at 7:21 PM Viacheslav Ovsiienko
> <viacheslavo at mellanox.com> wrote:
>>
>> The execution time of rx/tx burst routine depends on the burst size.
>> It would be meaningful to research this dependency, the patch
>> provides an extra profiling data per rx/tx burst size.
>>
>> Signed-off-by: Viacheslav Ovsiienko <viacheslavo at mellanox.com>
>
>> +#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
>> + if (!(fwdprof_flags & (nrx_tx ? RECORD_CORE_CYCLES_TX
>> + : RECORD_CORE_CYCLES_RX)))
>> + return;
>> + for (nb_pkt = 0; nb_pkt < MAX_PKT_BURST; nb_pkt++) {
>> + nb_burst = nrx_tx ? pbs->pkt_retry_spread[nb_pkt]
>> + : pbs->pkt_burst_spread[nb_pkt];
>> + if (nb_burst == 0)
>> + continue;
>> + printf(" CPU cycles/%u packet burst=%u (total cycles="
>> + "%"PRIu64" / total %s bursts=%u)\n",
>> + (unsigned int)nb_pkt,
>> + (unsigned int)(pbs->pkt_ticks_spread[nb_pkt] / nb_burst),
>> + pbs->pkt_ticks_spread[nb_pkt],
>> + nrx_tx ? "TX" : "RX", nb_burst);
>> + }
>> +#endif
>
>
> # Thanks for this feature, IMO, It worth to mention in release notes.
>
> # I see a lot of code get added under RTE_TEST_PMD_RECORD_CORE_CYCLES.
> I agree that it should be under RTE_TEST_PMD_RECORD_CORE_CYCLES. Since
> this flag is not
> by default, there is a risk of compilation issue when this flag is get enabled.
> IMO, it is better to move to _if (0)_ semantics to enable
> - performance when compiler opt-out the code.
> - It forces the compiler to check the compilation errors irrespective
> of the RTE_TEST_PMD_RECORD_CORE_CYCLES state.
>
> Something like below,
>
> static __rte_always_inline int
> testpmd_has_stats_feature(void)
> {
> #ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
> return RTE_TEST_PMD_RECORD_CORE_CYCLES ;
> #else
> return 0;
> #endif
> }
>
>
> if (testpmd_has_stats_feature()) {
>
> }
>
Hi Jerin,
In this usage, compiler will removed the "if (0) { }" block, right?
I think this is good idea, we can use it other places too, including this one.
More information about the dev
mailing list