14/06/2021 14:22, Morten Brørup:
> > From: dev [mailto:dev-boun...@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. :-)
Embedded-style is highly customized and limited.
DPDK is more used in standard servers where
deployment must be easy and dynamically configurable.
That's my view on where we go, but I understand
some can have opposite goals. Thus the discussion :)
> 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,
It could be used for device arrays which are managed
(alloc/free) in the main thread as part of init
and hotplug operations.
Other threads should be readers only.
> 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.
Correct.
If fast path threads need to alloc/free, this is not the right API.
That's not a queue, just a growing array where each element has an index.
> 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.
It could be convenient but risky if users don't understand well
the limitations. I am not sure what to do.
> [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?
2 reasons:
1/ anyway we are limited to int32_t for the index.
2/ having the same type for all avoid compiler complaining
when comparing values.
> 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.
The only hot member is the array itself.
Depending on mutex implementation, all could be in a single cacheline.
