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 E1F42A0C44; Mon, 14 Jun 2021 17:49:10 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C8D114068C; Mon, 14 Jun 2021 17:49:10 +0200 (CEST) Received: from mail-io1-f51.google.com (mail-io1-f51.google.com [209.85.166.51]) by mails.dpdk.org (Postfix) with ESMTP id E32724067A for ; Mon, 14 Jun 2021 17:49:08 +0200 (CEST) Received: by mail-io1-f51.google.com with SMTP id k5so30293043iow.12 for ; Mon, 14 Jun 2021 08:49:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=43PdKOSVIi0NVwBEhAaH4H5PmQm7jm7BtQsV8wJ7Fo8=; b=QVUoo3x+o2R9/RwK6qNFXt6AaleJp+KGUEKxGcFxWKBMEnHzND6vo3h54HHXILN91F Hi+AiLXtxiGuP1+xVqpbz4KskF3Xm7EpRaHNiXRqpxrhyS2gS0xabzLoAE+KfE6W5jxz Ijc969FBXZajNW/i8dsw0EQQJ1Mnf2Kut75QPmIHfahOBcNSMRp/FE8ndR4U3P84ZH56 VFIqYe8TL0UyhrxQq3q8FBCeflyROdmS/3QzZrdpitLYxrBC+XM14YK/ZSqIZ1bp+HeE meVvBAOGQ/rQqQNvW7yr0+smk7/fhnH7vIcnHYoV3oqS8gcIOPXaEp9Asawfkl0csOFE 2qYw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=43PdKOSVIi0NVwBEhAaH4H5PmQm7jm7BtQsV8wJ7Fo8=; b=FW0vUi75pGWPsJhyQCPzfpuo1ykpC/zpNhDt/yQZiiR7F3HfFR7SU5+pSZQXqeUTs4 CbFKSVZzTZ8ncKhx7wrarwdTjMhmdJ7IYYrIA9EiMnjqOe5T8vFJiOiKpmwAVWJPmpSE K9TxRjLcRtZ4GzDNq7ZWzj0yb3zEeatqOOP95V05BaYTfnZ2yp4UikxnV4pnEfrymgxl uce/YQ4+uVpLm24X0CsOpgyr8wXzUzylCD3yKAY1sWGQLsf+5XrZ/GiOVkZzywFYFhFM zaE29SIY/H5McWRA/Mu5rk5Eb/LSbTB50iXxHoxYhvRJSbZkCIckXQyCxMgmbCBhVqlp r+QA== X-Gm-Message-State: AOAM532r8X66Po4zC60aN8RjpZhqt3H7sOtQYiiSGe5U1E0jfWG/k5Xn bfpwSQCkBsd1UgSt8wPB7B7sXOoVs4E2nVmFz24= X-Google-Smtp-Source: ABdhPJzVZqHTG/dVqwrHZBABvKpJoBrJHWjVXszax6tuqh094kEQVZOUvShI9DEvtHyAvBFVssFbjTZ8+3Gzf8liSDE= X-Received: by 2002:a05:6602:2203:: with SMTP id n3mr14801532ion.1.1623685748187; Mon, 14 Jun 2021 08:49:08 -0700 (PDT) MIME-Version: 1.0 References: <20210614105839.3379790-1-thomas@monjalon.net> <98CBD80474FA8B44BF855DF32C47DC35C6184E@smartserver.smartshare.dk> <2004320.XGyPsaEoyj@thomas> In-Reply-To: From: Jerin Jacob Date: Mon, 14 Jun 2021 21:18:52 +0530 Message-ID: To: "Ananyev, Konstantin" Cc: Thomas Monjalon , "Richardson, Bruce" , =?UTF-8?Q?Morten_Br=C3=B8rup?= , "dev@dpdk.org" , "olivier.matz@6wind.com" , "andrew.rybchenko@oktetlabs.ru" , "honnappa.nagarahalli@arm.com" , "Yigit, Ferruh" , "jerinj@marvell.com" , "gakhil@marvell.com" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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" On Mon, Jun 14, 2021 at 8:29 PM Ananyev, Konstantin wrote: > > > > > > 14/06/2021 15:15, Bruce Richardson: > > > On Mon, Jun 14, 2021 at 02:22:42PM +0200, Morten Br=C3=B8rup wrote: > > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monja= lon > > > > > 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 maxim= um. > > > > > > > > > > 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 performanc= e. > > > > > > > > > > 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 sam= e > > > > > 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 programm= ing" with fixed size arrays, rather than runtime configurability. It's > > my personal opinion, and the DPDK Tech Board clearly prefers reducing t= he 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 shou= ld 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 intend= ed 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 p= ublic instead of internal. E.g. if the library is used to make > > RTE_MAX_ETHPORTS dynamic instead of compile time fixed, then some appli= cations might also need dynamically sized arrays for their > > application specific per-port runtime data, and this library could serv= e 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 c= ases > > > 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 dyna= mic > > > arrays that grow/shrink at runtime for ethdevs. I would suggest a hal= f-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 us= e 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 dereferenc= es per > > > rx/tx burst call in this scenario so performance should be the same a= s > > > before (potentially better if array is in hugepage memory, I suppose)= . > > > > I think we need some benchmarks to decide what is the best tradeoff. > > I spent time on this implementation, but sorry I won't have time for be= nchmarks. > > Volunteers? > > I had only a quick look at your approach so far. > But from what I can read, in MT environment your suggestion will require > extra synchronization for each read-write access to such parray element (= lock, rcu, ...). > I think what Bruce suggests will be much ligther, easier to implement and= less error prone. > At least for rte_ethdevs[] and friends. +1 > Konstantin > >