From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id C6604A0093; Tue, 8 Nov 2022 14:32:32 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7C999400D4; Tue, 8 Nov 2022 14:32:32 +0100 (CET) Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by mails.dpdk.org (Postfix) with ESMTP id 9E50D4003C for ; Tue, 8 Nov 2022 14:32:30 +0100 (CET) Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailout.nyi.internal (Postfix) with ESMTP id 517835C013B; Tue, 8 Nov 2022 08:32:30 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute2.internal (MEProxy); Tue, 08 Nov 2022 08:32:30 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:cc:content-transfer-encoding:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to; s=fm1; t=1667914350; x= 1668000750; bh=M2vLWYD0EE0C5OFwIfqMZCXvld6o46rbG7rDGQok/f0=; b=c gHVi4Pv2iQ830EvcCf/Kf3dvxh4QrEGD2pfoeHje2CpkrrpcueT0PHD2vprEBdCT YDd+crY06AZlCCRGIvhJDi9/8172ZHdohOL8Ohyl6SwCgGOdBxWgZ029pIETAn6g zdaDPuXmP55GAGL27f0BbT+xm9FQpGi3MfucEp+Ew4cxEO/k3yqDFB6lS7IsNNgI d9cacF4asp3KOuCaoWN/h3BE1wIEjDP93UYTpI7pqKIvNft2+8NEmXaHIQ5d6D38 0jO9DhYQHrOzU8UK0PCwDEcC1Ft0r820ygMRkRrGNXo33Xkr0Pr7Et+lP5wep8oR TAUz/g96PMjvT5Ix0ppwg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:date:date:feedback-id:feedback-id:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1667914350; x= 1668000750; bh=M2vLWYD0EE0C5OFwIfqMZCXvld6o46rbG7rDGQok/f0=; b=Z 77wY/j9os1SKREMTq1+8uUAUGZ+1w0CYqwE8Fk4n5jvanoL7QRBqLwQKoaYBgQPx JoObNy7tP2lsjemCl2RIDmmXsAAHAOCK7KS65GRiZpYcUUwhOPgYSTod8I4JhhWV W27dy9/bmG3mRbBrzrPbeoyolNfM8csC66cO0R8ta62IenkOCJFHKpj98ISRQ8F/ TRG1F2U9P3OgYg3RqAHJDj6FNOjuK7fXGpzDYwcHEc5E9J/T714EjkwZilZk4PUu twPm+ZJq/2kybYIrf0YEQ6HfmEBsqEzz710PK3b4nw6h5208EVuhIycX3gY4UHBr MCWV695DkpYxa5MViXlAw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvgedrfedtgdehgecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvfevufffkfgjfhgggfgtsehtqhertddttddunecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg ftrfgrthhtvghrnhepfefhjeeluedvvedtuddtuedtvefhieejtefhffeujefhteduudev tdektdeikeffnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrh homhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth X-ME-Proxy: Feedback-ID: i47234305:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 8 Nov 2022 08:32:29 -0500 (EST) From: Thomas Monjalon To: Morten =?ISO-8859-1?Q?Br=F8rup?= Cc: david.marchand@redhat.com, dev@dpdk.org, andrew.rybchenko@oktetlabs.ru, olivier.matz@6wind.com Subject: Re: FW: [PATCH v4 3/3] mempool: use cache for frequently updated stats Date: Tue, 08 Nov 2022 14:32:27 +0100 Message-ID: <6110999.17fYzF0512@thomas> In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35D874A1@smartserver.smartshare.dk> References: <98CBD80474FA8B44BF855DF32C47DC35D874A1@smartserver.smartshare.dk> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="iso-8859-1" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 08/11/2022 12:25, Morten Br=F8rup: > From: Morten Br=F8rup=20 > Sent: Tuesday, 8 November 2022 12.22 >=20 > > From: Konstantin Ananyev [mailto:konstantin.ananyev@huawei.com] > > Sent: Tuesday, 8 November 2022 10.20 > >=20 > > > When built with stats enabled (RTE_LIBRTE_MEMPOOL_STATS defined), the > > > performance of mempools with caches is improved as follows. > > > > > > When accessing objects in the mempool, either the put_bulk and > > put_objs or > > > the get_success_bulk and get_success_objs statistics counters are > > likely > > > to be incremented. > > > > > > By adding an alternative set of these counters to the mempool cache > > > structure, accessing the dedicated statistics structure is avoided in > > the > > > likely cases where these counters are incremented. > > > > > > The trick here is that the cache line holding the mempool cache > > structure > > > is accessed anyway, in order to access the 'len' or 'flushthresh' > > fields. > > > Updating some statistics counters in the same cache line has lower > > > performance cost than accessing the statistics counters in the > > dedicated > > > statistics structure, which resides in another cache line. > > > > > > mempool_perf_autotest with this patch shows the following > > improvements in > > > rate_persec. > > > > > > The cost of enabling mempool stats (without debug) after this patch: > > > -6.8 % and -6.7 %, respectively without and with cache. > > > > > > v4: > > > * Fix checkpatch warnings: > > > A couple of typos in the patch description. > > > The macro to add to a mempool cache stat variable should not use > > > do {} while (0). Personally, I would tend to disagree with this, > > but > > > whatever keeps the CI happy. > > > v3: > > > * Don't update the description of the RTE_MEMPOOL_STAT_ADD macro. > > > This change belongs in the first patch of the series. > > > v2: > > > * Move the statistics counters into a stats structure. > > > > > > Signed-off-by: Morten Br=F8rup > > > --- >=20 > [...] >=20 > > > +/** > > > + * @internal When stats is enabled, store some statistics. > > > + * > > > + * @param cache > > > + * Pointer to the memory pool cache. > > > + * @param name > > > + * Name of the statistics field to increment in the memory pool > > cache. > > > + * @param n > > > + * Number to add to the statistics. > > > + */ > > > +#ifdef RTE_LIBRTE_MEMPOOL_STATS > > > +#define RTE_MEMPOOL_CACHE_STAT_ADD(cache, name, n) (cache)->stats.na= me +=3D n > >=20 > > As Andrew already pointed, it needs to be: ((cache)->stats.name +=3D (n= )) > > Apart from that, LGTM. > > Series-Acked-by: Konstantin Ananyev >=20 > @Thomas, this series should be ready to apply... it now has been: > Reviewed-by: (mempool maintainer) Andrew Rybchenko > Reviewed-By: Mattias R=F6nnblom > Acked-by: Konstantin Ananyev Being acked does not mean it is good to apply in -rc3. Please tell what is the benefit for 22.11 (before/after and condition). Note there is a real risk doing such change that late. > Please fix the RTE_MEMPOOL_CACHE_STAT_ADD macro while merging, to satisfy= checkpatch. ;-) >=20 > It should be: >=20 > +#ifdef RTE_LIBRTE_MEMPOOL_STATS > +#define RTE_MEMPOOL_CACHE_STAT_ADD(cache, name, n) ((cache)->stats.name = +=3D (n)) > +#else > +#define RTE_MEMPOOL_CACHE_STAT_ADD(cache, name, n) do {} while (0) > +#endif Would be easier if you fix it. > @Thomas/@David: I changed the state of this patch series to Awaiting Upst= ream in patchwork. Is that helpful, or should I change them to some other s= tate? You should keep it as "New".