DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Morten Brørup" <mb@smartsharesystems.com>
To: "Thomas Monjalon" <thomas@monjalon.net>, <dev@dpdk.org>
Cc: <olivier.matz@6wind.com>, <andrew.rybchenko@oktetlabs.ru>,
	<honnappa.nagarahalli@arm.com>, <konstantin.ananyev@intel.com>,
	<bruce.richardson@intel.com>, <ferruh.yigit@intel.com>,
	<jerinj@marvell.com>, <gakhil@marvell.com>
Subject: Re: [dpdk-dev] [PATCH] parray: introduce internal API for dynamic arrays
Date: Mon, 14 Jun 2021 14:22:42 +0200	[thread overview]
Message-ID: <98CBD80474FA8B44BF855DF32C47DC35C6184E@smartserver.smartshare.dk> (raw)
In-Reply-To: <20210614105839.3379790-1-thomas@monjalon.net>

> 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.

[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


  reply	other threads:[~2021-06-14 12:22 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-14 10:58 Thomas Monjalon
2021-06-14 12:22 ` Morten Brørup [this message]
2021-06-14 13:15   ` Bruce Richardson
2021-06-14 13:32     ` Thomas Monjalon
2021-06-14 14:59       ` Ananyev, Konstantin
2021-06-14 15:48         ` Jerin Jacob
2021-06-15  6:52           ` Thomas Monjalon
2021-06-15  8:00             ` Jerin Jacob
2021-06-15  9:18               ` Thomas Monjalon
2021-06-15  9:33             ` Ananyev, Konstantin
2021-06-15  9:50               ` Thomas Monjalon
2021-06-15 10:08                 ` Ananyev, Konstantin
2021-06-15 14:02                   ` Thomas Monjalon
2021-06-15 14:37                     ` Honnappa Nagarahalli
2021-06-14 15:54         ` Ananyev, Konstantin
2021-06-17 13:08           ` Ferruh Yigit
2021-06-17 14:58             ` Ananyev, Konstantin
2021-06-17 15:17               ` Morten Brørup
2021-06-17 16:12                 ` Ferruh Yigit
2021-06-17 16:55                   ` Morten Brørup
2021-06-18 10:21                     ` Ferruh Yigit
2021-06-17 17:05                   ` Ananyev, Konstantin
2021-06-18  9:14                     ` Morten Brørup
2021-06-18 10:47                       ` Ferruh Yigit
2021-06-18 11:16                         ` Morten Brørup
2021-06-18 10:28                     ` Ferruh Yigit
2021-06-17 15:44               ` Ferruh Yigit
2021-06-18 10:41                 ` Ananyev, Konstantin
2021-06-18 10:49                   ` Ferruh Yigit
2021-06-21 11:06                   ` Ananyev, Konstantin
2021-06-21 12:10                     ` Morten Brørup
2021-06-21 12:30                       ` Ananyev, Konstantin
2021-06-21 13:28                         ` Morten Brørup
     [not found]                           ` <DM6PR11MB4491D4F6FAFDD6E8EEC2A78F9A099@DM6PR11MB4491.namprd11.prod.outlook .com>
2021-06-22  8:33                           ` Ananyev, Konstantin
2021-06-22 10:01                             ` Morten Brørup
2021-06-22 12:13                               ` Ananyev, Konstantin
2021-06-22 13:18                                 ` Morten Brørup
2021-06-21 14:10                         ` Ferruh Yigit
2021-06-21 14:38                           ` Ananyev, Konstantin
2021-06-21 15:56                             ` Ferruh Yigit
2021-06-21 18:17                               ` Ananyev, Konstantin
2021-06-21 14:05                     ` Ferruh Yigit
2021-06-21 14:42                       ` Ananyev, Konstantin
2021-06-21 15:32                         ` Ferruh Yigit
2021-06-21 15:37                           ` Ananyev, Konstantin
2021-06-14 15:48       ` Morten Brørup
2021-06-15  6:48         ` Thomas Monjalon
2021-06-15  7:53           ` Morten Brørup
2021-06-15  8:44             ` Bruce Richardson
2021-06-15  9:28               ` Thomas Monjalon
2021-06-16  9:42           ` Jerin Jacob
2021-06-16 11:27             ` Morten Brørup
2021-06-16 12:00               ` Jerin Jacob
2021-06-16 13:02               ` Bruce Richardson
2021-06-16 15:01                 ` Morten Brørup
2021-06-16 17:40                   ` Bruce Richardson
2021-06-16 12:22             ` Burakov, Anatoly
2021-06-16 12:59               ` Jerin Jacob
2021-06-16 22:58                 ` Dmitry Kozlyuk
2021-06-14 13:28   ` Thomas Monjalon
2021-06-16 11:11 ` Burakov, Anatoly

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=98CBD80474FA8B44BF855DF32C47DC35C6184E@smartserver.smartshare.dk \
    --to=mb@smartsharesystems.com \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=gakhil@marvell.com \
    --cc=honnappa.nagarahalli@arm.com \
    --cc=jerinj@marvell.com \
    --cc=konstantin.ananyev@intel.com \
    --cc=olivier.matz@6wind.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).