[FFmpeg-devel] [PATCH 1/3] avutil: add staticpool
Hendrik Leppkes
h.leppkes at gmail.com
Sat Jan 20 12:26:58 EET 2018
On Sat, Jan 20, 2018 at 11:22 AM, wm4 <nfxjfg at googlemail.com> wrote:
> On Sat, 20 Jan 2018 11:29:13 +0700
> Muhammad Faiz <mfcc64 at gmail.com> 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, \
>> + memory_order_acquire, memory_order_acquire)); \
>> + \
>> + index = val & ((size) - 1); \
>> + if (index) \
>> + return &type##_staticpool_table[index].buf; \
>> + \
>> + index = atomic_fetch_add_explicit(&type##_staticpool_last, 1, memory_order_relaxed) + 1; \
>> + if (index < (size)) { \
>> + type##_staticpool_table[index].index = index; \
>> + return &type##_staticpool_table[index].buf; \
>> + } \
>> + \
>> + atomic_fetch_add_explicit(&type##_staticpool_last, -1, memory_order_relaxed); \
>> + return av_malloc(sizeof(type)); \
>> +} \
>> + \
>> +static inline type *type##_staticpool_mallocz(void) \
>> +{ \
>> + type *ptr = type##_staticpool_malloc(); \
>> + if (ptr) \
>> + memset(ptr, 0, sizeof(*ptr)); \
>> + return ptr; \
>> +} \
>> + \
>> +static void type##_staticpool_free(type *ptr) \
>> +{ \
>> + type##_StaticPoolWrapper *entry = (type##_StaticPoolWrapper *) ptr; \
>> + unsigned val, serial, index, new_val; \
>> + \
>> + if ((uintptr_t)ptr <= (uintptr_t)(type##_staticpool_table) || \
>> + (uintptr_t)ptr >= (uintptr_t)(type##_staticpool_table + size)) { \
>> + av_free(ptr); \
>> + return; \
>> + } \
>> + \
>> + if (CONFIG_MEMORY_POISONING) \
>> + memset(&entry->buf, FF_MEMORY_POISON, sizeof(entry->buf)); \
>> + \
>> + val = atomic_load_explicit(&type##_staticpool_next, memory_order_relaxed); \
>> + do { \
>> + index = val & ((size) - 1); \
>> + serial = val & ~((size) - 1); \
>> + atomic_store_explicit(&entry->next, index, memory_order_relaxed); \
>> + new_val = entry->index | (serial + (size)); \
>> + } while (!atomic_compare_exchange_strong_explicit(&type##_staticpool_next, &val, new_val, \
>> + memory_order_release, memory_order_relaxed)); \
>> +} \
>> + \
>> +static inline void type##_staticpool_freep(type **ptr) \
>> +{ \
>> + if (ptr) { \
>> + type##_staticpool_free(*ptr); \
>> + *ptr = NULL; \
>> + } \
>> +} \
>> +/* FF_STATICPOOL_DECLARE */
>> +
>> +#define FF_STATICPOOL_MALLOC(type) type##_staticpool_malloc()
>> +#define FF_STATICPOOL_MALLOCZ(type) type##_staticpool_mallocz()
>> +#define FF_STATICPOOL_FREE(type, v) type##_staticpool_free(v)
>> +#define FF_STATICPOOL_FREEP(type, v) type##_staticpool_freep(v)
>> +
>> +#endif
>
> I'm against this. Global mutable data is always bad. If you have 2
> uses ffmpeg in the same process, their efficiency might get halved
> because the available pool size is shared by both. The macro mess is
> very ugly. Can we please not define pages of code as macros. Last but
> not least this is the job of the memory manager, which already does
> fancy things like per thread pools. I don't trust the atomics use
> either, I'm don't want to have to debug that ever.
>
I generally agree on the global state. We should not be writing new
global state into our libraries, we're working on getting rid of that,
not introduce more.
- Hendrik
More information about the ffmpeg-devel
mailing list