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

[Muhammad Faiz]

On Sun, Jan 21, 2018 at 7:11 PM, wm4  wrote:
> On Sun, 21 Jan 2018 10:24:21 +0700
> Muhammad Faiz  wrote:
>
>> >  I don't trust the atomics use
>> > either, I'm don't want to have to debug that ever.
>>
>> Of course, using atomics is more complicated that using mutex (with
>> benefits that it will be faster when properly used).
>> But it is not a valid reason to avoid using atomic because it is more
>> complicated.
>
> Sure, but it also means it should be really be confined to cases where
> it _really_ helps with performance.
>
> Where is this a bottleneck at all?

Performance difference is noticeable with audio-only stuff.
Because audio processing is typically fast, malloc-free cycle of
AVFrame, AVBuffer, etc becomes bottlenecks.


>
> I also think that this really belongs into a malloc implementation
> instead. You might also want to try "alternative" malloc
> implementations like jemalloc.

jemalloc nicely is fast. The performance is on par with staticpool and
even faster on high contending situation. I hope that new glibc-malloc
is also fast. So I drop the patch.

With this malloc performance, usage of AVBufferPool on audio frame
becomes questionable.

Thank's.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[wm4]

On Sun, 21 Jan 2018 10:24:21 +0700
Muhammad Faiz  wrote:

> >  I don't trust the atomics use
> > either, I'm don't want to have to debug that ever.  
> 
> Of course, using atomics is more complicated that using mutex (with
> benefits that it will be faster when properly used).
> But it is not a valid reason to avoid using atomic because it is more
> complicated.

Sure, but it also means it should be really be confined to cases where
it _really_ helps with performance.

Where is this a bottleneck at all?

I also think that this really belongs into a malloc implementation
instead. You might also want to try "alternative" malloc
implementations like jemalloc.

Another bad point is that this will make interaction with memory
debuggers worse. (They can't track whether the memory is considered
allocated or free.)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[Muhammad Faiz]

On Sat, Jan 20, 2018 at 5:22 PM, wm4  wrote:
> On Sat, 20 Jan 2018 11:29:13 +0700
> Muhammad Faiz  wrote:
>
>> Help avoiding malloc-free cycles when allocating-freeing common
>> structures.
>>
>> Signed-off-by: Muhammad Faiz 
>> ---
>>  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 00..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 
>> +#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 {   
>> \
>> +typebuf;
>> \
>> +unsignedindex;  
>> \
>> +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(##_staticpool_next, 
>> memory_order_acquire);  \
>> +do {
>> \
>> +index  = val & ((size) - 1);
>> \
>> +serial = val & ~((size) - 1);   
>> \
>> +new_val = 
>> atomic_load_explicit(##_staticpool_table[index].next, 
>> memory_order_relaxed) | (serial + (size)); \
>> +} while 
>> (!atomic_compare_exchange_strong_explicit(##_staticpool_next, , 
>> new_val, \
>> +  memory_order_acquire, 
>> memory_order_acquire)); \
>> +
>> \
>> +index = val & ((size) - 1); 
>> \
>> +if (index)  
>> \
>> +return ##_staticpool_table[index].buf; 
>> \
>> +
>> \
>> +index = atomic_fetch_add_explicit(##_staticpool_last, 1, 
>> memory_order_relaxed) + 1; \
>> +if (index < (size)) {   
>> \
>> +type##_staticpool_table[index].index = index;   
>> \
>> +return ##_staticpool_table[index].buf; 
>> \
>> +}  

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

[Muhammad Faiz]

