Re: [FFmpeg-devel] [PATCH 17/42] avcodec/refstruct: Add RefStruct pool API

2023-10-04 Thread Andreas Rheinhardt 
Anton Khirnov:
> Quoting Andreas Rheinhardt (2023-09-19 21:57:09)
>> Very similar to the AVBufferPool API, but with some differences:
>> 1. Reusing an already existing entry does not incur an allocation
>> at all any more (the AVBufferPool API needs to allocate an AVBufferRef).
>> 2. The tasks done while holding the lock are smaller; e.g.
>> allocating new entries is now performed without holding the lock.
>> The same goes for freeing.
>> 3. The entries are freed as soon as possible (the AVBufferPool API
>> frees them in two batches: The first in av_buffer_pool_uninit() and
>> the second immediately before the pool is freed when the last
>> outstanding entry is returned to the pool).
>> 4. The API is designed for objects and not naked buffers and
>> therefore has a reset callback. This is called whenever an object
>> is returned to the pool.
>> 5. Just like with the RefStruct API, custom allocators are not
>> supported.
>>
>> (If desired, the FFRefStructPool struct itself could be made
>> reference counted via the RefStruct API; an FFRefStructPool
>> would then be freed via ff_refstruct_unref().)
> 
> Considering that it's intended to be used from multiple threads, that
> seems like the better option. Though I have not seen the following
> patches yet, so maybe the advantage is not as big as I'd think.
> 

It is implemented in patch #24.

>> +/**
>> + * If this flag is not set, every object in the pool will be zeroed before
>> + * the init callback is called or before it is turned over to the user
>> + * for the first time if no init callback has been provided.
>> + */
>> +#define FF_REFSTRUCT_POOL_FLAG_NO_ZEROING 
>> FF_REFSTRUCT_FLAG_NO_ZEROING
> 
> Do you think using the same namespace really improves things? It does
> not seem so to me.
> 

Actually, the namespaces are separate. The pool API uses
FF_REFSTRUCT_POOL_FLAG_*, the ordinary RefStruct API uses
FF_REFSTRUCT_FLAG_*. That some pool flags are simply mapped to non-pool
flags is an implementation detail and does not join the namespaces.

- Andreas

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 17/42] avcodec/refstruct: Add RefStruct pool API

2023-10-04 Thread Anton Khirnov 
Quoting Andreas Rheinhardt (2023-09-19 21:57:09)
> Very similar to the AVBufferPool API, but with some differences:
> 1. Reusing an already existing entry does not incur an allocation
> at all any more (the AVBufferPool API needs to allocate an AVBufferRef).
> 2. The tasks done while holding the lock are smaller; e.g.
> allocating new entries is now performed without holding the lock.
> The same goes for freeing.
> 3. The entries are freed as soon as possible (the AVBufferPool API
> frees them in two batches: The first in av_buffer_pool_uninit() and
> the second immediately before the pool is freed when the last
> outstanding entry is returned to the pool).
> 4. The API is designed for objects and not naked buffers and
> therefore has a reset callback. This is called whenever an object
> is returned to the pool.
> 5. Just like with the RefStruct API, custom allocators are not
> supported.
> 
> (If desired, the FFRefStructPool struct itself could be made
> reference counted via the RefStruct API; an FFRefStructPool
> would then be freed via ff_refstruct_unref().)

Considering that it's intended to be used from multiple threads, that
seems like the better option. Though I have not seen the following
patches yet, so maybe the advantage is not as big as I'd think.

> +/**
> + * If this flag is not set, every object in the pool will be zeroed before
> + * the init callback is called or before it is turned over to the user
> + * for the first time if no init callback has been provided.
> + */
> +#define FF_REFSTRUCT_POOL_FLAG_NO_ZEROING 
> FF_REFSTRUCT_FLAG_NO_ZEROING

Do you think using the same namespace really improves things? It does
not seem so to me.

-- 
Anton Khirnov
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 17/42] avcodec/refstruct: Add RefStruct pool API

