[dpdk-dev] [PATCH] parray: introduce internal API for dynamic arrays
Burakov, Anatoly
anatoly.burakov at intel.com
Wed Jun 16 13:11:13 CEST 2021
On 14-Jun-21 11:58 AM, Thomas Monjalon wrote:
> 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.
>
> Signed-off-by: Thomas Monjalon <thomas at monjalon.net>
> ---
<snip>
> +int32_t
> +rte_parray_find_next(struct rte_parray *obj, int32_t index)
> +{
> + if (obj == NULL || index < 0) {
> + rte_errno = EINVAL;
> + return -1;
> + }
> +
> + pthread_mutex_lock(&obj->mutex);
> +
> + while (index < obj->size && obj->array[index] == NULL)
> + index++;
> + if (index >= obj->size)
> + index = -1;
> +
> + pthread_mutex_unlock(&obj->mutex);
> +
> + rte_errno = 0;
> + return index;
> +}
> +
Just a general comment about this:
I'm not really sure i like this "kinda-sorta-threadsafe-but-not-really"
approach. IMO something either should be thread-safe, or it should be
explicitly not thread-safe. There's no point in locking here because any
user of find_next() will *necessarily* race with other users, because by
the time we exit the function, the result becomes stale - so why are we
locking in the first place?
Would be perhaps be better to leave it as non-thread-safe at its core,
but introduce wrappers for atomic-like access to the array? E.g.
something like `rte_parray_find_next_free_and_set()` that will perform
the lock-find-next-set-unlock sequence? Or, alternatively, have the
mutex there, but provide API's for explicit locking, and put the burden
on the user to actually do the locking correctly.
--
Thanks,
Anatoly
More information about the dev
mailing list