From 6c677fb54f458456e3b5a15cffa6774bd7a86f54 Mon Sep 17 00:00:00 2001 From: Philippe Proulx Date: Tue, 12 Jun 2018 16:33:14 -0400 Subject: [PATCH] lib: remove useless checks, make functions inline on fast path This patch: * Removes a few useless checks on the fast path with the help of bt_object_put_no_null_check(), bt_object_get_no_null_check(), and bt_object_get_no_null_check_no_parent_check() when it's possible. * Makes setting any frozen flag on the fast path only happen in developer mode. * Makes fast path functions `static inline` (specific creating and recycling functions). Signed-off-by: Philippe Proulx Change-Id: If40b5f8dea9a085e8e30e25a9542767f3eb9815e --- include/babeltrace/ctf-ir/event-internal.h | 167 +++++++++++++-- include/babeltrace/ctf-ir/fields-internal.h | 38 ++-- include/babeltrace/ctf-ir/packet-internal.h | 6 +- .../babeltrace/graph/notification-internal.h | 7 +- include/babeltrace/object-internal.h | 16 ++ lib/ctf-ir/event.c | 193 ++---------------- lib/ctf-ir/packet.c | 30 ++- lib/ctf-writer/event.c | 2 +- lib/ctf-writer/stream.c | 2 +- lib/graph/notification/event.c | 75 ++++--- lib/graph/notification/packet.c | 26 +-- 11 files changed, 271 insertions(+), 291 deletions(-) diff --git a/include/babeltrace/ctf-ir/event-internal.h b/include/babeltrace/ctf-ir/event-internal.h index e17ff29c..fe5e64de 100644 --- a/include/babeltrace/ctf-ir/event-internal.h +++ b/include/babeltrace/ctf-ir/event-internal.h @@ -33,9 +33,11 @@ #include #include #include +#include #include #include #include +#include #include #include #include @@ -66,19 +68,20 @@ BT_HIDDEN int _bt_event_common_validate(struct bt_event_common *event); BT_HIDDEN -void _bt_event_common_freeze(struct bt_event_common *event); +void _bt_event_common_set_is_frozen(struct bt_event_common *event, + bool is_frozen); BT_HIDDEN -void _bt_event_freeze(struct bt_event *event); +void _bt_event_set_is_frozen(struct bt_event *event, bool is_frozen); #ifdef BT_DEV_MODE -# define bt_event_common_validate _bt_event_common_validate -# define bt_event_common_freeze _bt_event_common_freeze -# define bt_event_freeze _bt_event_freeze +# define bt_event_common_validate _bt_event_common_validate +# define bt_event_common_set_is_frozen _bt_event_common_set_is_frozen +# define bt_event_set_is_frozen _bt_event_set_is_frozen #else -# define bt_event_common_validate(_event) 0 -# define bt_event_common_freeze(_event) -# define bt_event_freeze(_event) +# define bt_event_common_validate(_event) 0 +# define bt_event_common_set_is_frozen(_event, _is_frozen) +# define bt_event_set_is_frozen(_event, _is_frozen) #endif #define BT_ASSERT_PRE_EVENT_COMMON_HOT(_event, _name) \ @@ -248,20 +251,152 @@ void bt_event_common_finalize(struct bt_object *obj, } } -BT_HIDDEN -struct bt_event *bt_event_new(struct bt_event_class *event_class); +BT_UNUSED +static inline +void _bt_event_reset_dev_mode(struct bt_event *event) +{ + GHashTableIter iter; + gpointer key, value; -BT_HIDDEN -struct bt_event *bt_event_create(struct bt_event_class *event_class, - struct bt_packet *packet); + BT_ASSERT(event); -BT_HIDDEN -void bt_event_recycle(struct bt_event *event); + if (event->common.header_field) { + bt_field_common_set_is_frozen_recursive( + event->common.header_field->field, false); + bt_field_common_reset_recursive( + event->common.header_field->field); + } + + if (event->common.stream_event_context_field) { + bt_field_common_set_is_frozen_recursive( + event->common.stream_event_context_field, false); + bt_field_common_reset_recursive( + event->common.stream_event_context_field); + } + + if (event->common.context_field) { + bt_field_common_set_is_frozen_recursive( + event->common.context_field, false); + bt_field_common_reset_recursive(event->common.context_field); + } + + if (event->common.payload_field) { + bt_field_common_set_is_frozen_recursive( + event->common.payload_field, false); + bt_field_common_reset_recursive(event->common.payload_field); + } + + g_hash_table_iter_init(&iter, event->clock_values); + while (g_hash_table_iter_next(&iter, &key, &value)) { + struct bt_clock_value *clock_value = value; + + BT_ASSERT(clock_value); + bt_clock_value_reset(clock_value); + bt_clock_value_set_is_frozen(clock_value, false); + } +} + +#ifdef BT_DEV_MODE +# define bt_event_reset_dev_mode _bt_event_reset_dev_mode +#else +# define bt_event_reset_dev_mode(_x) +#endif BT_HIDDEN void bt_event_destroy(struct bt_event *event); +static inline +void bt_event_reset(struct bt_event *event) +{ + BT_ASSERT(event); + bt_event_set_is_frozen(event, false); + bt_event_reset_dev_mode(event); + bt_object_put_no_null_check(&event->packet->base); + event->packet = NULL; +} + +static inline +void bt_event_recycle(struct bt_event *event) +{ + struct bt_event_class *event_class; + + BT_ASSERT(event); + BT_LIB_LOGD("Recycling event: %!+e", event); + + /* + * Those are the important ordered steps: + * + * 1. Reset the event object (put any permanent reference it + * has, unfreeze it and its fields in developer mode, etc.), + * but do NOT put its class's reference. This event class + * contains the pool to which we're about to recycle this + * event object, so we must guarantee its existence thanks + * to this existing reference. + * + * 2. Move the event class reference to our `event_class` + * variable so that we can set the event's class member + * to NULL before recycling it. We CANNOT do this after + * we put the event class reference because this bt_put() + * could destroy the event class, also destroying its + * event pool, thus also destroying our event object (this + * would result in an invalid write access). + * + * 3. Recycle the event object. + * + * 4. Put our event class reference. + */ + bt_event_reset(event); + event_class = BT_FROM_COMMON(event->common.class); + BT_ASSERT(event_class); + event->common.class = NULL; + bt_object_pool_recycle_object(&event_class->event_pool, event); + bt_object_put_no_null_check(&event_class->common.base); +} + +static inline +void bt_event_set_packet(struct bt_event *event, struct bt_packet *packet) +{ + BT_ASSERT_PRE_NON_NULL(event, "Event"); + BT_ASSERT_PRE_NON_NULL(packet, "Packet"); + BT_ASSERT_PRE_EVENT_COMMON_HOT(BT_TO_COMMON(event), "Event"); + BT_ASSERT(!event->packet); + event->packet = packet; + bt_object_get_no_null_check_no_parent_check(&event->packet->base); + BT_LOGV("Set event's packet: event-addr=%p, " + "event-class-name=\"%s\", event-class-id=%" PRId64 ", " + "packet-addr=%p", + event, bt_event_class_common_get_name(event->common.class), + bt_event_class_common_get_id(event->common.class), packet); +} + +static inline +struct bt_event *bt_event_create(struct bt_event_class *event_class, + struct bt_packet *packet) +{ + struct bt_event *event = NULL; + + BT_ASSERT(event_class); + event = bt_object_pool_create_object(&event_class->event_pool); + if (unlikely(!event)) { + BT_LIB_LOGE("Cannot allocate one event from event class's event pool: " + "%![event-class-]+E", event_class); + goto end; + } + + if (unlikely(!event->common.class)) { + event->common.class = BT_TO_COMMON(event_class); + bt_object_get_no_null_check(&event_class->common.base); + } + + BT_ASSERT(packet); + bt_event_set_packet(event, packet); + goto end; + +end: + return event; +} + BT_HIDDEN -int bt_event_set_packet(struct bt_event *event, struct bt_packet *packet); +struct bt_event *bt_event_new(struct bt_event_class *event_class); #endif /* BABELTRACE_CTF_IR_EVENT_INTERNAL_H */ diff --git a/include/babeltrace/ctf-ir/fields-internal.h b/include/babeltrace/ctf-ir/fields-internal.h index 115d4ffb..d6da0a4a 100644 --- a/include/babeltrace/ctf-ir/fields-internal.h +++ b/include/babeltrace/ctf-ir/fields-internal.h @@ -396,9 +396,12 @@ int bt_field_common_sequence_set_length(struct bt_field_common *field, struct bt_field_common_sequence *sequence = BT_FROM_COMMON(field); BT_ASSERT_PRE_NON_NULL(field, "Sequence field"); + BT_ASSERT_PRE(((int64_t) length) >= 0, + "Invalid sequence length (too large): length=%" PRId64, + length); BT_ASSERT_PRE_FIELD_COMMON_HOT(field, "Sequence field"); - if (length > sequence->elements->len) { + if (unlikely(length > sequence->elements->len)) { /* Make more room */ struct bt_field_type_common_sequence *sequence_ft; uint64_t cur_len = sequence->elements->len; @@ -654,27 +657,15 @@ const char *bt_field_common_string_get_value(struct bt_field_common *field) } static inline -int bt_field_common_string_set_value(struct bt_field_common *field, - const char *value) +int bt_field_common_string_clear(struct bt_field_common *field) { - struct bt_field_common_string *string = BT_FROM_COMMON(field); - size_t str_len; + struct bt_field_common_string *string_field = BT_FROM_COMMON(field); BT_ASSERT_PRE_NON_NULL(field, "String field"); - BT_ASSERT_PRE_NON_NULL(value, "Value"); BT_ASSERT_PRE_FIELD_COMMON_HOT(field, "String field"); BT_ASSERT_PRE_FIELD_COMMON_HAS_TYPE_ID(field, BT_FIELD_TYPE_ID_STRING, "Field"); - - str_len = strlen(value); - - if (str_len + 1 > string->buf->len) { - g_array_set_size(string->buf, str_len + 1); - } - - memcpy(string->buf->data, value, str_len); - ((char *) string->buf->data)[str_len] = '\0'; - string->size = str_len; + string_field->size = 0; bt_field_common_set(field, true); return 0; } @@ -700,7 +691,7 @@ int bt_field_common_string_append_len(struct bt_field_common *field, new_size = string_field->size + length; - if (new_size + 1 > string_field->buf->len) { + if (unlikely(new_size + 1 > string_field->buf->len)) { g_array_set_size(string_field->buf, new_size + 1); } @@ -717,23 +708,22 @@ int bt_field_common_string_append(struct bt_field_common *field, const char *value) { BT_ASSERT_PRE_NON_NULL(value, "Value"); - return bt_field_common_string_append_len(field, value, strlen(value)); } static inline -int bt_field_common_string_clear(struct bt_field_common *field) +int bt_field_common_string_set_value(struct bt_field_common *field, + const char *value) { - struct bt_field_common_string *string_field = BT_FROM_COMMON(field); - BT_ASSERT_PRE_NON_NULL(field, "String field"); + BT_ASSERT_PRE_NON_NULL(value, "Value"); BT_ASSERT_PRE_FIELD_COMMON_HOT(field, "String field"); BT_ASSERT_PRE_FIELD_COMMON_HAS_TYPE_ID(field, BT_FIELD_TYPE_ID_STRING, "Field"); - string_field->size = 0; - bt_field_common_set(field, true); - return 0; + bt_field_common_string_clear(field); + return bt_field_common_string_append_len(field, + value, strlen(value)); } static inline diff --git a/include/babeltrace/ctf-ir/packet-internal.h b/include/babeltrace/ctf-ir/packet-internal.h index 4cbf0ac8..37aeab7b 100644 --- a/include/babeltrace/ctf-ir/packet-internal.h +++ b/include/babeltrace/ctf-ir/packet-internal.h @@ -41,12 +41,12 @@ struct bt_packet { }; BT_HIDDEN -void _bt_packet_freeze(struct bt_packet *packet); +void _bt_packet_set_is_frozen(struct bt_packet *packet, bool is_frozen); #ifdef BT_DEV_MODE -# define bt_packet_freeze _bt_packet_freeze +# define bt_packet_set_is_frozen _bt_packet_set_is_frozen #else -# define bt_packet_freeze(_packet) +# define bt_packet_set_is_frozen(_packet, _is_frozen) #endif /* BT_DEV_MODE */ BT_HIDDEN diff --git a/include/babeltrace/graph/notification-internal.h b/include/babeltrace/graph/notification-internal.h index 243aac09..5b7ce539 100644 --- a/include/babeltrace/graph/notification-internal.h +++ b/include/babeltrace/graph/notification-internal.h @@ -29,6 +29,7 @@ #include #include +#include #include #include #include @@ -77,7 +78,7 @@ struct bt_notification *bt_notification_create_from_pool( { struct bt_notification *notif = bt_object_pool_create_object(pool); - if (!notif) { + if (unlikely(!notif)) { #ifdef BT_LIB_LOGE BT_LIB_LOGE("Cannot allocate one notification from notification pool: " "%![pool-]+o, %![graph-]+g", pool, graph); @@ -85,14 +86,14 @@ struct bt_notification *bt_notification_create_from_pool( goto error; } - if (!notif->graph) { + if (likely(!notif->graph)) { notif->graph = graph; } goto end; error: - BT_PUT(notif); + BT_ASSERT(!notif); end: return notif; diff --git a/include/babeltrace/object-internal.h b/include/babeltrace/object-internal.h index 70391b3b..6eed014c 100644 --- a/include/babeltrace/object-internal.h +++ b/include/babeltrace/object-internal.h @@ -251,6 +251,22 @@ void bt_object_inc_ref_count(struct bt_object *obj) BT_ASSERT(obj->ref_count != 0); } +static inline +void bt_object_get_no_null_check_no_parent_check(struct bt_object *obj) +{ + BT_ASSERT(obj); + BT_ASSERT(obj->is_shared); + +#ifdef BT_LOGV + BT_LOGV("Incrementing object's reference count: %llu -> %llu: " + "addr=%p, cur-count=%llu, new-count=%llu", + obj->ref_count, obj->ref_count + 1, + obj, obj->ref_count, obj->ref_count + 1); +#endif + + bt_object_inc_ref_count(obj); +} + static inline void bt_object_get_no_null_check(struct bt_object *obj) { diff --git a/lib/ctf-ir/event.c b/lib/ctf-ir/event.c index 9fa9223e..ea552727 100644 --- a/lib/ctf-ir/event.c +++ b/lib/ctf-ir/event.c @@ -278,7 +278,8 @@ end: } BT_HIDDEN -void _bt_event_common_freeze(struct bt_event_common *event) +void _bt_event_common_set_is_frozen(struct bt_event_common *event, + bool is_frozen) { BT_ASSERT(event); @@ -294,26 +295,28 @@ void _bt_event_common_freeze(struct bt_event_common *event) if (event->header_field) { BT_LOGD_STR("Freezing event's header field."); bt_field_common_set_is_frozen_recursive( - event->header_field->field, true); + event->header_field->field, is_frozen); } if (event->stream_event_context_field) { BT_LOGD_STR("Freezing event's stream event context field."); bt_field_common_set_is_frozen_recursive( - event->stream_event_context_field, true); + event->stream_event_context_field, is_frozen); } if (event->context_field) { BT_LOGD_STR("Freezing event's context field."); - bt_field_common_set_is_frozen_recursive(event->context_field, true); + bt_field_common_set_is_frozen_recursive(event->context_field, + is_frozen); } if (event->payload_field) { BT_LOGD_STR("Freezing event's payload field."); - bt_field_common_set_is_frozen_recursive(event->payload_field, true); + bt_field_common_set_is_frozen_recursive(event->payload_field, + is_frozen); } - event->frozen = 1; + event->frozen = is_frozen; } BT_HIDDEN @@ -622,105 +625,6 @@ end: return event; } -BT_ASSERT_PRE_FUNC -static inline -void _bt_event_reset_dev_mode(struct bt_event *event) -{ - GHashTableIter iter; - gpointer key, value; - - BT_ASSERT(event); - - if (event->common.header_field) { - bt_field_common_set_is_frozen_recursive( - event->common.header_field->field, false); - bt_field_common_reset_recursive( - event->common.header_field->field); - } - - if (event->common.stream_event_context_field) { - bt_field_common_set_is_frozen_recursive( - event->common.stream_event_context_field, false); - bt_field_common_reset_recursive( - event->common.stream_event_context_field); - } - - if (event->common.context_field) { - bt_field_common_set_is_frozen_recursive( - event->common.context_field, false); - bt_field_common_reset_recursive(event->common.context_field); - } - - if (event->common.payload_field) { - bt_field_common_set_is_frozen_recursive( - event->common.payload_field, false); - bt_field_common_reset_recursive(event->common.payload_field); - } - - g_hash_table_iter_init(&iter, event->clock_values); - while (g_hash_table_iter_next(&iter, &key, &value)) { - struct bt_clock_value *clock_value = value; - - BT_ASSERT(clock_value); - bt_clock_value_reset(clock_value); - bt_clock_value_set_is_frozen(clock_value, false); - } -} - -#ifdef BT_DEV_MODE -# define bt_event_reset_dev_mode _bt_event_reset_dev_mode -#else -# define bt_event_reset_dev_mode(_x) -#endif - -static inline -void bt_event_reset(struct bt_event *event) -{ - BT_ASSERT(event); - event->common.frozen = false; - bt_event_reset_dev_mode(event); - BT_PUT(event->packet); -} - -BT_HIDDEN -struct bt_event *bt_event_create(struct bt_event_class *event_class, - struct bt_packet *packet) -{ - int ret; - struct bt_event *event = NULL; - - BT_ASSERT(event_class); - event = bt_object_pool_create_object(&event_class->event_pool); - if (!event) { - BT_LIB_LOGE("Cannot allocate one event from event class's event pool: " - "%![event-class-]+E", event_class); - goto error; - } - - if (!event->common.class) { - event->common.class = bt_get(event_class); - } - - BT_ASSERT(packet); - ret = bt_event_set_packet(event, packet); - if (ret) { - BT_LIB_LOGE("Cannot set event's packet: " - "%![event-]+e, %![packet-]+a", event, packet); - goto error; - } - - goto end; - -error: - if (event) { - bt_event_recycle(event); - event = NULL; - } - -end: - return event; -} - struct bt_event_class *bt_event_borrow_class(struct bt_event *event) { BT_ASSERT_PRE_NON_NULL(event, "Event"); @@ -833,84 +737,11 @@ end: } BT_HIDDEN -int bt_event_set_packet(struct bt_event *event, struct bt_packet *packet) +void _bt_event_set_is_frozen(struct bt_event *event, bool is_frozen) { - BT_ASSERT_PRE_NON_NULL(event, "Event"); - BT_ASSERT_PRE_NON_NULL(packet, "Packet"); - BT_ASSERT_PRE_EVENT_COMMON_HOT(BT_TO_COMMON(event), "Event"); - - /* - * Make sure the new packet was created by this event's - * stream, if it is set. - */ - if (bt_event_borrow_stream(event)) { - BT_ASSERT_PRE(packet->stream == bt_event_borrow_stream(event), - "Packet's stream and event's stream differ: " - "%![event-]+e, %![packet-]+a", - event, packet); - } else { - BT_ASSERT_PRE(bt_event_class_borrow_stream_class( - BT_FROM_COMMON(event->common.class)) == - BT_FROM_COMMON(packet->stream->common.stream_class), - "Packet's stream class and event's stream class differ: " - "%![event-]+e, %![packet-]+a", - event, packet); - } - - bt_get(packet); - BT_MOVE(event->packet, packet); - BT_LOGV("Set event's packet: event-addr=%p, " - "event-class-name=\"%s\", event-class-id=%" PRId64 ", " - "packet-addr=%p", - event, bt_event_class_common_get_name(event->common.class), - bt_event_class_common_get_id(event->common.class), packet); - return 0; -} - -BT_HIDDEN -void _bt_event_freeze(struct bt_event *event) -{ - _bt_event_common_freeze(BT_TO_COMMON(event)); + bt_event_common_set_is_frozen(BT_TO_COMMON(event), is_frozen); BT_LOGD_STR("Freezing event's packet."); - bt_packet_freeze(event->packet); -} - -BT_HIDDEN -void bt_event_recycle(struct bt_event *event) -{ - struct bt_event_class *event_class; - - BT_ASSERT(event); - BT_LIB_LOGD("Recycling event: %!+e", event); - - /* - * Those are the important ordered steps: - * - * 1. Reset the event object (put any permanent reference it - * has, unfreeze it and its fields in developer mode, etc.), - * but do NOT put its class's reference. This event class - * contains the pool to which we're about to recycle this - * event object, so we must guarantee its existence thanks - * to this existing reference. - * - * 2. Move the event class reference to our `event_class` - * variable so that we can set the event's class member - * to NULL before recycling it. We CANNOT do this after - * we put the event class reference because this bt_put() - * could destroy the event class, also destroying its - * event pool, thus also destroying our event object (this - * would result in an invalid write access). - * - * 3. Recycle the event object. - * - * 4. Put our event class reference. - */ - bt_event_reset(event); - event_class = BT_FROM_COMMON(event->common.class); - BT_ASSERT(event_class); - event->common.class = NULL; - bt_object_pool_recycle_object(&event_class->event_pool, event); - bt_put(event_class); + bt_packet_set_is_frozen(event->packet, is_frozen); } int bt_event_move_header(struct bt_event *event, diff --git a/lib/ctf-ir/packet.c b/lib/ctf-ir/packet.c index 227a7fd9..7df3d32e 100644 --- a/lib/ctf-ir/packet.c +++ b/lib/ctf-ir/packet.c @@ -62,7 +62,7 @@ struct bt_field *bt_packet_borrow_context(struct bt_packet *packet) } BT_HIDDEN -void _bt_packet_freeze(struct bt_packet *packet) +void _bt_packet_set_is_frozen(struct bt_packet *packet, bool is_frozen) { if (!packet || packet->frozen) { return; @@ -72,22 +72,24 @@ void _bt_packet_freeze(struct bt_packet *packet) if (packet->header) { BT_LOGD_STR("Freezing packet's header field."); - bt_field_set_is_frozen_recursive((void *) packet->header->field, true); + bt_field_set_is_frozen_recursive((void *) packet->header->field, + is_frozen); } if (packet->context) { BT_LOGD_STR("Freezing packet's context field."); - bt_field_set_is_frozen_recursive((void *) packet->context->field, true); + bt_field_set_is_frozen_recursive((void *) packet->context->field, + is_frozen); } - packet->frozen = 1; + packet->frozen = is_frozen; } static inline void bt_packet_reset(struct bt_packet *packet) { BT_ASSERT(packet); - packet->frozen = false; + bt_packet_set_is_frozen(packet, false); if (packet->header) { bt_field_set_is_frozen_recursive( @@ -161,7 +163,7 @@ void bt_packet_recycle(struct bt_packet *packet) BT_ASSERT(stream); packet->stream = NULL; bt_object_pool_recycle_object(&stream->packet_pool, packet); - bt_put(stream); + bt_object_put_no_null_check(&stream->common.base); } BT_HIDDEN @@ -263,24 +265,20 @@ struct bt_packet *bt_packet_create(struct bt_stream *stream) BT_ASSERT_PRE_NON_NULL(stream, "Stream"); packet = bt_object_pool_create_object(&stream->packet_pool); - if (!packet) { + if (unlikely(!packet)) { BT_LIB_LOGE("Cannot allocate one packet from stream's packet pool: " "%![stream-]+s", stream); - goto error; + goto end; } - if (!packet->stream) { - packet->stream = bt_get(stream); + if (unlikely(!packet->stream)) { + packet->stream = stream; + bt_object_get_no_null_check_no_parent_check( + &packet->stream->common.base); } goto end; -error: - if (packet) { - bt_packet_recycle(packet); - packet = NULL; - } - end: return packet; } diff --git a/lib/ctf-writer/event.c b/lib/ctf-writer/event.c index 1e61cb98..b2a7f232 100644 --- a/lib/ctf-writer/event.c +++ b/lib/ctf-writer/event.c @@ -287,7 +287,7 @@ end: BT_HIDDEN void _bt_ctf_event_freeze(struct bt_ctf_event *event) { - _bt_event_common_freeze(BT_TO_COMMON(event)); + _bt_event_common_set_is_frozen(BT_TO_COMMON(event), true); } int bt_ctf_event_set_header(struct bt_ctf_event *event, diff --git a/lib/ctf-writer/stream.c b/lib/ctf-writer/stream.c index 5ed1ebaf..4145a0ca 100644 --- a/lib/ctf-writer/stream.c +++ b/lib/ctf-writer/stream.c @@ -1373,7 +1373,7 @@ int bt_ctf_stream_append_event(struct bt_ctf_stream *stream, /* Save the new event and freeze it */ BT_LOGV_STR("Freezing the event to append."); - bt_event_common_freeze(BT_TO_COMMON(event)); + bt_event_common_set_is_frozen(BT_TO_COMMON(event), true); g_ptr_array_add(stream->events, event); /* diff --git a/lib/graph/notification/event.c b/lib/graph/notification/event.c index 3ed1c4cd..ad5d5dd1 100644 --- a/lib/graph/notification/event.c +++ b/lib/graph/notification/event.c @@ -28,6 +28,7 @@ #include #include +#include #include #include #include @@ -82,22 +83,11 @@ struct bt_notification *bt_notification_event_create( struct bt_clock_class_priority_map *cc_prio_map) { struct bt_notification_event *notification = NULL; + struct bt_event *event; BT_ASSERT_PRE_NON_NULL(event_class, "Event class"); BT_ASSERT_PRE_NON_NULL(packet, "Packet"); - - if (cc_prio_map) { - /* Function's reference, released at the end */ - bt_get(cc_prio_map); - } else { - cc_prio_map = bt_clock_class_priority_map_create(); - if (!cc_prio_map) { - BT_LOGE_STR("Cannot create empty clock class priority map."); - goto error; - } - } - - BT_ASSERT(cc_prio_map); + BT_ASSERT_PRE_NON_NULL(cc_prio_map, "Clock class priority map"); BT_LOGD("Creating event notification object: " "event-class-addr=%p, " "event-class-name=\"%s\", event-class-id=%" PRId64 ", " @@ -105,24 +95,43 @@ struct bt_notification *bt_notification_event_create( event_class, bt_event_class_get_name(event_class), bt_event_class_get_id(event_class), cc_prio_map); - BT_ASSERT_PRE(event_class_has_trace(event_class), "Event class is not part of a trace: %!+E", event_class); - notification = (void *) bt_notification_create_from_pool( - &graph->event_notif_pool, graph); - if (!notification) { - /* bt_notification_create_from_pool() logs errors */ + event = bt_event_create(event_class, packet); + if (unlikely(!event)) { + BT_LIB_LOGE("Cannot create event from event class: " + "%![event-class-]+E", event_class); goto error; } - notification->event = bt_event_create(event_class, packet); - if (!notification->event) { - BT_LIB_LOGE("Cannot create event from event class: " - "%![event-class-]+E", event_class); + /* + * Create notification from pool _after_ we have everything + * (in this case, a valid event object) so that we never have an + * error condition with a non-NULL notification object. + * Otherwise: + * + * * We cannot recycle the notification on error because + * bt_notification_event_recycle() expects a complete + * notification (and the event or clock class priority map + * object could be unset). + * + * * We cannot destroy the notification because we would need + * to notify the graph (pool owner) so that it removes the + * notification from its notification array. + */ + notification = (void *) bt_notification_create_from_pool( + &graph->event_notif_pool, graph); + if (unlikely(!notification)) { + /* bt_notification_create_from_pool() logs errors */ goto error; } - notification->cc_prio_map = bt_get(cc_prio_map); + BT_ASSERT(!notification->cc_prio_map); + notification->cc_prio_map = cc_prio_map; + bt_object_get_no_null_check_no_parent_check( + ¬ification->cc_prio_map->base); + BT_ASSERT(!notification->event); + notification->event = event; BT_LOGD_STR("Freezing event notification's clock class priority map."); bt_clock_class_priority_map_freeze(notification->cc_prio_map); BT_LOGD("Created event notification object: " @@ -136,10 +145,10 @@ struct bt_notification *bt_notification_event_create( goto end; error: - BT_PUT(notification); + BT_ASSERT(!notification); + bt_event_destroy(event); end: - bt_put(cc_prio_map); return (void *) notification; } @@ -168,21 +177,19 @@ void bt_notification_event_recycle(struct bt_notification *notif) BT_ASSERT(event_notif); - if (!notif->graph) { + if (unlikely(!notif->graph)) { bt_notification_event_destroy(notif); return; } BT_LOGD("Recycling event notification: addr=%p", notif); bt_notification_reset(notif); - - if (event_notif->event) { - BT_LOGD_STR("Recycling event."); - bt_event_recycle(event_notif->event); - event_notif->event = NULL; - } - - BT_PUT(event_notif->cc_prio_map); + BT_ASSERT(event_notif->event); + BT_LOGD_STR("Recycling event."); + bt_event_recycle(event_notif->event); + event_notif->event = NULL; + bt_object_put_no_null_check(&event_notif->cc_prio_map->base); + event_notif->cc_prio_map = NULL; graph = notif->graph; notif->graph = NULL; bt_object_pool_recycle_object(&graph->event_notif_pool, notif); diff --git a/lib/graph/notification/packet.c b/lib/graph/notification/packet.c index 17b00d46..316eb613 100644 --- a/lib/graph/notification/packet.c +++ b/lib/graph/notification/packet.c @@ -37,6 +37,7 @@ #include #include #include +#include #include BT_HIDDEN @@ -87,10 +88,13 @@ struct bt_notification *bt_notification_packet_begin_create( &graph->packet_begin_notif_pool, graph); if (!notification) { /* bt_notification_create_from_pool() logs errors */ - goto error; + goto end; } - notification->packet = bt_get(packet); + BT_ASSERT(!notification->packet); + notification->packet = packet; + bt_object_get_no_null_check_no_parent_check( + ¬ification->packet->base); BT_LOGD("Created packet beginning notification object: " "packet-addr=%p, stream-addr=%p, stream-name=\"%s\", " "stream-class-addr=%p, stream-class-name=\"%s\", " @@ -101,9 +105,6 @@ struct bt_notification *bt_notification_packet_begin_create( bt_stream_class_get_id(stream_class), notification); goto end; -error: - BT_PUT(notification); - end: return (void *) notification; } @@ -127,14 +128,15 @@ void bt_notification_packet_begin_recycle(struct bt_notification *notif) BT_ASSERT(packet_begin_notif); - if (!notif->graph) { + if (unlikely(!notif->graph)) { bt_notification_packet_begin_destroy(notif); return; } BT_LOGD("Recycling packet beginning notification: addr=%p", notif); bt_notification_reset(notif); - BT_PUT(packet_begin_notif->packet); + bt_object_put_no_null_check(&packet_begin_notif->packet->base); + packet_begin_notif->packet = NULL; graph = notif->graph; notif->graph = NULL; bt_object_pool_recycle_object(&graph->packet_begin_notif_pool, notif); @@ -201,10 +203,13 @@ struct bt_notification *bt_notification_packet_end_create( &graph->packet_end_notif_pool, graph); if (!notification) { /* bt_notification_create_from_pool() logs errors */ - goto error; + goto end; } - notification->packet = bt_get(packet); + BT_ASSERT(!notification->packet); + notification->packet = packet; + bt_object_get_no_null_check_no_parent_check( + ¬ification->packet->base); BT_LOGD("Created packet end notification object: " "packet-addr=%p, stream-addr=%p, stream-name=\"%s\", " "stream-class-addr=%p, stream-class-name=\"%s\", " @@ -215,9 +220,6 @@ struct bt_notification *bt_notification_packet_end_create( bt_stream_class_get_id(stream_class), notification); goto end; -error: - BT_PUT(notification); - end: return (void *) notification; } -- 2.34.1