2023-09-20 Thread Andreas Rheinhardt 
Michael Niedermayer:
> On Tue, Sep 19, 2023 at 09:57:09PM +0200, Andreas Rheinhardt wrote:
>> Very similar to the AVBufferPool API, but with some differences:
>> 1. Reusing an already existing entry does not incur an allocation
>> at all any more (the AVBufferPool API needs to allocate an AVBufferRef).
>> 2. The tasks done while holding the lock are smaller; e.g.
>> allocating new entries is now performed without holding the lock.
>> The same goes for freeing.
>> 3. The entries are freed as soon as possible (the AVBufferPool API
>> frees them in two batches: The first in av_buffer_pool_uninit() and
>> the second immediately before the pool is freed when the last
>> outstanding entry is returned to the pool).
>> 4. The API is designed for objects and not naked buffers and
>> therefore has a reset callback. This is called whenever an object
>> is returned to the pool.
>> 5. Just like with the RefStruct API, custom allocators are not
>> supported.
>>
>> (If desired, the FFRefStructPool struct itself could be made
>> reference counted via the RefStruct API; an FFRefStructPool
>> would then be freed via ff_refstruct_unref().)
>>
>> Signed-off-by: Andreas Rheinhardt 
>> ---
>>  libavcodec/refstruct.c | 194 -
>>  libavcodec/refstruct.h | 128 +++
>>  2 files changed, 321 insertions(+), 1 deletion(-)
> 
> seems to break building on ppc (--disable-pthreads)
> 
> CClibavcodec/refstruct.o
> src/libavcodec/refstruct.c: In function ‘pool_free’:
> src/libavcodec/refstruct.c:187:5: error: implicit declaration of function 
> ‘pthread_mutex_destroy’; did you mean ‘ff_mutex_destroy’? 
> [-Werror=implicit-function-declaration]
>  pthread_mutex_destroy(&pool->mutex);
>  ^
>  ff_mutex_destroy
> src/libavcodec/refstruct.c: In function ‘pool_return_entry’:
> src/libavcodec/refstruct.c:205:5: error: implicit declaration of function 
> ‘pthread_mutex_lock’; did you mean ‘ff_mutex_lock’? 
> [-Werror=implicit-function-declaration]
>  pthread_mutex_lock(&pool->mutex);
>  ^~
>  ff_mutex_lock
> src/libavcodec/refstruct.c:211:5: error: implicit declaration of function 
> ‘pthread_mutex_unlock’; did you mean ‘ff_mutex_unlock’? 
> [-Werror=implicit-function-declaration]
>  pthread_mutex_unlock(&pool->mutex);
>  ^~~~
>  ff_mutex_unlock
> src/libavcodec/refstruct.c: In function ‘ff_refstruct_pool_alloc_ext_c’:
> src/libavcodec/refstruct.c:339:11: error: implicit declaration of function 
> ‘pthread_mutex_init’; did you mean ‘ff_mutex_init’? 
> [-Werror=implicit-function-declaration]
>  err = pthread_mutex_init(&pool->mutex, NULL);
>^~
>ff_mutex_init
> cc1: some warnings being treated as errors
> /home/michael/ffmpeg-git/ffmpeg/ffbuild/common.mak:81: recipe for target 
> 'libavcodec/refstruct.o' failed
> make: *** [libavcodec/refstruct.o] Error 1
> make: Target 'all' not remade because of errors.

Sorry for not having tested --disable-pthreads. Will switch to our
ff_mutex-wrappers.

- Andreas

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 17/42] avcodec/refstruct: Add RefStruct pool API

2023-09-20 Thread Michael Niedermayer 
On Tue, Sep 19, 2023 at 09:57:09PM +0200, Andreas Rheinhardt wrote:
> Very similar to the AVBufferPool API, but with some differences:
> 1. Reusing an already existing entry does not incur an allocation
> at all any more (the AVBufferPool API needs to allocate an AVBufferRef).
> 2. The tasks done while holding the lock are smaller; e.g.
> allocating new entries is now performed without holding the lock.
> The same goes for freeing.
> 3. The entries are freed as soon as possible (the AVBufferPool API
> frees them in two batches: The first in av_buffer_pool_uninit() and
> the second immediately before the pool is freed when the last
> outstanding entry is returned to the pool).
> 4. The API is designed for objects and not naked buffers and
> therefore has a reset callback. This is called whenever an object
> is returned to the pool.
> 5. Just like with the RefStruct API, custom allocators are not
> supported.
> 
> (If desired, the FFRefStructPool struct itself could be made
> reference counted via the RefStruct API; an FFRefStructPool
> would then be freed via ff_refstruct_unref().)
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/refstruct.c | 194 -
>  libavcodec/refstruct.h | 128 +++
>  2 files changed, 321 insertions(+), 1 deletion(-)

