Hi Bruce, Slava, > -----Original Message----- > From: Slava Ovsiienko [mailto:[email protected]] > Sent: Thursday, June 27, 2019 5:49 AM > To: Richardson, Bruce <[email protected]> > Cc: [email protected]; Iremonger, Bernard <[email protected]>; > Yigit, Ferruh <[email protected]> > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: add profiling for Rx/Tx burst > routines > > OK, what do you think about this: > > #ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES > if (record_cycle & RECORD_TX_CORE_CYCLES) { > .. do measurement stuff .. > } > #endif > > + add some new command to config in runtime: "set record_cycle 3" > > We keep existing RTE_TEST_PMD_RECORD_CORE_CYCLES, do not introduce > new build-time configs and get some new runtime configuring. > > WBR, > Slava > > > -----Original Message----- > > From: Bruce Richardson <[email protected]> > > Sent: Wednesday, June 26, 2019 16:21 > > To: Slava Ovsiienko <[email protected]> > > Cc: [email protected]; [email protected]; [email protected] > > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: add profiling for Rx/Tx > > burst routines > > > > On Wed, Jun 26, 2019 at 01:19:24PM +0000, Slava Ovsiienko wrote: > > > Hi, Bruce > > > > > > Do you mean using "if (core_rx_cycle_enabled) {...}" instead of #ifdef ? > > > > > > No, I did not try runtime control settings. > > > Instead I compared performance with all RECORD_CORE_XX_CYCLES > > options > > > enabled/disabled builds and have seen the ~1-2% performance > > > difference > > on my setups (mainly fwd txonly with retry). > > > So, ticks measuring is not free. > > > > > > With best regards, > > > Slava > > > > > Yes, I realise that measuring ticks is going to have a performance impact. > > However, what I was referring to was exactly the former - using an "if" > > rather than an "ifdef". I would hope with ticks disable using this > > option shows no perf impact, and we can reduce the use of build-time > configs. > > > > /Bruce
Given that RTE_TEST_PMD_RECORD_CORE_CYCLES is already in the config file. I think it is better to be consistent and add the new RECORD macros there. Would it be reasonable to have runtime settings available as well? Regards, Bernard
