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 12C8CA0C44; Mon, 14 Jun 2021 14:22:46 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id EF67E4067A; Mon, 14 Jun 2021 14:22:45 +0200 (CEST) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id 139D34003F for ; Mon, 14 Jun 2021 14:22: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 14:22:42 +0200 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35C6184E@smartserver.smartshare.dk> In-Reply-To: <20210614105839.3379790-1-thomas@monjalon.net> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [dpdk-dev] [PATCH] parray: introduce internal API for dynamic arrays Thread-Index: AddhDE/ekfdVxSC6SfWTgHzQUwpP9wABS3yw References: <20210614105839.3379790-1-thomas@monjalon.net> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Thomas Monjalon" , 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 12.59 >=20 > 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. >=20 > An approach to this problem is to allocate the array at runtime, > being as efficient as static arrays, but still limited to a maximum. >=20 > 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. >=20 > After resize, the previous array is kept until the next resize > to avoid crashs during a read without any lock. >=20 > 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. >=20 > 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. [snip] > + > +/** Main object representing a dynamic array of pointers. */ > +struct rte_parray { > + /** Array of pointer to dynamically allocated struct. */ > + void **array; > + /** Old array before resize, freed on next resize. */ > + void **old_array; > + /* Lock for alloc/free operations. */ > + pthread_mutex_t mutex; > + /** Current size of the full array. */ > + int32_t size; > + /** Number of allocated elements. */ > + int32_t count; > + /** Last allocated element. */ > + int32_t last; > +}; Why not uint32_t for size, count and last? Consider if the hot members of the struct should be moved closer = together, for increasing the probability that they end up in the same = cache line if the structure is not cache line aligned. Probably not = important, just wanted to mention it. -Morten