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()) { }