[vlc-devel] [PATCH] core: Add atomic refcounter helper
Romain Vimont
rom1v at videolabs.io
Sat Jun 30 22:19:12 CEST 2018
On Sat, Jun 30, 2018 at 09:12:27PM +0200, Rémi Denis-Courmont wrote:
> 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...),
(which refcount would need uintptr_t?)
> - order on decrement - acq_rel is not always needed,
That's not a problem to guarantee a memory order stronger than required.
> - corner cases where the decrement is known to not reach zero,
Like?
> - 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.
Even if it does not fit all (corner) cases, I think it is worth having a
simple refcounter for the most general case (a refcounted structure).
In the typical case:
if (vlc_atomic_rc_dec(rc))
destroy_object();
is simpler and less error-prone than:
if (atomic_fetch_sub_explicit(refs, 1, memory_order_acq_rel) == 1)
destroy_object();
> 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é.
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
More information about the vlc-devel
mailing list