[vlc-devel] [PATCH] core: Add atomic refcounter helper
Rémi Denis-Courmont
remi at remlab.net
Sat Jun 30 21:12:27 CEST 2018
Hei,
It is not that simple. Leaving aside that asserts in public headers are troublesome, there is a lot of variance in reference counters:
- type sizes (uint, uintptr...),
- order on decrement - acq_rel is not always needed,
- corner cases where the decrement is known to not reach zero,
- corner cases where the decrement cannot be permitted to reach zero(e.g. vlc_object_t, IIRC).
We used to have a wrapper and it was removed because it turned more into an inconvenience.
Le 29 juin 2018 21:03:49 GMT+02:00, Romain Vimont <rom1v at videolabs.io> a écrit :
>Implement an atomic refcounter with a weak but correct (1) memory
>order,
>and expose a simple API.
>
>Use it in some existing code to demo its usage.
>
>(1) See for example "Using weakly ordered C++ atomics correctly" by
>Hans
> Boehm at CppCon 2016 (the refcounting part also applies to C11):
> <https://www.youtube.com/watch?v=M15UKpNlpeM&t=45m15s>
>---
> include/vlc_atomic.h | 25 +++++++++++++++++++++++++
> src/input/item.c | 6 +++---
> src/input/item.h | 4 ++--
> src/input/resource.c | 10 +++++-----
> src/misc/picture.c | 9 +++------
> src/misc/picture.h | 5 ++---
> 6 files changed, 40 insertions(+), 19 deletions(-)
>
>diff --git a/include/vlc_atomic.h b/include/vlc_atomic.h
>index 868918fe8e6..2cb5e23f489 100644
>--- a/include/vlc_atomic.h
>+++ b/include/vlc_atomic.h
>@@ -30,6 +30,7 @@
>* Atomic operations do not require locking, but they are not very
>powerful.
> */
>
>+# include <assert.h>
> # include <stdatomic.h>
>
> typedef atomic_uint_least32_t vlc_atomic_float;
>@@ -57,4 +58,28 @@ static inline void
>vlc_atomic_store_float(vlc_atomic_float *atom, float f)
> atomic_store(atom, u.i);
> }
>
>+typedef atomic_uint vlc_atomic_rc_t;
>+
>+/** Init the RC to 1 */
>+static inline void vlc_atomic_rc_init(vlc_atomic_rc_t *var)
>+{
>+ atomic_init(var, 1);
>+}
>+
>+/** Increment the RC */
>+static inline void vlc_atomic_rc_inc(vlc_atomic_rc_t *var)
>+{
>+ unsigned prev = atomic_fetch_add_explicit(var, 1,
>memory_order_relaxed);
>+ assert(prev);
>+ VLC_UNUSED(prev);
>+}
>+
>+/** Decrement the RC and return true if it reaches 0 */
>+static inline bool vlc_atomic_rc_dec(vlc_atomic_rc_t *var)
>+{
>+ unsigned prev = atomic_fetch_sub_explicit(var, 1,
>memory_order_acq_rel);
>+ assert(prev);
>+ return prev == 1;
>+}
>+
> #endif
>diff --git a/src/input/item.c b/src/input/item.c
>index 6789f73caa5..8e846425b4b 100644
>--- a/src/input/item.c
>+++ b/src/input/item.c
>@@ -471,7 +471,7 @@ input_item_t *input_item_Hold( input_item_t *p_item
>)
> {
> input_item_owner_t *owner = item_owner(p_item);
>
>- atomic_fetch_add( &owner->refs, 1 );
>+ vlc_atomic_rc_inc( &owner->rc );
> return p_item;
> }
>
>@@ -479,7 +479,7 @@ void input_item_Release( input_item_t *p_item )
> {
> input_item_owner_t *owner = item_owner(p_item);
>
>- if( atomic_fetch_sub(&owner->refs, 1) != 1 )
>+ if( !vlc_atomic_rc_dec( &owner->rc ) )
> return;
>
> vlc_event_manager_fini( &p_item->event_manager );
>@@ -1060,7 +1060,7 @@ input_item_NewExt( const char *psz_uri, const
>char *psz_name,
> if( unlikely(owner == NULL) )
> return NULL;
>
>- atomic_init( &owner->refs, 1 );
>+ vlc_atomic_rc_init( &owner->rc );
>
> input_item_t *p_input = &owner->item;
> vlc_event_manager_t * p_em = &p_input->event_manager;
>diff --git a/src/input/item.h b/src/input/item.h
>index dce1dfc0584..bce49777f8d 100644
>--- a/src/input/item.h
>+++ b/src/input/item.h
>@@ -25,7 +25,7 @@
> #define LIBVLC_INPUT_ITEM_H 1
>
> #include "input_interface.h"
>-#include <stdatomic.h>
>+#include <vlc_atomic.h>
>
>void input_item_SetErrorWhenReading( input_item_t *p_i, bool b_error );
>void input_item_UpdateTracksInfo( input_item_t *item, const es_format_t
>*fmt );
>@@ -34,7 +34,7 @@ bool input_item_ShouldPreparseSubItems( input_item_t
>*p_i );
> typedef struct input_item_owner
> {
> input_item_t item;
>- atomic_uint refs;
>+ vlc_atomic_rc_t rc;
> } input_item_owner_t;
>
> # define item_owner(item) ((struct input_item_owner *)(item))
>diff --git a/src/input/resource.c b/src/input/resource.c
>index f806cfeb2c9..0cad860b106 100644
>--- a/src/input/resource.c
>+++ b/src/input/resource.c
>@@ -28,10 +28,10 @@
> # include "config.h"
> #endif
>
>-#include <stdatomic.h>
> #include <assert.h>
>
> #include <vlc_common.h>
>+#include <vlc_atomic.h>
> #include <vlc_vout.h>
> #include <vlc_spu.h>
> #include <vlc_aout.h>
>@@ -45,7 +45,7 @@
>
> struct input_resource_t
> {
>- atomic_uint refs;
>+ vlc_atomic_rc_t rc;
>
> vlc_object_t *p_parent;
>
>@@ -422,7 +422,7 @@ input_resource_t *input_resource_New( vlc_object_t
>*p_parent )
> if( !p_resource )
> return NULL;
>
>- atomic_init( &p_resource->refs, 1 );
>+ vlc_atomic_rc_init( &p_resource->rc );
> p_resource->p_parent = p_parent;
> vlc_mutex_init( &p_resource->lock );
> vlc_mutex_init( &p_resource->lock_hold );
>@@ -431,7 +431,7 @@ input_resource_t *input_resource_New( vlc_object_t
>*p_parent )
>
> void input_resource_Release( input_resource_t *p_resource )
> {
>- if( atomic_fetch_sub( &p_resource->refs, 1 ) != 1 )
>+ if( !vlc_atomic_rc_dec( &p_resource->rc ) )
> return;
>
> DestroySout( p_resource );
>@@ -446,7 +446,7 @@ void input_resource_Release( input_resource_t
>*p_resource )
>
> input_resource_t *input_resource_Hold( input_resource_t *p_resource )
> {
>- atomic_fetch_add( &p_resource->refs, 1 );
>+ vlc_atomic_rc_inc( &p_resource->rc );
> return p_resource;
> }
>
>diff --git a/src/misc/picture.c b/src/misc/picture.c
>index e633cee42bc..5cabd420aab 100644
>--- a/src/misc/picture.c
>+++ b/src/misc/picture.c
>@@ -205,7 +205,7 @@ static picture_priv_t *picture_NewPrivate(const
>video_format_t *restrict p_fmt)
> return NULL;
> }
>
>- atomic_init( &priv->gc.refs, 1 );
>+ vlc_atomic_rc_init( &priv->gc.rc );
> priv->gc.opaque = NULL;
>
> return priv;
>@@ -305,8 +305,7 @@ picture_t *picture_Hold( picture_t *p_picture )
> assert( p_picture != NULL );
>
> picture_priv_t *priv = (picture_priv_t *)p_picture;
>- uintptr_t refs = atomic_fetch_add( &priv->gc.refs, 1 );
>- assert( refs > 0 );
>+ vlc_atomic_rc_inc( &priv->gc.rc );
> return p_picture;
> }
>
>@@ -315,9 +314,7 @@ void picture_Release( picture_t *p_picture )
> assert( p_picture != NULL );
>
> picture_priv_t *priv = (picture_priv_t *)p_picture;
>- uintptr_t refs = atomic_fetch_sub( &priv->gc.refs, 1 );
>- assert( refs != 0 );
>- if( refs > 1 )
>+ if( !vlc_atomic_rc_dec( &priv->gc.rc ) )
> return;
>
> PictureDestroyContext( p_picture );
>diff --git a/src/misc/picture.h b/src/misc/picture.h
>index 70ee64878da..4a70b6a6c53 100644
>--- a/src/misc/picture.h
>+++ b/src/misc/picture.h
>@@ -18,8 +18,7 @@
> * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
>*****************************************************************************/
>
>-#include <stdatomic.h>
>-
>+#include <vlc_atomic.h>
> #include <vlc_picture.h>
>
> typedef struct
>@@ -27,7 +26,7 @@ typedef struct
> picture_t picture;
> struct
> {
>- atomic_uintptr_t refs;
>+ vlc_atomic_rc_t rc;
> void (*destroy)(picture_t *);
> void *opaque;
> } gc;
>--
>2.18.0
>
>_______________________________________________
>vlc-devel mailing list
>To unsubscribe or modify your subscription options:
>https://mailman.videolan.org/listinfo/vlc-devel
--
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20180630/617e51ef/attachment.html>
More information about the vlc-devel
mailing list