seems to break building on ppc (--disable-pthreads)

CC  libavcodec/refstruct.o
src/libavcodec/refstruct.c: In function ‘pool_free’:
src/libavcodec/refstruct.c:187:5: error: implicit declaration of function 
‘pthread_mutex_destroy’; did you mean ‘ff_mutex_destroy’? 
[-Werror=implicit-function-declaration]
 pthread_mutex_destroy(&pool->mutex);
 ^
 ff_mutex_destroy
src/libavcodec/refstruct.c: In function ‘pool_return_entry’:
src/libavcodec/refstruct.c:205:5: error: implicit declaration of function 
‘pthread_mutex_lock’; did you mean ‘ff_mutex_lock’? 
[-Werror=implicit-function-declaration]
 pthread_mutex_lock(&pool->mutex);
 ^~
 ff_mutex_lock
src/libavcodec/refstruct.c:211:5: error: implicit declaration of function 
‘pthread_mutex_unlock’; did you mean ‘ff_mutex_unlock’? 
[-Werror=implicit-function-declaration]
 pthread_mutex_unlock(&pool->mutex);
 ^~~~
 ff_mutex_unlock
src/libavcodec/refstruct.c: In function ‘ff_refstruct_pool_alloc_ext_c’:
src/libavcodec/refstruct.c:339:11: error: implicit declaration of function 
‘pthread_mutex_init’; did you mean ‘ff_mutex_init’? 
[-Werror=implicit-function-declaration]
 err = pthread_mutex_init(&pool->mutex, NULL);
   ^~
   ff_mutex_init
cc1: some warnings being treated as errors
/home/michael/ffmpeg-git/ffmpeg/ffbuild/common.mak:81: recipe for target 
'libavcodec/refstruct.o' failed
make: *** [libavcodec/refstruct.o] Error 1
make: Target 'all' not remade because of errors.


[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

During times of universal deceit, telling the truth becomes a
revolutionary act. -- George Orwell


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 17/42] avcodec/refstruct: Add RefStruct pool API

2023-09-19 Thread Andreas Rheinhardt 
Very similar to the AVBufferPool API, but with some differences:
1. Reusing an already existing entry does not incur an allocation
at all any more (the AVBufferPool API needs to allocate an AVBufferRef).
2. The tasks done while holding the lock are smaller; e.g.
allocating new entries is now performed without holding the lock.
The same goes for freeing.
3. The entries are freed as soon as possible (the AVBufferPool API
frees them in two batches: The first in av_buffer_pool_uninit() and
the second immediately before the pool is freed when the last
outstanding entry is returned to the pool).
4. The API is designed for objects and not naked buffers and
therefore has a reset callback. This is called whenever an object
is returned to the pool.
5. Just like with the RefStruct API, custom allocators are not
supported.

(If desired, the FFRefStructPool struct itself could be made
reference counted via the RefStruct API; an FFRefStructPool
would then be freed via ff_refstruct_unref().)

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/refstruct.c | 194 -
 libavcodec/refstruct.h | 128 +++
 2 files changed, 321 insertions(+), 1 deletion(-)

diff --git a/libavcodec/refstruct.c b/libavcodec/refstruct.c
index 604938922a..7539b7942e 100644
--- a/libavcodec/refstruct.c
+++ b/libavcodec/refstruct.c
@@ -23,8 +23,11 @@
 #include "internal.h"
 #include "refstruct.h"
 
+#include "libavutil/avassert.h"
+#include "libavutil/error.h"
 #include "libavutil/macros.h"
 #include "libavutil/mem.h"
