From e5f8db56f4cc9b5fba71bafead14ea6f260cdaee Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Fri, 15 Nov 2019 14:39:59 -0500 Subject: [PATCH] ctf: make msg-iter not use bt_packet_context_field The bt_packet_context_field API exists so that the CTF message iterator could store packet context fields in contexts where it doesn't create trace-ir objects. For example, in add_ds_file_to_ds_file_group, we use a message iterator just to sniff the properties of the first packet of a data stream file. However, the CTF message iterator doesn't really use it anymore, it just stores the relevant properties in fields of the ctf_msg_iter structure and transfers them to the caller in ctf_msg_iter_get_packet_properties. So the bt_packet_context_field API no longer has a reason to be. This patch makes msg-iter stop using it, so it can be removed. Currently, the packet_context_field is created when needed, which is just before reading the packet context, in read_packet_context_begin_state. A bt_packet is later created in create_msg_packet_beginning, and the packet_context_field is moved into the bt_packet. If we want to get rid of the packet_context_field, it means we need to create the bt_packet earlier (if not in dry_run mode), so it is available in read_packet_context_begin_state, so that the packet context fields are read directly into the bt_packet. And to create the bt_packet, the bt_stream needs to be available. The bt_stream is currently created just before sending the stream beginning message, in ctf_msg_iter_get_next_message. So this patch moves the creation of the bt_stream and bt_packet as early as possible, just after we have read the packet header, in after_packet_header_state. In read_packet_context_begin_state, we can then find the packet already created in msg_iter->packet. This change made it necessary to set the dry run flag on the CTF message iterator in add_ds_file_to_ds_file_group (which should have probably already been there anyway). This iterator is used to read the packet context, which, previously, didn't trigger the creation of the bt_stream and bt_packet. But now that we create those earlier, it is necessary to set the flag to avoid them being created. It was also needed to change the check in after_packet_context_state to know whether we are at the first packet, and therefore need to emit a stream beginning message, or if we are at a subsequent packet, and therefore need to skip that step. We would previously check whether the msg_it->stream field was set, but this is now always true given that we set the stream earlier. The only way that I found to fix that is to add the boolean flag emit_stream_beginning_message in the message iterator. Change-Id: Ib907480727d00d51aabd89986375d533ad385c96 Signed-off-by: Simon Marchi Reviewed-on: https://review.lttng.org/c/babeltrace/+/2393 Reviewed-by: Philippe Proulx --- src/plugins/ctf/common/msg-iter/msg-iter.c | 105 ++++++--------------- src/plugins/ctf/fs-src/fs.c | 2 + 2 files changed, 30 insertions(+), 77 deletions(-) diff --git a/src/plugins/ctf/common/msg-iter/msg-iter.c b/src/plugins/ctf/common/msg-iter/msg-iter.c index 70c839bc..2adf68f7 100644 --- a/src/plugins/ctf/common/msg-iter/msg-iter.c +++ b/src/plugins/ctf/common/msg-iter/msg-iter.c @@ -157,9 +157,6 @@ struct ctf_msg_iter { struct ctf_event_class *ec; } meta; - /* Current packet context field wrapper (NULL if not created yet) */ - bt_packet_context_field *packet_context_field; - /* Current packet (NULL if not created yet) */ bt_packet *packet; @@ -178,6 +175,12 @@ struct ctf_msg_iter { */ bool emit_delayed_packet_beginning_msg; + /* + * True if this is the first packet we are reading, and therefore if we + * should emit a stream beginning message. + */ + bool emit_stream_beginning_message; + /* Database of current dynamic scopes */ struct { bt_field *stream_packet_context; @@ -686,11 +689,6 @@ void release_all_dscopes(struct ctf_msg_iter *msg_it) { msg_it->dscopes.stream_packet_context = NULL; - if (msg_it->packet_context_field) { - bt_packet_context_field_release(msg_it->packet_context_field); - msg_it->packet_context_field = NULL; - } - release_event_dscopes(msg_it); } @@ -1001,8 +999,22 @@ enum ctf_msg_iter_status after_packet_header_state( goto end; } + if (!msg_it->dry_run) { + status = set_current_stream(msg_it); + if (status != CTF_MSG_ITER_STATUS_OK) { + goto end; + } + + status = set_current_packet(msg_it); + if (status != CTF_MSG_ITER_STATUS_OK) { + goto end; + } + } + msg_it->state = STATE_DSCOPE_STREAM_PACKET_CONTEXT_BEGIN; + status = CTF_MSG_ITER_STATUS_OK; + end: return status; } @@ -1027,31 +1039,11 @@ enum ctf_msg_iter_status read_packet_context_begin_state( goto end; } - BT_ASSERT(!msg_it->packet_context_field); - if (packet_context_fc->in_ir && !msg_it->dry_run) { - /* - * Create free packet context field from stream class. - * This field is going to be moved to the packet once we - * create it. We cannot create the packet now because a - * packet is created from a stream, and this API must be - * able to return the packet context properties without - * creating a stream - * (ctf_msg_iter_get_packet_properties()). - */ - msg_it->packet_context_field = - bt_packet_context_field_create( - msg_it->meta.sc->ir_sc); - if (!msg_it->packet_context_field) { - BT_COMP_LOGE_APPEND_CAUSE(self_comp, - "Cannot create packet context field wrapper from stream class."); - status = CTF_MSG_ITER_STATUS_ERROR; - goto end; - } - + BT_ASSERT(!msg_it->dscopes.stream_packet_context); + BT_ASSERT(msg_it->packet); msg_it->dscopes.stream_packet_context = - bt_packet_context_field_borrow_field( - msg_it->packet_context_field); + bt_packet_borrow_context_field(msg_it->packet); BT_ASSERT(msg_it->dscopes.stream_packet_context); } @@ -1143,15 +1135,10 @@ enum ctf_msg_iter_status after_packet_context_state(struct ctf_msg_iter *msg_it) goto end; } - if (msg_it->stream) { - /* - * Stream exists, which means we already emitted at - * least one packet beginning message, so the initial - * stream beginning message was also emitted. - */ - msg_it->state = STATE_CHECK_EMIT_MSG_DISCARDED_EVENTS; - } else { + if (msg_it->emit_stream_beginning_message) { msg_it->state = STATE_EMIT_MSG_STREAM_BEGINNING; + } else { + msg_it->state = STATE_CHECK_EMIT_MSG_DISCARDED_EVENTS; } end: @@ -1800,11 +1787,6 @@ void ctf_msg_iter_reset_for_next_stream_file(struct ctf_msg_iter *msg_it) release_all_dscopes(msg_it); msg_it->cur_dscope_field = NULL; - if (msg_it->packet_context_field) { - bt_packet_context_field_release(msg_it->packet_context_field); - msg_it->packet_context_field = NULL; - } - msg_it->buf.addr = NULL; msg_it->buf.sz = 0; msg_it->buf.at = 0; @@ -1834,6 +1816,7 @@ void ctf_msg_iter_reset(struct ctf_msg_iter *msg_it) msg_it->prev_packet_snapshots.packets = UINT64_C(-1); msg_it->prev_packet_snapshots.beginning_clock = UINT64_C(-1); msg_it->prev_packet_snapshots.end_clock = UINT64_C(-1); + msg_it->emit_stream_beginning_message = true; } static @@ -2512,33 +2495,11 @@ bt_message *create_msg_packet_beginning(struct ctf_msg_iter *msg_it, bool use_default_cs) { bt_self_component *self_comp = msg_it->self_comp; - int ret; bt_message *msg; const bt_stream_class *sc = msg_it->meta.sc->ir_sc; BT_ASSERT(msg_it->packet); BT_ASSERT(sc); - - if (msg_it->packet_context_field) { - ret = bt_packet_move_context_field( - msg_it->packet, msg_it->packet_context_field); - if (ret) { - msg = NULL; - goto end; - } - - msg_it->packet_context_field = NULL; - - /* - * At this point msg_it->dscopes.stream_packet_context - * has the same value as the packet context field within - * msg_it->packet. - */ - BT_ASSERT(bt_packet_borrow_context_field( - msg_it->packet) == - msg_it->dscopes.stream_packet_context); - } - BT_ASSERT(msg_it->self_msg_iter); if (msg_it->meta.sc->packets_have_ts_begin) { @@ -2954,11 +2915,6 @@ enum ctf_msg_iter_status ctf_msg_iter_get_next_message( goto end; case STATE_EMIT_MSG_PACKET_BEGINNING: - status = set_current_packet(msg_it); - if (status != CTF_MSG_ITER_STATUS_OK) { - goto end; - } - if (G_UNLIKELY(msg_it->meta.tc->quirks.barectf_event_before_packet)) { msg_it->emit_delayed_packet_beginning_msg = true; /* @@ -2988,14 +2944,9 @@ enum ctf_msg_iter_status ctf_msg_iter_get_next_message( goto end; case STATE_EMIT_MSG_STREAM_BEGINNING: - BT_ASSERT(!msg_it->stream); - status = set_current_stream(msg_it); - if (status != CTF_MSG_ITER_STATUS_OK) { - goto end; - } - /* create_msg_stream_beginning() logs errors */ *message = create_msg_stream_beginning(msg_it); + msg_it->emit_stream_beginning_message = false; if (!*message) { status = CTF_MSG_ITER_STATUS_ERROR; diff --git a/src/plugins/ctf/fs-src/fs.c b/src/plugins/ctf/fs-src/fs.c index 76283b4c..ceeb4bc2 100644 --- a/src/plugins/ctf/fs-src/fs.c +++ b/src/plugins/ctf/fs-src/fs.c @@ -764,6 +764,8 @@ int add_ds_file_to_ds_file_group(struct ctf_fs_trace *ctf_fs_trace, goto error; } + ctf_msg_iter_set_dry_run(msg_iter, true); + ret = ctf_msg_iter_get_packet_properties(msg_iter, &props); if (ret) { BT_COMP_OR_COMP_CLASS_LOGE_APPEND_CAUSE(self_comp, self_comp_class, -- 2.34.1