On 8/26/2020 6:07 PM, Dharmik Thakkar wrote: > > >> On Aug 26, 2020, at 11:41 AM, Bruce Richardson <bruce.richard...@intel.com> >> wrote: >> >> On Wed, Aug 26, 2020 at 05:33:20PM +0100, Ferruh Yigit wrote: >>> On 7/14/2020 10:51 PM, Dharmik Thakkar wrote: >>>> Meson build system lacks support for >>>> CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and >>>> CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS configuration options. >>>> >>>> One solution is to add these options within meson_options.txt >>>> >>>> Since adding these as runtime config causes no performance impact, >>> >>> Hi Dharmik, >>> >>> These are on the datapath, and even disable there will be additional >>> checks, isn't it expected to have some impact? Did you do any >>> measurements for it? > > Hi Ferruh, > > In my measurements, I saw a maximum performance degradation of 0.3% for rx > and tx throughput (pps) > with runtime option (both disabled and enabled cases) when compared to > compile time option.. > There was no difference in cycles per packet measurement. > I did these measurements with Mellanox-ConnectX-5 card on N1SDP server with > ’set fwd mac retry’ option. >
Thanks Dharmik, 0.3% does not look a lot, I also don't get any recognizable drop when the stats are disabled (there is a drop (up to %15) when stats are enabled but that is OK). And there is a benefit to have the stats runtime configurable. This minor drop for runtime configuration looks OK to me, but lets wait a little more if there is any strong objection to it, we can proceed afterwards. > >>> >> Branches that always predict the same way can be very cheap, and unless >> proven to be a problem, I'd see no issue with having a few on the datapath >> - especially if it just one or two per burst. If we start seeing a >> significant number, or ones that occur for every packet, then we perhaps >> need to be more cautious. >> >> I also think that using lots of different CFLAGS for turning things on and >> off is as bad - or even worse - than using the old build-time config >> options, just this time we don't have a single list of them somewhere! >> Therefore, I think we really need to start converting these to runtime >> options where we can. >> >> Regards, >> /Bruce > > >