On Thu, Apr 9, 2020 at 5:16 PM Ferruh Yigit <ferruh.yi...@intel.com> wrote: > > On 3/20/2020 6:13 AM, Jerin Jacob wrote: > > On Thu, Mar 19, 2020 at 7:21 PM Viacheslav Ovsiienko > > <viachesl...@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 <viachesl...@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,
Hi Ferruh, > In this usage, compiler will removed the "if (0) { }" block, right? Yes. > I think this is good idea, we can use it other places too, including this one. Yes.