From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 7DA28A046B for ; Fri, 28 Jun 2019 16:20:21 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 40C9E37A8; Fri, 28 Jun 2019 16:20:20 +0200 (CEST) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 618ED3798 for ; Fri, 28 Jun 2019 16:20:18 +0200 (CEST) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 28 Jun 2019 07:20:17 -0700 X-IronPort-AV: E=Sophos;i="5.63,427,1557212400"; d="scan'208";a="361528319" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.21.39]) by fmsmga006-auth.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 28 Jun 2019 07:20:16 -0700 Date: Fri, 28 Jun 2019 15:20:13 +0100 From: Bruce Richardson To: "Iremonger, Bernard" Cc: Slava Ovsiienko , "dev@dpdk.org" , "Yigit, Ferruh" Message-ID: <20190628142012.GA366@bricha3-MOBL.ger.corp.intel.com> References: <1561553317-16777-1-git-send-email-viacheslavo@mellanox.com> <20190626125732.GC862@bricha3-MOBL.ger.corp.intel.com> <20190626132124.GD862@bricha3-MOBL.ger.corp.intel.com> <8CEF83825BEC744B83065625E567D7C260DBE4EB@IRSMSX108.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <8CEF83825BEC744B83065625E567D7C260DBE4EB@IRSMSX108.ger.corp.intel.com> User-Agent: Mutt/1.11.4 (2019-03-13) Subject: Re: [dpdk-dev] [PATCH] app/testpmd: add profiling for Rx/Tx burst routines X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Fri, Jun 28, 2019 at 02:45:13PM +0100, Iremonger, Bernard wrote: > Hi Bruce, Slava, > > > -----Original Message----- > > From: Slava Ovsiienko [mailto:viacheslavo@mellanox.com] > > Sent: Thursday, June 27, 2019 5:49 AM > > To: Richardson, Bruce > > Cc: dev@dpdk.org; Iremonger, Bernard ; > > Yigit, Ferruh > > 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 > > > Sent: Wednesday, June 26, 2019 16:21 > > > To: Slava Ovsiienko > > > Cc: dev@dpdk.org; bernard.iremonger@intel.com; ferruh.yigit@intel.com > > > 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? > That configuration option is only present right now for the make builds, so I'd like to see it replaced with a runtime option rather than see about adding more config options to the meson build. The first step should be to avoid adding more config options and just look to use dynamic ones. Ideally the existing build option should be replaced at the same time. /Bruce