From cdc4693da047ef54d55bf17d803605e4880b734e Mon Sep 17 00:00:00 2001 From: Philippe Proulx Date: Tue, 5 Jun 2018 17:43:28 -0400 Subject: [PATCH] Fix: set BT object's shared flag in all modes Issue ===== The bt_object_set_is_shared() function is only effective in a developer mode build. Since the shared flag is 0 when allocated, and bt_object_init() calls bt_object_set_is_shared() to make the object shared initially, all objects are non-shared in non-developer mode. This was the initial goal, because the only purpose of this shared flag was to check if the developer is calling bt_get() or bt_put() with a non-shared object. This check is only performed in developer mode anyway. However, the object pool system resets the object's reference count to 1 only when it is shared. Solution ======== Make bt_object_set_is_shared() always effective, and make the object pool unconditionally reset the reference count to 1 when recycling. This avoids a check and the reference count is always 1 for a unique object anyway (the test was superfluous). I leave bt_object_set_is_shared() to be always enabled for future use cases: it does not cost a lot to have it executed in non-developer mode as its execution cost is amortized anyway for recycled objects. Known drawbacks =============== Unnoticeable performance impact. Signed-off-by: Philippe Proulx --- include/babeltrace/object-internal.h | 18 ++++++------------ include/babeltrace/object-pool-internal.h | 11 ++++++----- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/include/babeltrace/object-internal.h b/include/babeltrace/object-internal.h index 4efba28e..afb9e6ee 100644 --- a/include/babeltrace/object-internal.h +++ b/include/babeltrace/object-internal.h @@ -50,7 +50,7 @@ struct bt_object { /* * True if this object is shared, that is, it uses reference - * counting. Only used in developer mode. + * counting. */ bool is_shared; }; @@ -137,27 +137,21 @@ void bt_object_set_parent(void *child_ptr, void *parent) #endif /* - * It is assumed that a "child" being "parented" is publicly reachable. - * Therefore, a reference to its parent must be taken. The reference - * to the parent will be released once the object's reference count - * falls to zero. + * It is assumed that a "child" being "parented" is publicly + * reachable. Therefore, a reference to its parent must be + * taken. The reference to the parent will be released once the + * object's reference count falls to zero. */ BT_PUT(child->parent); child->parent = bt_get(parent); } -#ifdef BT_DEV_MODE static inline -void _bt_object_set_is_shared(struct bt_object *obj, bool is_shared) +void bt_object_set_is_shared(struct bt_object *obj, bool is_shared) { obj->is_shared = is_shared; } -# define bt_object_set_is_shared _bt_object_set_is_shared -#else -# define bt_object_set_is_shared(_obj, _is_shared) -#endif - static inline void bt_object_init(void *ptr, bt_object_release_func release) { diff --git a/include/babeltrace/object-pool-internal.h b/include/babeltrace/object-pool-internal.h index fc9d8c0a..ef855890 100644 --- a/include/babeltrace/object-pool-internal.h +++ b/include/babeltrace/object-pool-internal.h @@ -119,11 +119,6 @@ void *bt_object_pool_create_object(struct bt_object_pool *pool) pool->size--; obj = pool->objects->pdata[pool->size]; pool->objects->pdata[pool->size] = NULL; - - if (obj->is_shared) { - /* Object is shared: reset reference count to 1 */ - obj->ref_count.count = 1; - } goto end; } @@ -152,6 +147,8 @@ end: static inline void bt_object_pool_recycle_object(struct bt_object_pool *pool, void *obj) { + struct bt_object *bt_obj = obj; + BT_ASSERT(pool); BT_ASSERT(obj); @@ -170,6 +167,10 @@ void bt_object_pool_recycle_object(struct bt_object_pool *pool, void *obj) g_ptr_array_set_size(pool->objects, pool->size + 1); } + /* Reset reference count to 1 since it could be 0 now */ + bt_obj->ref_count.count = 1; + + /* Back to the pool */ pool->objects->pdata[pool->size] = obj; pool->size++; -- 2.34.1