On Sat, Jan 20, 2018 at 5:18 PM, wm4  wrote:
> On Sat, 20 Jan 2018 12:52:46 +0700
> Muhammad Faiz  wrote:
>
>> On Sat, Jan 20, 2018 at 11:49 AM, James Almer  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 
>> >> ---
>> >>  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 00..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 
>> >> +#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 {
>> >>\
>> >> +typebuf; 
>> >>\
>> >> +unsignedindex;   
>> >>\
>> >> +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(##_staticpool_next, 
>> >> memory_order_acquire);  \
>> >> +do { 
>> >>\
>> >> +index  = val & ((size) - 1); 
>> >>\
>> >> +serial = val & ~((size) - 1);
>> >>\
>> >> +new_val = 
>> >> atomic_load_explicit(##_staticpool_table[index].next, 
>> >> memory_order_relaxed) | (serial + (size)); \
>> >> +} while 
>> >> (!atomic_compare_exchange_strong_explicit(##_staticpool_next, , 
>> >> 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_*

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

[Muhammad Faiz]

On Sat, Jan 20, 2018 at 6:32 PM, Rostislav Pehlivanov
 wrote:
> On 20 January 2018 at 11:13, Rostislav Pehlivanov 
> wrote:
>
>>
>>
>> On 20 January 2018 at 10:22, wm4  wrote:
>>
>>> On Sat, 20 Jan 2018 11:29:13 +0700
>>> Muhammad Faiz  wrote:
>>>
>>> > Help avoiding malloc-free cycles when allocating-freeing common
>>> > structures.
>>> >
>>> > Signed-off-by: Muhammad Faiz 
>>> > ---
>>> >  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 00..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 
>>> > +#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 {
>>>  \
>>> > +typebuf;
>>>   \
>>> > +unsignedindex;
>>>   \
>>> > +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(##_staticpool_next,
>>> memory_order_acquire);  \
>>> > +do {
>>>   \
>>> > +index  = val & ((size) - 1);
>>>   \
>>> > +serial = val & ~((size) - 1);
>>>  \
>>> > +new_val = 
>>> > atomic_load_explicit(##_staticpool_table[index].next,
>>> memory_order_relaxed) | (serial + (size)); \
>>> > +} while (!atomic_compare_exchange_stro
>>> ng_explicit(##_staticpool_next, , new_val, \
>>> > +
>>> memory_order_acquire, memory_order_acquire)); \
>>> > +
>>>   \
>>> > +index = val & ((size) - 1);
>>>  \
>>> > +if (index)
>>>   \
>>> > +return ##_staticpool_table[index].buf;
>>>\
>>> > +
>>>   \
>>> > +index = atomic_fetch_add_explicit(##_staticpool_last, 1,
>>> memory_order_relaxed) + 1; \
>>> > +if (index < (size)) {
>>>  \
>>> > +type##_staticpool_table[index].index = index;
>>>\
>>> > +return ##_staticpool_table[index].buf;
>>>\
>>> > +}
>>>  \
>>> > +
>>>   \
>>> > +atomic_fetch_add_explicit(##_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;

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

[Rostislav Pehlivanov]

On 20 January 2018 at 11:13, Rostislav Pehlivanov 
wrote:

>
>
> On 20 January 2018 at 10:22, wm4  wrote:
>
>> On Sat, 20 Jan 2018 11:29:13 +0700
>> Muhammad Faiz  wrote:
>>
>> > Help avoiding malloc-free cycles when allocating-freeing common
>> > structures.
>> >
>> > Signed-off-by: Muhammad Faiz 
>> > ---
>> >  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 00..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 
>> > +#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 {
>>  \
>> > +typebuf;
>>   \
>> > +unsignedindex;
>>   \
>> > +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(##_staticpool_next,
>> memory_order_acquire);  \
>> > +do {
>>   \
>> > +index  = val & ((size) - 1);
>>   \
>> > +serial = val & ~((size) - 1);
>>  \
>> > +new_val = 
>> > atomic_load_explicit(##_staticpool_table[index].next,
>> memory_order_relaxed) | (serial + (size)); \
>> > +} while (!atomic_compare_exchange_stro
>> ng_explicit(##_staticpool_next, , new_val, \
>> > +
>> memory_order_acquire, memory_order_acquire)); \
>> > +
>>   \
>> > +index = val & ((size) - 1);
>>  \
>> > +if (index)
>>   \
>> > +return ##_staticpool_table[index].buf;
>>\
>> > +
>>   \
>> > +index = atomic_fetch_add_explicit(##_staticpool_last, 1,
>> memory_order_relaxed) + 1; \
>> > +if (index < (size)) {
>>  \
>> > +type##_staticpool_table[index].index = index;
>>\
>> > +return ##_staticpool_table[index].buf;
>>\
>> > +}
>>  \
>> > +
>>   \
>> > +atomic_fetch_add_explicit(##_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);
>> 

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

[Rostislav Pehlivanov]

On 20 January 2018 at 10:22, wm4  wrote:

> On Sat, 20 Jan 2018 11:29:13 +0700
> Muhammad Faiz  wrote:
>
> > Help avoiding malloc-free cycles when allocating-freeing common
> > structures.
> >
> > Signed-off-by: Muhammad Faiz 
> > ---
> >  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 00..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 
> > +#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 {
>  \
> > +typebuf;
> \
> > +unsignedindex;
> \
> > +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(##_staticpool_next,
> memory_order_acquire);  \
> > +do {
> \
> > +index  = val & ((size) - 1);
> \
> > +serial = val & ~((size) - 1);
>  \
> > +new_val = 
> > atomic_load_explicit(##_staticpool_table[index].next,
> memory_order_relaxed) | (serial + (size)); \
> > +} while 
> > (!atomic_compare_exchange_strong_explicit(##_staticpool_next,
> , new_val, \
> > +
> memory_order_acquire, memory_order_acquire)); \
> > +
> \
> > +index = val & ((size) - 1);
>  \
> > +if (index)
> \
> > +return ##_staticpool_table[index].buf;
>  \
> > +
> \
> > +index = atomic_fetch_add_explicit(##_staticpool_last, 1,
> memory_order_relaxed) + 1; \
> > +if (index < (size)) {
>  \
> > +type##_staticpool_table[index].index = index;
>  \
> > +return ##_staticpool_table[index].buf;
>  \
> > +}
>  \
> > +
> \
> > +atomic_fetch_add_explicit(##_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(>buf, FF_MEMORY_POISON, sizeof(entry->buf));
> \
> > +
> \
> > +val = atomic_load_explicit(##_staticpool_next,
> 

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

[Hendrik Leppkes]

On Sat, Jan 20, 2018 at 11:22 AM, wm4  wrote:
> On Sat, 20 Jan 2018 11:29:13 +0700
> Muhammad Faiz  wrote:
>
>> Help avoiding malloc-free cycles when allocating-freeing common
>> structures.
>>
>> Signed-off-by: Muhammad Faiz 
>> ---
>>  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 00..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 
>> +#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 {   
>> \
>> +typebuf;
>> \
>> +unsignedindex;  
>> \
>> +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(##_staticpool_next, 
>> memory_order_acquire);  \
>> +do {
>> \
>> +index  = val & ((size) - 1);
>> \
>> +serial = val & ~((size) - 1);   
>> \
>> +new_val = 
>> atomic_load_explicit(##_staticpool_table[index].next, 
>> memory_order_relaxed) | (serial + (size)); \
>> +} while 
>> (!atomic_compare_exchange_strong_explicit(##_staticpool_next, , 
>> new_val, \
>> +  memory_order_acquire, 
>> memory_order_acquire)); \
>> +
>> \
>> +index = val & ((size) - 1); 
>> \
>> +if (index)  
>> \
>> +return ##_staticpool_table[index].buf; 
>> \
>> +
>> \
>> +index = atomic_fetch_add_explicit(##_staticpool_last, 1, 
>> memory_order_relaxed) + 1; \
>> +if (index < (size)) {   
>> \
>> +type##_staticpool_table[index].index = index;   
>> \
>> +return ##_staticpool_table[index].buf; 
>> \
>> +} 

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

[wm4]

On Sat, 20 Jan 2018 11:29:13 +0700
Muhammad Faiz  wrote:

> Help avoiding malloc-free cycles when allocating-freeing common
> structures.
> 
> Signed-off-by: Muhammad Faiz 
> ---
>  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 00..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 
> +#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 {
>\
> +typebuf; 
>\
> +unsignedindex;   
>\
> +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(##_staticpool_next, 
> memory_order_acquire);  \
> +do { 
>\
> +index  = val & ((size) - 1); 
>\
> +serial = val & ~((size) - 1);
>\
> +new_val = atomic_load_explicit(##_staticpool_table[index].next, 
> memory_order_relaxed) | (serial + (size)); \
> +} while 
> (!atomic_compare_exchange_strong_explicit(##_staticpool_next, , 
> new_val, \
> +  memory_order_acquire, 
> memory_order_acquire)); \
> + 
>\
> +index = val & ((size) - 1);  
>\
> +if (index)   
>\
> +return ##_staticpool_table[index].buf;  
>\
> + 
>\
> +index = atomic_fetch_add_explicit(##_staticpool_last, 1, 
> memory_order_relaxed) + 1; \
> +if (index < (size)) {
>\
> +type##_staticpool_table[index].index = index;
>\
> +return ##_staticpool_table[index].buf;  
>\
> +}
>\
> + 
>\
> +

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

[wm4]

On Sat, 20 Jan 2018 12:52:46 +0700
Muhammad Faiz  wrote:

> On Sat, Jan 20, 2018 at 11:49 AM, James Almer  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 
> >> ---
> >>  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 00..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 
> >> +#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 { 
> >>   \
> >> +typebuf;  
> >>   \
> >> +unsignedindex;
> >>   \
> >> +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(##_staticpool_next, 
> >> memory_order_acquire);  \
> >> +do {  
> >>   \
> >> +index  = val & ((size) - 1);  
> >>   \
> >> +serial = val & ~((size) - 1); 
> >>   \
> >> +new_val = 
> >> atomic_load_explicit(##_staticpool_table[index].next, 
> >> memory_order_relaxed) | (serial + (size)); \
> >> +} while 
> >> (!atomic_compare_exchange_strong_explicit(##_staticpool_next, , 
> >> 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

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

[Muhammad Faiz]

On Sat, Jan 20, 2018 at 11:49 AM, James Almer  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 
>> ---
>>  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 00..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 
>> +#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 {   
>> \
>> +typebuf;
>> \
>> +unsignedindex;  
>> \
>> +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(##_staticpool_next, 
>> memory_order_acquire);  \
>> +do {
>> \
>> +index  = val & ((size) - 1);
>> \
>> +serial = val & ~((size) - 1);   
>> \
>> +new_val = 
>> atomic_load_explicit(##_staticpool_table[index].next, 
>> memory_order_relaxed) | (serial + (size)); \
>> +} while 
>> (!atomic_compare_exchange_strong_explicit(##_staticpool_next, , 
>> 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.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org

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

[James Almer]

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 
> ---
>  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 00..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 
> +#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 {
>\
> +typebuf; 
>\
> +unsignedindex;   
>\
> +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(##_staticpool_next, 
> memory_order_acquire);  \
> +do { 
>\
> +index  = val & ((size) - 1); 
>\
> +serial = val & ~((size) - 1);
>\
> +new_val = atomic_load_explicit(##_staticpool_table[index].next, 
> memory_order_relaxed) | (serial + (size)); \
> +} while 
> (!atomic_compare_exchange_strong_explicit(##_staticpool_next, , 
> 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.

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.

> +  memory_order_acquire, 
> memory_order_acquire)); \
> + 
>\
> +index = val & ((size) - 1);  
>\
> +if (index)   
>\
> +return ##_staticpool_table[index].buf;  
>\
> + 
>\
> +index = atomic_fetch_add_explicit(##_staticpool_last, 1, 
> memory_order_relaxed) + 1; \
> +if (index < (size)) {
>\
> +