+#include "libavutil/thread.h"
 
 typedef struct RefCount {
 /**
@@ -35,6 +38,7 @@ typedef struct RefCount {
 atomic_uintptr_t  refcount;
 FFRefStructOpaque opaque;
 void (*free_cb)(FFRefStructOpaque opaque, void *obj);
+void (*free)(void *ref);
 } RefCount;
 
 #if __STDC_VERSION__ >= 201112L
@@ -64,6 +68,7 @@ static void refcount_init(RefCount *ref, FFRefStructOpaque 
opaque,
 atomic_init(&ref->refcount, 1);
 ref->opaque  = opaque;
 ref->free_cb = free_cb;
+ref->free= av_free;
 }
 
 void *ff_refstruct_alloc_ext_c(size_t size, unsigned flags, FFRefStructOpaque 
opaque,
@@ -103,7 +108,7 @@ void ff_refstruct_unref(void *objp)
 if (atomic_fetch_sub_explicit(&ref->refcount, 1, memory_order_acq_rel) == 
1) {
 if (ref->free_cb)
 ref->free_cb(ref->opaque, obj);
-av_free(ref);
+ref->free(ref);
 }
 
 return;
@@ -151,3 +156,190 @@ int ff_refstruct_exclusive(const void *data)
  * accept const atomics in C11 (see also N1807). */
 return atomic_load_explicit((atomic_uintptr_t*)&ref->refcount, 
memory_order_acquire) == 1;
 }
+
+struct FFRefStructPool {
+size_t size;
+FFRefStructOpaque opaque;
+int  (*init_cb)(FFRefStructOpaque opaque, void *obj);
+void (*reset_cb)(FFRefStructOpaque opaque, void *obj);
+void (*free_entry_cb)(FFRefStructOpaque opaque, void *obj);
+void (*free_cb)(FFRefStructOpaque opaque);
+
+int uninited;
+unsigned entry_flags;
+unsigned pool_flags;
+
+/** The number of outstanding entries not in available_entries. */
+atomic_uintptr_t refcount;
+/**
+ * This is a linked list of available entries;
+ * the RefCount's opaque pointer is used as next pointer
+ * for available entries.
+ * While the entries are in use, the opaque is a pointer
+ * to the corresponding FFRefStructPool.
+ */
+RefCount *available_entries;
+pthread_mutex_t mutex;
+};
+
+static void pool_free(FFRefStructPool *pool)
+{
+pthread_mutex_destroy(&pool->mutex);
+if (pool->free_cb)
+pool->free_cb(pool->opaque);
+av_free(pool);
+}
+
+static void pool_free_entry(FFRefStructPool *pool, RefCount *ref)
+{
+if (pool->free_entry_cb)
+pool->free_entry_cb(pool->opaque, get_userdata(ref));
+av_free(ref);
+}
+
+static void pool_return_entry(void *ref_)
+{
+RefCount *ref = ref_;
+FFRefStructPool *pool = ref->opaque.nc;
+
+pthread_mutex_lock(&pool->mutex);
+if (!pool->uninited) {
+ref->opaque.nc = pool->available_entries;
+pool->available_entries = ref;
+ref = NULL;
+}
+pthread_mutex_unlock(&pool->mutex);
+
+if (ref)
+pool_free_entry(pool, ref);
+
+if (atomic_fetch_sub_explicit(&pool->refcount, 1, memory_order_acq_rel) == 
1)
+pool_free(pool);
+}
+
+static void pool_reset_entry(FFRefStructOpaque opaque, void *entry)
+{
+FFRefStructPool *pool = opaque.nc;
+
+pool->reset_cb(pool->opaque, entry);
+}
+
+static int refstruct_pool_get_ext(void *datap, FFRefStructPool *pool)
+{
+void *ret = NULL;
+
+memcpy(datap, &(void *){ NULL }, sizeof(void*));
+
+pthread_mutex_lock(&pool->mutex);
+av_assert1(!pool->uninited);
+if (pool->available_entries) {
+RefCount *ref = pool->available_entries;
+ret = get_userdata(ref);
+pool->available_entries = ref->opaque.nc;
+ref->opaque.nc = pool;
+atomic