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 <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2393
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>

diff --git a/src/plugins/ctf/common/msg-iter/msg-iter.c b/src/plugins/ctf/common/msg-iter/msg-iter.c
index 70c839bcae5a242c292dc5085d8af1901a264e8e..2adf68f71dcd4849b5d82ec57b8ab55ec4cb79ef 100644 (file)
--- 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 76283b4ca7b3fad94382b07d8457215aeb1f219c..ceeb4bc268436141154e59b3bfe8b9502bf51a19 100644 (file)
--- 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,