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 82C3842C4C; Wed, 7 Jun 2023 10:03:52 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 25CD140A84; Wed, 7 Jun 2023 10:03:52 +0200 (CEST) Received: from dkmailrelay1.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id C5B9C40698 for ; Wed, 7 Jun 2023 10:03:50 +0200 (CEST) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesys.local [192.168.4.10]) by dkmailrelay1.smartsharesystems.com (Postfix) with ESMTP id 9AD2620266; Wed, 7 Jun 2023 10:03:49 +0200 (CEST) Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Subject: RE: [PATCH] mempool: optimize get objects with constant n X-MimeOLE: Produced By Microsoft Exchange V6.5 Date: Wed, 7 Jun 2023 10:03:48 +0200 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D87995@smartserver.smartshare.dk> In-Reply-To: <1739067.KUTt5R2Mg1@thomas> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH] mempool: optimize get objects with constant n Thread-Index: AdmZFOh3OMjDTvXMSTStpAo2c1WgMgAAB22Q References: <20230411064845.37713-1-mb@smartsharesystems.com> <98CBD80474FA8B44BF855DF32C47DC35D8788D@smartserver.smartshare.dk> <1739067.KUTt5R2Mg1@thomas> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Thomas Monjalon" , "Bruce Richardson" Cc: , , 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 > From: Thomas Monjalon [mailto:thomas@monjalon.net] > Sent: Wednesday, 7 June 2023 09.52 >=20 > 18/04/2023 14:55, Bruce Richardson: > > On Tue, Apr 18, 2023 at 01:29:49PM +0200, Morten Br=F8rup wrote: > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > > > > On Tue, Apr 11, 2023 at 08:48:45AM +0200, Morten Br=F8rup wrote: > > > > > + if (__extension__(__builtin_constant_p(n)) && n <=3D = cache->len) { > > > > > + /* > > > > > + * The request size is known at build time, and > > > > > + * the entire request can be satisfied from the cache, > > > > > + * so let the compiler unroll the fixed length copy loop. > > > > > + */ > > > > > + cache->len -=3D n; > > > > > + for (index =3D 0; index < n; index++) > > > > > + *obj_table++ =3D *--cache_objs; > > > > > + > > > > > > > > This loop looks a little awkward to me. Would it be clearer (and = perhaps > > > > easier for compilers to unroll efficiently if it was rewritten = as: > > > > > > > > cache->len -=3D n; > > > > cache_objs =3D &cache->objs[cache->len]; > > > > for (index =3D 0; index < n; index++) > > > > obj_table[index] =3D cache_objs[index]; > > > > > > The mempool cache is a stack, so the copy loop needs get the = objects in > decrementing order. I.e. the source index decrements and the = destination index > increments. > > > > > > > BTW: Please add this as a comment in the code too, above the loop to = avoid > > future developers (or even future me), asking this question again! >=20 > Looks like this request was missed. I intentionally omitted it, because I disagree with the suggestion. Otherwise, reminders that the mempool cache is a stack should be = plastered all over the source code, not just here. For reference, this = copy loop (without such a reminder) also exists elsewhere in the same = file. Apologies for not responding to Bruce's request earlier.