[FFmpeg-devel] [PATCH 1/3] avutil: add staticpool

[Muhammad Faiz]

On Sat, Jan 20, 2018 at 5:18 PM, wm4 <nfxjfg at googlemail.com> wrote:
> On Sat, 20 Jan 2018 12:52:46 +0700
> Muhammad Faiz <mfcc64 at gmail.com> wrote:
>
>> On Sat, Jan 20, 2018 at 11:49 AM, James Almer <jamrial at gmail.com> wrote:
>> > On 1/20/2018 1:29 AM, Muhammad Faiz wrote:
>> >> Help avoiding malloc-free cycles when allocating-freeing common
>> >> structures.
>> >>
>> >> Signed-off-by: Muhammad Faiz <mfcc64 at gmail.com>
>> >> ---
>> >>  libavutil/staticpool.h | 117 +++++++++++++++++++++++++++++++++++++++++++++++++
>> >>  1 file changed, 117 insertions(+)
>> >>  create mode 100644 libavutil/staticpool.h
>> >>
>> >> diff --git a/libavutil/staticpool.h b/libavutil/staticpool.h
>> >> new file mode 100644
>> >> index 0000000000..9c9b2784bc
>> >> --- /dev/null
>> >> +++ b/libavutil/staticpool.h
>> >> @@ -0,0 +1,117 @@
>> >> +/*
>> >> + * This file is part of FFmpeg.
>> >> + *
>> >> + * FFmpeg is free software; you can redistribute it and/or
>> >> + * modify it under the terms of the GNU Lesser General Public
>> >> + * License as published by the Free Software Foundation; either
>> >> + * version 2.1 of the License, or (at your option) any later version.
>> >> + *
>> >> + * FFmpeg is distributed in the hope that it will be useful,
>> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> >> + * Lesser General Public License for more details.
>> >> + *
>> >> + * You should have received a copy of the GNU Lesser General Public
>> >> + * License along with FFmpeg; if not, write to the Free Software
>> >> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>> >> + */
>> >> +
>> >> +#ifndef AVUTIL_STATICPOOL_H
>> >> +#define AVUTIL_STATICPOOL_H
>> >> +
>> >> +#include <stdatomic.h>
>> >> +#include "avassert.h"
>> >> +#include "mem.h"
>> >> +
>> >> +/**
>> >> + * FF_STATICPOOL allocate memory without av_malloc if possible
>> >> + * @param size must be 2^n between 64 and 4096
>> >> + */
>> >> +#define FF_STATICPOOL_DECLARE(type, size)                                       \
>> >> +typedef struct type##_StaticPoolWrapper {                                       \
>> >> +    type        buf;                                                            \
>> >> +    unsigned    index;                                                          \
>> >> +    atomic_uint next;                                                           \
>> >> +} type##_StaticPoolWrapper;                                                     \
>> >> +                                                                                \
>> >> +static atomic_uint              type##_staticpool_next;                         \
>> >> +static atomic_uint              type##_staticpool_last;                         \
>> >> +static type##_StaticPoolWrapper type##_staticpool_table[size];                  \
>> >> +                                                                                \
>> >> +static type *type##_staticpool_malloc(void)                                     \
>> >> +{                                                                               \
>> >> +    unsigned val, index, serial, new_val;                                       \
>> >> +                                                                                \
>> >> +    av_assert0((size) >= 64 && (size) <= 4096 && !((size) & ((size) - 1)));     \
>> >> +                                                                                \
>> >> +    /* use serial, avoid spinlock */                                            \
>> >> +    /* acquire, so we don't get stalled table[index].next */                    \
>> >> +    val = atomic_load_explicit(&type##_staticpool_next, memory_order_acquire);  \
>> >> +    do {                                                                        \
>> >> +        index  = val & ((size) - 1);                                            \
>> >> +        serial = val & ~((size) - 1);                                           \
>> >> +        new_val = atomic_load_explicit(&type##_staticpool_table[index].next, memory_order_relaxed) | (serial + (size)); \
>> >> +    } while (!atomic_compare_exchange_strong_explicit(&type##_staticpool_next, &val, new_val, \
>> >
>> > The wrappers for atomic_compare_exchange_* in the compat folder are not
>> > really working and fixing them is supposedly not trivial, so this will
>> > only work with GCC and Clang but not with for example MSVC or SunCC.
>>
>> What's the problem? I only see that stdatomic compat make typedef
>> every atomic type to intptr_t, so atomic_*64_t won't work if
>> sizeof(intptr_t) == 32.
>> Here I use atomic_uint, so I guess it will work.
>>
>> Note that if atomic_compare_exchange_* is broken then atomic_fetch_*
>> will also be broken because atomic_fetch_* call
>> atomic_compare_exchange_* on suncc compat.
>>
>> >
>> > Can you implement this using mutexes instead, or otherwise avoid using
>> > atomic_compare_exchange_*? You can even use static mutex initialization
>> > now for all targets, and not just native pthreads.
>>
>> Using mutex makes implementation slower.
>> Using atomic_exchange requires spin lock.
>
> Uncontended mutexes are spinlocks.

Of course.
But this code isn't spinlock, so probably the performance won't suffer
so much when contended.