From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Carrillo, Erik G" Subject: Re: [PATCH v2 1/3] timer: add per-installer pending lists for each lcore Date: Tue, 29 Aug 2017 16:13:10 +0000 Message-ID: References: <1503499644-29432-2-git-send-email-erik.g.carrillo@intel.com> <1503692823-16214-1-git-send-email-erik.g.carrillo@intel.com> <2601191342CEEE43887BDE71AB9772584F23D8BE@IRSMSX103.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: "dev@dpdk.org" , Stephen Hemminger To: "Ananyev, Konstantin" , "rsanford@akamai.com" Return-path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id A8A31100F for ; Tue, 29 Aug 2017 18:14:19 +0200 (CEST) In-Reply-To: <2601191342CEEE43887BDE71AB9772584F23D8BE@IRSMSX103.ger.corp.intel.com> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Konstantin, > -----Original Message----- > From: Ananyev, Konstantin > Sent: Tuesday, August 29, 2017 5:57 AM > To: Carrillo, Erik G ; rsanford@akamai.com > Cc: dev@dpdk.org > Subject: RE: [dpdk-dev] [PATCH v2 1/3] timer: add per-installer pending l= ists > for each lcore >=20 > Hi Gabriel, >=20 > > > > Instead of each priv_timer struct containing a single skiplist, this > > commit adds a skiplist for each enabled lcore to priv_timer. In the > > case that multiple lcores repeatedly install timers on the same target > > lcore, this change reduces lock contention for the target lcore's > > skiplists and increases performance. >=20 > I am not an rte_timer expert, but there is one thing that worries me: > It seems that complexity of timer_manage() has increased with that patch > quite a bit: > now it has to check/process up to RTE_MAX_LCORE skiplists instead of one= , > also it has to somehow to properly sort up to RTE_MAX_LCORE lists of > retrieved (ready to run) timers. > Wouldn't all that affect it's running time? >=20 Yes, it would indeed increase it. > I understand your intention to reduce lock contention, but I suppose at l= east > it could be done in a configurable way. > Let say allow user to specify dimension of pending_lists[] at init phase= or so. > Then timer from lcore_id=3DN will endup in > pending_lists[N%RTE_DIM(pendilng_list)]. >=20 This is a neat idea, and seemingly would allow the original performance to = be maintained for applications where contention is not an issue. I'll loo= k into this change, as it may address other developers' concerns as well - = thanks for the suggestion. > Another thought - might be better to divide pending timers list not by cl= ient > (lcore) id, but by expiration time - some analog of timer wheel or so. > That, I think might greatly decrease the probability that timer_manage() = and > timer_add() will try to access the same list. > From other side timer_manage() still would have to consume skip-lists one > by one. > Though I suppose that's quite radical change from what we have right now. > Konstantin >=20 Thanks, Gabriel