From ad5268b5815118fd5f01551b43e493214528f88d Mon Sep 17 00:00:00 2001 From: Francis Deslauriers Date: Tue, 26 Feb 2019 18:45:02 -0500 Subject: [PATCH] lib: replace trace is_static with destruction listeners General idea ------------ Replace bt_trace is_static infrastructure with the concept of destruction listener and extend it for the bt_trace_class object. This allows for downstream components to ask to be notify of the destruction of those objects. This notification is useful for component that keep data related trace IR objects that should be deallocated when the trace and/or trace class are destroyed. This feature is mostly useful to handle sources which could create and release lots of trace class and trace objects. In this case, when the source message iterator is done with the stream objects (emitted all the relevant stream end messages), it puts the stream, trace, and trace class references. Then downstream users are notified that specific external objects are about to be destroyed, so they release/close any associated resource. The flt.lttng-utils.debug-info component will make use of this capability to cleanup the mapping between input and output objects it needs to manage to do its function. Implementation -------------- Both trace class and trace offer the same interface and are implemented in the exact same way. Destruction listener callbacks and their associated private pointer are stored in an array and are all called at the execution of the destroy functions. Destruction listeners are registered using the following functions: bt_trace_status bt_trace_add_destruction_listener( const bt_trace *trace, bt_trace_destruction_listener_func listener, void *data, uint64_t *listener_id); bt_trace_class_status bt_trace_class_add_destruction_listener( const bt_trace_class *trace, bt_trace_class_destruction_listener_func listener, void *data, uint64_t *listener_id); It's possible to remove a destruction listener using the destruction listener id received when register it initially used the following functions: bt_trace_status bt_trace_remove_destruction_listener( const bt_trace *trace, uint64_t listener_id); bt_trace_class_status bt_trace_class_remove_destruction_listener( const bt_trace_class *trace_class, uint64_t listener_id); Signed-off-by: Francis Deslauriers --- .../babeltrace/trace-ir/trace-class-const.h | 11 ++ .../trace-ir/trace-class-internal.h | 1 + include/babeltrace/trace-ir/trace-const.h | 18 +-- include/babeltrace/trace-ir/trace-internal.h | 4 +- include/babeltrace/trace-ir/trace.h | 2 - lib/lib-logging.c | 2 - lib/trace-ir/stream.c | 2 - lib/trace-ir/trace-class.c | 106 ++++++++++++++ lib/trace-ir/trace.c | 136 ++++++------------ plugins/ctf/fs-src/fs.c | 9 -- plugins/text/dmesg/dmesg.c | 6 - 11 files changed, 167 insertions(+), 130 deletions(-) diff --git a/include/babeltrace/trace-ir/trace-class-const.h b/include/babeltrace/trace-ir/trace-class-const.h index 0f80f304..839463de 100644 --- a/include/babeltrace/trace-ir/trace-class-const.h +++ b/include/babeltrace/trace-ir/trace-class-const.h @@ -44,6 +44,9 @@ typedef enum bt_trace_class_status { BT_TRACE_CLASS_STATUS_NOMEM = -12, } bt_trace_class_status; +typedef void (* bt_trace_class_destruction_listener_func)( + const bt_trace_class *trace_class, void *data); + extern bt_bool bt_trace_class_assigns_automatic_stream_class_id( const bt_trace_class *trace_class); @@ -74,6 +77,14 @@ bt_trace_class_borrow_stream_class_by_index_const( extern const bt_stream_class *bt_trace_class_borrow_stream_class_by_id_const( const bt_trace_class *trace_class, uint64_t id); +extern bt_trace_class_status bt_trace_class_add_destruction_listener( + const bt_trace_class *trace_class, + bt_trace_class_destruction_listener_func listener, + void *data, uint64_t *listener_id); + +extern bt_trace_class_status bt_trace_class_remove_destruction_listener( + const bt_trace_class *trace_class, uint64_t listener_id); + extern void bt_trace_class_get_ref(const bt_trace_class *trace_class); extern void bt_trace_class_put_ref(const bt_trace_class *trace_class); diff --git a/include/babeltrace/trace-ir/trace-class-internal.h b/include/babeltrace/trace-ir/trace-class-internal.h index 2ffa3a3c..5afd3920 100644 --- a/include/babeltrace/trace-ir/trace-class-internal.h +++ b/include/babeltrace/trace-ir/trace-class-internal.h @@ -63,6 +63,7 @@ struct bt_trace_class { GPtrArray *stream_classes; bool assigns_automatic_stream_class_id; + GArray *destruction_listeners; bool frozen; }; diff --git a/include/babeltrace/trace-ir/trace-const.h b/include/babeltrace/trace-ir/trace-const.h index adb38240..15031029 100644 --- a/include/babeltrace/trace-ir/trace-const.h +++ b/include/babeltrace/trace-ir/trace-const.h @@ -44,11 +44,8 @@ typedef enum bt_trace_status { BT_TRACE_STATUS_NOMEM = -12, } bt_trace_status; -typedef void (* bt_trace_is_static_listener_func)(const bt_trace *trace, - void *data); - -typedef void (* bt_trace_listener_removed_func)(const bt_trace *trace, - void *data); +typedef void (* bt_trace_destruction_listener_func)( + const bt_trace *trace, void *data); extern const bt_trace_class *bt_trace_borrow_class_const( const bt_trace *trace); @@ -63,15 +60,12 @@ extern const bt_stream *bt_trace_borrow_stream_by_index_const( extern const bt_stream *bt_trace_borrow_stream_by_id_const( const bt_trace *trace, uint64_t id); -extern bt_bool bt_trace_is_static(const bt_trace *trace); - -extern bt_trace_status bt_trace_add_is_static_listener( +extern bt_trace_status bt_trace_add_destruction_listener( const bt_trace *trace, - bt_trace_is_static_listener_func listener, - bt_trace_listener_removed_func listener_removed, void *data, - uint64_t *listener_id); + bt_trace_destruction_listener_func listener, + void *data, uint64_t *listener_id); -extern bt_trace_status bt_trace_remove_is_static_listener( +extern bt_trace_status bt_trace_remove_destruction_listener( const bt_trace *trace, uint64_t listener_id); extern void bt_trace_get_ref(const bt_trace *trace); diff --git a/include/babeltrace/trace-ir/trace-internal.h b/include/babeltrace/trace-ir/trace-internal.h index aa4cee51..16076673 100644 --- a/include/babeltrace/trace-ir/trace-internal.h +++ b/include/babeltrace/trace-ir/trace-internal.h @@ -64,9 +64,7 @@ struct bt_trace { */ GHashTable *stream_classes_stream_count; - GArray *is_static_listeners; - bool is_static; - bool in_remove_listener; + GArray *destruction_listeners; bool frozen; }; diff --git a/include/babeltrace/trace-ir/trace.h b/include/babeltrace/trace-ir/trace.h index 0ae25f9a..36565dfc 100644 --- a/include/babeltrace/trace-ir/trace.h +++ b/include/babeltrace/trace-ir/trace.h @@ -52,8 +52,6 @@ extern bt_stream *bt_trace_borrow_stream_by_index(bt_trace *trace, extern bt_stream *bt_trace_borrow_stream_by_id(bt_trace *trace, uint64_t id); -extern bt_trace_status bt_trace_make_static(bt_trace *trace); - #ifdef __cplusplus } #endif diff --git a/lib/lib-logging.c b/lib/lib-logging.c index 4b5557aa..11187648 100644 --- a/lib/lib-logging.c +++ b/lib/lib-logging.c @@ -479,8 +479,6 @@ static inline void format_trace(char **buf_ch, bool extended, PRFIELD(trace->streams->len)); } - BUF_APPEND(", %sis-static=%d", PRFIELD(trace->is_static)); - if (!trace->class) { return; } diff --git a/lib/trace-ir/stream.c b/lib/trace-ir/stream.c index ced7d81f..5ae914ca 100644 --- a/lib/trace-ir/stream.c +++ b/lib/trace-ir/stream.c @@ -108,8 +108,6 @@ struct bt_stream *create_stream_with_id(struct bt_stream_class *stream_class, "%![sc-]+S, %![trace-]+t", stream_class, trace); BT_ASSERT_PRE(stream_id_is_unique(trace, stream_class, id), "Duplicate stream ID: %![trace-]+t, id=%" PRIu64, trace, id); - BT_ASSERT_PRE(!trace->is_static, - "Trace is static: %![trace-]+t", trace); BT_LIB_LOGD("Creating stream object: %![trace-]+t, id=%" PRIu64, trace, id); stream = g_new0(struct bt_stream, 1); diff --git a/lib/trace-ir/trace-class.c b/lib/trace-ir/trace-class.c index 7fa6a2f5..3aa398fb 100644 --- a/lib/trace-ir/trace-class.c +++ b/lib/trace-ir/trace-class.c @@ -54,6 +54,11 @@ #include #include +struct bt_trace_class_destruction_listener_elem { + bt_trace_class_destruction_listener_func func; + void *data; +}; + #define BT_ASSERT_PRE_TRACE_CLASS_HOT(_tc) \ BT_ASSERT_PRE_HOT((_tc), "Trace class", ": %!+T", (_tc)) @@ -63,6 +68,26 @@ void destroy_trace_class(struct bt_object *obj) struct bt_trace_class *tc = (void *) obj; BT_LIB_LOGD("Destroying trace class object: %!+T", tc); + /* + * Call destruction listener functions so that everything else + * still exists in the trace class. + */ + if (tc->destruction_listeners) { + uint64_t i; + BT_LIB_LOGV("Calling trace class destruction listener(s): %!+T", tc); + /* Call all the trace class destruction listeners */ + for (i = 0; i < tc->destruction_listeners->len; i++) { + struct bt_trace_class_destruction_listener_elem elem = + g_array_index(tc->destruction_listeners, + struct bt_trace_class_destruction_listener_elem, i); + + if (elem.func) { + elem.func(tc, elem.data); + } + } + g_array_free(tc->destruction_listeners, TRUE); + tc->destruction_listeners = NULL; + } if (tc->environment) { BT_LOGD_STR("Destroying environment attributes."); @@ -118,6 +143,13 @@ struct bt_trace_class *bt_trace_class_create(bt_self_component *self_comp) goto error; } + tc->destruction_listeners = g_array_new(FALSE, TRUE, + sizeof(struct bt_trace_class_destruction_listener_elem)); + if (!tc->destruction_listeners) { + BT_LOGE_STR("Failed to allocate one GArray."); + goto error; + } + tc->assigns_automatic_stream_class_id = true; BT_LIB_LOGD("Created trace class object: %!+T", tc); goto end; @@ -163,6 +195,80 @@ void bt_trace_class_set_uuid(struct bt_trace_class *tc, bt_uuid uuid) BT_LIB_LOGV("Set trace class's UUID: %!+T", tc); } +enum bt_trace_class_status bt_trace_class_add_destruction_listener( + const struct bt_trace_class *_tc, + bt_trace_class_destruction_listener_func listener, + void *data, uint64_t *listener_id) +{ + struct bt_trace_class *tc = (void *) _tc; + uint64_t i; + struct bt_trace_class_destruction_listener_elem new_elem = { + .func = listener, + .data = data, + }; + + BT_ASSERT_PRE_NON_NULL(tc, "Trace class"); + BT_ASSERT_PRE_NON_NULL(listener, "Listener"); + + /* Find the next available spot */ + for (i = 0; i < tc->destruction_listeners->len; i++) { + struct bt_trace_class_destruction_listener_elem elem = + g_array_index(tc->destruction_listeners, + struct bt_trace_class_destruction_listener_elem, i); + + if (!elem.func) { + break; + } + } + + if (i == tc->destruction_listeners->len) { + g_array_append_val(tc->destruction_listeners, new_elem); + } else { + g_array_insert_val(tc->destruction_listeners, i, new_elem); + } + + if (listener_id) { + *listener_id = i; + } + + BT_LIB_LOGV("Added trace class destruction listener: %![tc-]+T, " + "listener-id=%" PRIu64, tc, i); + return BT_TRACE_CLASS_STATUS_OK; +} + +BT_ASSERT_PRE_FUNC +static +bool has_listener_id(const struct bt_trace_class *tc, uint64_t listener_id) +{ + BT_ASSERT(listener_id < tc->destruction_listeners->len); + return (&g_array_index(tc->destruction_listeners, + struct bt_trace_class_destruction_listener_elem, + listener_id))->func != NULL; +} + +enum bt_trace_class_status bt_trace_class_remove_destruction_listener( + const struct bt_trace_class *_tc, uint64_t listener_id) +{ + struct bt_trace_class *tc = (void *) _tc; + struct bt_trace_class_destruction_listener_elem *elem; + + BT_ASSERT_PRE_NON_NULL(tc, "Trace class"); + BT_ASSERT_PRE(has_listener_id(tc, listener_id), + "Trace class has no such trace class destruction listener ID: " + "%![tc-]+T, %" PRIu64, tc, listener_id); + elem = &g_array_index(tc->destruction_listeners, + struct bt_trace_class_destruction_listener_elem, + listener_id); + BT_ASSERT(elem->func); + + elem->func = NULL; + elem->data = NULL; + BT_LIB_LOGV("Removed trace class destruction listener: " + "%![tc-]+T, listener-id=%" PRIu64, + tc, listener_id); + return BT_TRACE_CLASS_STATUS_OK; +} + BT_ASSERT_FUNC static bool trace_has_environment_entry(const struct bt_trace_class *tc, const char *name) diff --git a/lib/trace-ir/trace.c b/lib/trace-ir/trace.c index 2ef2b6f1..792d89f7 100644 --- a/lib/trace-ir/trace.c +++ b/lib/trace-ir/trace.c @@ -55,9 +55,8 @@ #include #include -struct bt_trace_is_static_listener_elem { - bt_trace_is_static_listener_func func; - bt_trace_listener_removed_func removed; +struct bt_trace_destruction_listener_elem { + bt_trace_destruction_listener_func func; void *data; }; @@ -72,24 +71,24 @@ void destroy_trace(struct bt_object *obj) BT_LIB_LOGD("Destroying trace object: %!+t", trace); /* - * Call remove listeners first so that everything else still - * exists in the trace. + * Call destruction listener functions so that everything else + * still exists in the trace. */ - if (trace->is_static_listeners) { - size_t i; - - for (i = 0; i < trace->is_static_listeners->len; i++) { - struct bt_trace_is_static_listener_elem elem = - g_array_index(trace->is_static_listeners, - struct bt_trace_is_static_listener_elem, i); - - if (elem.removed) { - elem.removed((void *) trace, elem.data); + if (trace->destruction_listeners) { + uint64_t i; + BT_LIB_LOGV("Calling trace destruction listener(s): %!+t", trace); + /* Call all the trace destruction listeners */ + for (i = 0; i < trace->destruction_listeners->len; i++) { + struct bt_trace_destruction_listener_elem elem = + g_array_index(trace->destruction_listeners, + struct bt_trace_destruction_listener_elem, i); + + if (elem.func) { + elem.func(trace, elem.data); } } - - g_array_free(trace->is_static_listeners, TRUE); - trace->is_static_listeners = NULL; + g_array_free(trace->destruction_listeners, TRUE); + trace->destruction_listeners = NULL; } if (trace->name.str) { @@ -147,9 +146,9 @@ struct bt_trace *bt_trace_create(struct bt_trace_class *tc) goto error; } - trace->is_static_listeners = g_array_new(FALSE, TRUE, - sizeof(struct bt_trace_is_static_listener_elem)); - if (!trace->is_static_listeners) { + trace->destruction_listeners = g_array_new(FALSE, TRUE, + sizeof(struct bt_trace_destruction_listener_elem)); + if (!trace->destruction_listeners) { BT_LOGE_STR("Failed to allocate one GArray."); goto error; } @@ -231,79 +230,44 @@ const struct bt_stream *bt_trace_borrow_stream_by_id_const( return bt_trace_borrow_stream_by_id((void *) trace, id); } -bt_bool bt_trace_is_static(const struct bt_trace *trace) -{ - BT_ASSERT_PRE_NON_NULL(trace, "Trace"); - return (bt_bool) trace->is_static; -} - -enum bt_trace_status bt_trace_make_static(struct bt_trace *trace) -{ uint64_t i; - - BT_ASSERT_PRE_NON_NULL(trace, "Trace"); - trace->is_static = true; - bt_trace_freeze(trace); - BT_LIB_LOGV("Trace is now static: %!+t", trace); - - /* Call all the "trace is static" listeners */ - for (i = 0; i < trace->is_static_listeners->len; i++) { - struct bt_trace_is_static_listener_elem elem = - g_array_index(trace->is_static_listeners, - struct bt_trace_is_static_listener_elem, i); - - if (elem.func) { - elem.func((void *) trace, elem.data); - } - } - - return BT_TRACE_STATUS_OK; -} - -enum bt_trace_status bt_trace_add_is_static_listener( +enum bt_trace_status bt_trace_add_destruction_listener( const struct bt_trace *c_trace, - bt_trace_is_static_listener_func listener, - bt_trace_listener_removed_func listener_removed, void *data, - uint64_t *listener_id) + bt_trace_destruction_listener_func listener, + void *data, uint64_t *listener_id) { struct bt_trace *trace = (void *) c_trace; uint64_t i; - struct bt_trace_is_static_listener_elem new_elem = { + struct bt_trace_destruction_listener_elem new_elem = { .func = listener, - .removed = listener_removed, .data = data, }; BT_ASSERT_PRE_NON_NULL(trace, "Trace"); BT_ASSERT_PRE_NON_NULL(listener, "Listener"); - BT_ASSERT_PRE(!trace->is_static, - "Trace is already static: %!+t", trace); - BT_ASSERT_PRE(trace->in_remove_listener, - "Cannot call this function while executing a " - "remove listener: %!+t", trace); /* Find the next available spot */ - for (i = 0; i < trace->is_static_listeners->len; i++) { - struct bt_trace_is_static_listener_elem elem = - g_array_index(trace->is_static_listeners, - struct bt_trace_is_static_listener_elem, i); + for (i = 0; i < trace->destruction_listeners->len; i++) { + struct bt_trace_destruction_listener_elem elem = + g_array_index(trace->destruction_listeners, + struct bt_trace_destruction_listener_elem, i); if (!elem.func) { break; } } - if (i == trace->is_static_listeners->len) { - g_array_append_val(trace->is_static_listeners, new_elem); + if (i == trace->destruction_listeners->len) { + g_array_append_val(trace->destruction_listeners, new_elem); } else { - g_array_insert_val(trace->is_static_listeners, i, new_elem); + g_array_insert_val(trace->destruction_listeners, i, new_elem); } if (listener_id) { *listener_id = i; } - BT_LIB_LOGV("Added \"trace is static\" listener: " - "%![trace-]+t, listener-id=%" PRIu64, trace, i); + BT_LIB_LOGV("Added destruction listener: " "%![trace-]+t, " + "listener-id=%" PRIu64, trace, i); return BT_TRACE_STATUS_OK; } @@ -311,46 +275,30 @@ BT_ASSERT_PRE_FUNC static bool has_listener_id(const struct bt_trace *trace, uint64_t listener_id) { - BT_ASSERT(listener_id < trace->is_static_listeners->len); - return (&g_array_index(trace->is_static_listeners, - struct bt_trace_is_static_listener_elem, + BT_ASSERT(listener_id < trace->destruction_listeners->len); + return (&g_array_index(trace->destruction_listeners, + struct bt_trace_destruction_listener_elem, listener_id))->func != NULL; } -enum bt_trace_status bt_trace_remove_is_static_listener( +enum bt_trace_status bt_trace_remove_destruction_listener( const struct bt_trace *c_trace, uint64_t listener_id) { struct bt_trace *trace = (void *) c_trace; - struct bt_trace_is_static_listener_elem *elem; + struct bt_trace_destruction_listener_elem *elem; BT_ASSERT_PRE_NON_NULL(trace, "Trace"); - BT_ASSERT_PRE(!trace->is_static, - "Trace is already static: %!+t", trace); - BT_ASSERT_PRE(trace->in_remove_listener, - "Cannot call this function while executing a " - "remove listener: %!+t", trace); BT_ASSERT_PRE(has_listener_id(trace, listener_id), - "Trace has no such \"trace is static\" listener ID: " + "Trace has no such trace destruction listener ID: " "%![trace-]+t, %" PRIu64, trace, listener_id); - elem = &g_array_index(trace->is_static_listeners, - struct bt_trace_is_static_listener_elem, + elem = &g_array_index(trace->destruction_listeners, + struct bt_trace_destruction_listener_elem, listener_id); BT_ASSERT(elem->func); - if (elem->removed) { - /* Call remove listener */ - BT_LIB_LOGV("Calling remove listener: " - "%![trace-]+t, listener-id=%" PRIu64, - trace, listener_id); - trace->in_remove_listener = true; - elem->removed((void *) trace, elem->data); - trace->in_remove_listener = false; - } - elem->func = NULL; - elem->removed = NULL; elem->data = NULL; - BT_LIB_LOGV("Removed \"trace is static\" listener: " + BT_LIB_LOGV("Removed \"trace destruction listener: " "%![trace-]+t, listener-id=%" PRIu64, trace, listener_id); return BT_TRACE_STATUS_OK; diff --git a/plugins/ctf/fs-src/fs.c b/plugins/ctf/fs-src/fs.c index 6260a248..901c4956 100644 --- a/plugins/ctf/fs-src/fs.c +++ b/plugins/ctf/fs-src/fs.c @@ -1023,15 +1023,6 @@ struct ctf_fs_trace *ctf_fs_trace_create(bt_self_component_source *self_comp, goto error; } - /* - * create_ds_file_groups() created all the streams that this - * trace needs. There won't be any more. Therefore it is safe to - * make this trace static. - */ - if (ctf_fs_trace->trace) { - (void) bt_trace_make_static(ctf_fs_trace->trace); - } - goto end; error: diff --git a/plugins/text/dmesg/dmesg.c b/plugins/text/dmesg/dmesg.c index d8afa858..c93fa78e 100644 --- a/plugins/text/dmesg/dmesg.c +++ b/plugins/text/dmesg/dmesg.c @@ -299,12 +299,6 @@ int create_packet_and_stream_and_trace(struct dmesg_component *dmesg_comp) goto error; } - ret = bt_trace_make_static(dmesg_comp->trace); - if (ret) { - BT_LOGE_STR("Cannot make trace static."); - goto error; - } - goto end; error: -- 2.34.1