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 3D613A0C44; Mon, 14 Jun 2021 17:48:47 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id AE7104067E; Mon, 14 Jun 2021 17:48:46 +0200 (CEST) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id 27E274067A for ; Mon, 14 Jun 2021 17:48:45 +0200 (CEST) X-MimeOLE: Produced By Microsoft Exchange V6.5 Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Date: Mon, 14 Jun 2021 17:48:43 +0200 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35C61851@smartserver.smartshare.dk> In-Reply-To: <2004320.XGyPsaEoyj@thomas> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [dpdk-dev] [PATCH] parray: introduce internal API for dynamic arrays Thread-Index: AddhIbBpk8RgiGs1RCC4BZ58u5wdxwADwllw References: <20210614105839.3379790-1-thomas@monjalon.net> <98CBD80474FA8B44BF855DF32C47DC35C6184E@smartserver.smartshare.dk> <2004320.XGyPsaEoyj@thomas> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Thomas Monjalon" , "Bruce Richardson" Cc: , , , , , , , Subject: Re: [dpdk-dev] [PATCH] parray: introduce internal API for dynamic arrays 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 Sender: "dev" > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon > Sent: Monday, 14 June 2021 15.32 >=20 > 14/06/2021 15:15, Bruce Richardson: > > On Mon, Jun 14, 2021 at 02:22:42PM +0200, Morten Br=F8rup wrote: > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas > Monjalon > > > > Sent: Monday, 14 June 2021 12.59 > > > > > > > > Performance of access in a fixed-size array is very good > > > > because of cache locality > > > > and because there is a single pointer to dereference. > > > > The only drawback is the lack of flexibility: > > > > the size of such an array cannot be increase at runtime. > > > > > > > > An approach to this problem is to allocate the array at runtime, > > > > being as efficient as static arrays, but still limited to a > maximum. > > > > > > > > That's why the API rte_parray is introduced, > > > > allowing to declare an array of pointer which can be resized > > > > dynamically > > > > and automatically at runtime while keeping a good read > performance. > > > > > > > > After resize, the previous array is kept until the next resize > > > > to avoid crashs during a read without any lock. > > > > > > > > Each element is a pointer to a memory chunk dynamically > allocated. > > > > This is not good for cache locality but it allows to keep the > same > > > > memory per element, no matter how the array is resized. > > > > Cache locality could be improved with mempools. > > > > The other drawback is having to dereference one more pointer > > > > to read an element. > > > > > > > > There is not much locks, so the API is for internal use only. > > > > This API may be used to completely remove some compilation-time > > > > maximums. > > > > > > I get the purpose and overall intention of this library. > > > > > > I probably already mentioned that I prefer "embedded style > programming" with fixed size arrays, rather than runtime > configurability. It's my personal opinion, and the DPDK Tech Board > clearly prefers reducing the amount of compile time configurability, = so > there is no way for me to stop this progress, and I do not intend to > oppose to this library. :-) > > > > > > This library is likely to become a core library of DPDK, so I = think > it is important getting it right. Could you please mention a few > examples where you think this internal library should be used, and > where it should not be used. Then it is easier to discuss if the = border > line between control path and data plane is correct. E.g. this library > is not intended to be used for dynamically sized packet queues that > grow and shrink in the fast path. > > > > > > If the library becomes a core DPDK library, it should probably be > public instead of internal. E.g. if the library is used to make > RTE_MAX_ETHPORTS dynamic instead of compile time fixed, then some > applications might also need dynamically sized arrays for their > application specific per-port runtime data, and this library could > serve that purpose too. > > > > > > > Thanks Thomas for starting this discussion and Morten for follow-up. > > > > My thinking is as follows, and I'm particularly keeping in mind the > cases > > of e.g. RTE_MAX_ETHPORTS, as a leading candidate here. > > > > While I dislike the hard-coded limits in DPDK, I'm also not = convinced > that > > we should switch away from the flat arrays or that we need fully > dynamic > > arrays that grow/shrink at runtime for ethdevs. I would suggest a > half-way > > house here, where we keep the ethdevs as an array, but one > allocated/sized > > at runtime rather than statically. This would allow us to have a > > compile-time default value, but, for use cases that need it, allow > use of a > > flag e.g. "max-ethdevs" to change the size of the parameter given = to > the > > malloc call for the array. This max limit could then be provided to > apps > > too if they want to match any array sizes. [Alternatively those apps > could > > check the provided size and error out if the size has been increased > beyond > > what the app is designed to use?]. There would be no extra > dereferences per > > rx/tx burst call in this scenario so performance should be the same > as > > before (potentially better if array is in hugepage memory, I > suppose). If performance can be improved by allocating array memory differently, = we can just allocate memory differently - dynamically sized arrays are = not required. :-) >=20 > I think we need some benchmarks to decide what is the best tradeoff. While performance is always important, the DPDK community seems willing = to trade in a little bit of performance for obtaining some other great = benefit. I agree with this pragmatic approach. However, the word = "tradeoff" triggered another line of thinking: Regarding this library, we must carefully consider if the benefit is = worth the added complexity. We shouldn't introduce additional complexity = only to save a few MB of memory, and for the pure principle of avoiding = compile time configuration parameters. It would be much simpler to just increase RTE_MAX_ETHPORTS to something = big enough to hold a sufficiently large array. And possibly add an = rte_max_ethports variable to indicate the number of populated entries in = the array, for use when iterating over the array. Can we come up with another example than RTE_MAX_ETHPORTS where this = library provides a better benefit? > I spent time on this implementation, but sorry I won't have time for > benchmarks. > Volunteers?