From 6d54260a276ca4284cafe05d45e84f6d6cc7bce4 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Mon, 4 Nov 2019 15:17:10 -0500 Subject: [PATCH] ctf: remove ctf_fs_ds_file::msg_iter field I find it strange that ctf_fs_ds_file objects knows about the ctf_msg_iter that iterates on it. The ctf_fs_ds_file object just represents some data that is accessible (a data stream file and the portion currently mmap-ed), but I don't think it should know anything about the iterating operation. This patch removes the msg_iter field of ctf_fs_ds_file and adjusts the code accordingly. I think this change makes the code a bit clearer when we create ctf_msg_iter objects. Currently, we create the msg_iter first with a NULL medium ops data, then create the ctf_fs_ds_file. In ctf_fs_ds_file_create, it sets the ctf_msg_iter's medium ops data to the ctf_fs_ds_file. With this patch, we create the ctf_fs_ds_file first. Then, when we create the ctf_msg_iter, we can pass the ctf_fs_ds_file as the medium ops data directly when we create the ctf_msg_iter. Change-Id: I247f039d225888c059f08ccdccc86abf0816a6c4 Signed-off-by: Simon Marchi --- src/plugins/ctf/fs-src/data-stream-file.c | 31 +++++------ src/plugins/ctf/fs-src/data-stream-file.h | 9 +-- src/plugins/ctf/fs-src/fs.c | 67 +++++++++++++---------- 3 files changed, 55 insertions(+), 52 deletions(-) diff --git a/src/plugins/ctf/fs-src/data-stream-file.c b/src/plugins/ctf/fs-src/data-stream-file.c index 99ff70b1..6867529e 100644 --- a/src/plugins/ctf/fs-src/data-stream-file.c +++ b/src/plugins/ctf/fs-src/data-stream-file.c @@ -310,7 +310,8 @@ int convert_cycles_to_ns(struct ctf_clock_class *clock_class, static struct ctf_fs_ds_index *build_index_from_idx_file( struct ctf_fs_ds_file *ds_file, - struct ctf_fs_ds_file_info *file_info) + struct ctf_fs_ds_file_info *file_info, + struct ctf_msg_iter *msg_iter) { int ret; gchar *directory = NULL; @@ -335,7 +336,7 @@ struct ctf_fs_ds_index *build_index_from_idx_file( BT_COMP_LOGI("Building index from .idx file of stream file %s", ds_file->file->path->str); - ret = ctf_msg_iter_get_packet_properties(ds_file->msg_iter, &props); + ret = ctf_msg_iter_get_packet_properties(msg_iter, &props); if (ret) { BT_COMP_LOGI_STR("Cannot read first packet's header and context fields."); goto error; @@ -583,7 +584,8 @@ end: static struct ctf_fs_ds_index *build_index_from_stream_file( struct ctf_fs_ds_file *ds_file, - struct ctf_fs_ds_file_info *file_info) + struct ctf_fs_ds_file_info *file_info, + struct ctf_msg_iter *msg_iter) { int ret; struct ctf_fs_ds_index *index = NULL; @@ -615,14 +617,14 @@ struct ctf_fs_ds_index *build_index_from_stream_file( break; } - iter_status = ctf_msg_iter_seek(ds_file->msg_iter, + iter_status = ctf_msg_iter_seek(msg_iter, current_packet_offset_bytes); if (iter_status != CTF_MSG_ITER_STATUS_OK) { goto error; } iter_status = ctf_msg_iter_get_packet_properties( - ds_file->msg_iter, &props); + msg_iter, &props); if (iter_status != CTF_MSG_ITER_STATUS_OK) { goto error; } @@ -686,7 +688,6 @@ BT_HIDDEN struct ctf_fs_ds_file *ctf_fs_ds_file_create( struct ctf_fs_trace *ctf_fs_trace, bt_self_message_iterator *self_msg_iter, - struct ctf_msg_iter *msg_iter, bt_stream *stream, const char *path, bt_logging_level log_level) { @@ -715,12 +716,6 @@ struct ctf_fs_ds_file *ctf_fs_ds_file_create( goto error; } - ds_file->msg_iter = msg_iter; - ctf_msg_iter_set_medops_data(ds_file->msg_iter, ds_file); - if (!ds_file->msg_iter) { - goto error; - } - ds_file->mmap_max_len = offset_align * 2048; goto end; @@ -737,20 +732,21 @@ end: BT_HIDDEN struct ctf_fs_ds_index *ctf_fs_ds_file_build_index( struct ctf_fs_ds_file *ds_file, - struct ctf_fs_ds_file_info *file_info) + struct ctf_fs_ds_file_info *file_info, + struct ctf_msg_iter *msg_iter) { struct ctf_fs_ds_index *index; bt_self_component *self_comp = ds_file->self_comp; bt_logging_level log_level = ds_file->log_level; - index = build_index_from_idx_file(ds_file, file_info); + index = build_index_from_idx_file(ds_file, file_info, msg_iter); if (index) { goto end; } BT_COMP_LOGI("Failed to build index from .index file; " "falling back to stream indexing."); - index = build_index_from_stream_file(ds_file, file_info); + index = build_index_from_stream_file(ds_file, file_info, msg_iter); end: return index; } @@ -802,14 +798,13 @@ void ctf_fs_ds_file_destroy(struct ctf_fs_ds_file *ds_file) BT_HIDDEN bt_component_class_message_iterator_next_method_status ctf_fs_ds_file_next( - struct ctf_fs_ds_file *ds_file, + struct ctf_msg_iter *msg_iter, bt_message **msg) { enum ctf_msg_iter_status msg_iter_status; bt_component_class_message_iterator_next_method_status status; - msg_iter_status = ctf_msg_iter_get_next_message( - ds_file->msg_iter, msg); + msg_iter_status = ctf_msg_iter_get_next_message(msg_iter, msg); switch (msg_iter_status) { case CTF_MSG_ITER_STATUS_EOF: diff --git a/src/plugins/ctf/fs-src/data-stream-file.h b/src/plugins/ctf/fs-src/data-stream-file.h index 505e46af..ff9643b5 100644 --- a/src/plugins/ctf/fs-src/data-stream-file.h +++ b/src/plugins/ctf/fs-src/data-stream-file.h @@ -65,9 +65,6 @@ struct ctf_fs_ds_file { /* Owned by this */ bt_stream *stream; - /* Weak */ - struct ctf_msg_iter *msg_iter; - void *mmap_addr; /* @@ -95,7 +92,6 @@ BT_HIDDEN struct ctf_fs_ds_file *ctf_fs_ds_file_create( struct ctf_fs_trace *ctf_fs_trace, bt_self_message_iterator *self_msg_iter, - struct ctf_msg_iter *msg_iter, bt_stream *stream, const char *path, bt_logging_level log_level); @@ -104,13 +100,14 @@ void ctf_fs_ds_file_destroy(struct ctf_fs_ds_file *stream); BT_HIDDEN bt_component_class_message_iterator_next_method_status ctf_fs_ds_file_next( - struct ctf_fs_ds_file *ds_file, + struct ctf_msg_iter *msg_iter, bt_message **msg); BT_HIDDEN struct ctf_fs_ds_index *ctf_fs_ds_file_build_index( struct ctf_fs_ds_file *ds_file, - struct ctf_fs_ds_file_info *ds_file_info); + struct ctf_fs_ds_file_info *ds_file_info, + struct ctf_msg_iter *msg_iter); BT_HIDDEN struct ctf_fs_ds_index *ctf_fs_ds_index_create(bt_logging_level log_level, diff --git a/src/plugins/ctf/fs-src/fs.c b/src/plugins/ctf/fs-src/fs.c index 64fff672..0530f015 100644 --- a/src/plugins/ctf/fs-src/fs.c +++ b/src/plugins/ctf/fs-src/fs.c @@ -66,11 +66,13 @@ int msg_iter_data_set_current_ds_file(struct ctf_fs_msg_iter_data *msg_iter_data msg_iter_data->ds_file_group->ds_file_infos, msg_iter_data->ds_file_info_index); + /* Destroy the previous ds file. */ ctf_fs_ds_file_destroy(msg_iter_data->ds_file); + + /* Create the new ds file. */ msg_iter_data->ds_file = ctf_fs_ds_file_create( msg_iter_data->ds_file_group->ctf_fs_trace, msg_iter_data->self_msg_iter, - msg_iter_data->msg_iter, msg_iter_data->ds_file_group->stream, ds_file_info->path->str, msg_iter_data->log_level); @@ -78,6 +80,10 @@ int msg_iter_data_set_current_ds_file(struct ctf_fs_msg_iter_data *msg_iter_data ret = -1; } + /* Tell the ctf message iterator to iterate on the new ds file. */ + ctf_msg_iter_set_medops_data(msg_iter_data->msg_iter, + msg_iter_data->ds_file); + return ret; } @@ -103,10 +109,10 @@ void set_msg_iter_emits_stream_beginning_end_messages( struct ctf_fs_msg_iter_data *msg_iter_data) { ctf_msg_iter_set_emit_stream_beginning_message( - msg_iter_data->ds_file->msg_iter, + msg_iter_data->msg_iter, msg_iter_data->ds_file_info_index == 0); ctf_msg_iter_set_emit_stream_end_message( - msg_iter_data->ds_file->msg_iter, + msg_iter_data->msg_iter, msg_iter_data->ds_file_info_index == msg_iter_data->ds_file_group->ds_file_infos->len - 1); } @@ -123,7 +129,7 @@ bt_component_class_message_iterator_next_method_status ctf_fs_iterator_next_one( while (true) { bt_message *msg; - status = ctf_fs_ds_file_next(msg_iter_data->ds_file, &msg); + status = ctf_fs_ds_file_next(msg_iter_data->msg_iter, &msg); switch (status) { case BT_COMPONENT_CLASS_MESSAGE_ITERATOR_NEXT_METHOD_STATUS_OK: *out_msg = msg; @@ -803,21 +809,26 @@ int add_ds_file_to_ds_file_group(struct ctf_fs_trace *ctf_fs_trace, bt_self_component *self_comp = ctf_fs_trace->self_comp; bt_self_component_class *self_comp_class = ctf_fs_trace->self_comp_class; + /* + * Create a temporary ds_file to read some properties about the data + * stream file. + */ + ds_file = ctf_fs_ds_file_create(ctf_fs_trace, NULL, NULL, path, + log_level); + if (!ds_file) { + goto error; + } + + /* Create a temporary iterator to read the ds_file. */ msg_iter = ctf_msg_iter_create(ctf_fs_trace->metadata->tc, bt_common_get_page_size(log_level) * 8, - ctf_fs_ds_file_medops, NULL, log_level, self_comp, NULL); + ctf_fs_ds_file_medops, ds_file, log_level, self_comp, NULL); if (!msg_iter) { BT_COMP_LOGE_STR("Cannot create a CTF message iterator."); goto error; } - ds_file = ctf_fs_ds_file_create(ctf_fs_trace, NULL, msg_iter, - NULL, path, log_level); - if (!ds_file) { - goto error; - } - - ret = ctf_msg_iter_get_packet_properties(ds_file->msg_iter, &props); + 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, "Cannot get stream file's first packet's header and context fields (`%s`).", @@ -850,7 +861,7 @@ int add_ds_file_to_ds_file_group(struct ctf_fs_trace *ctf_fs_trace, goto error; } - index = ctf_fs_ds_file_build_index(ds_file, ds_file_info); + index = ctf_fs_ds_file_build_index(ds_file, ds_file_info, msg_iter); if (!index) { BT_COMP_OR_COMP_CLASS_LOGE_APPEND_CAUSE( self_comp, self_comp_class, @@ -1504,29 +1515,29 @@ int decode_clock_snapshot_after_event(struct ctf_fs_trace *ctf_fs_trace, int ret = 0; BT_ASSERT(ctf_fs_trace); + BT_ASSERT(index_entry); + BT_ASSERT(index_entry->path); + + ds_file = ctf_fs_ds_file_create(ctf_fs_trace, NULL, + NULL, index_entry->path, log_level); + if (!ds_file) { + BT_COMP_LOGE_APPEND_CAUSE(self_comp, "Failed to create a ctf_fs_ds_file"); + ret = -1; + goto end; + } + BT_ASSERT(ctf_fs_trace->metadata); BT_ASSERT(ctf_fs_trace->metadata->tc); msg_iter = ctf_msg_iter_create(ctf_fs_trace->metadata->tc, bt_common_get_page_size(log_level) * 8, ctf_fs_ds_file_medops, - NULL, log_level, self_comp, NULL); + ds_file, log_level, self_comp, NULL); if (!msg_iter) { /* ctf_msg_iter_create() logs errors. */ ret = -1; goto end; } - BT_ASSERT(index_entry); - BT_ASSERT(index_entry->path); - - ds_file = ctf_fs_ds_file_create(ctf_fs_trace, NULL, msg_iter, - NULL, index_entry->path, log_level); - if (!ds_file) { - BT_COMP_LOGE_APPEND_CAUSE(self_comp, "Failed to create a ctf_fs_ds_file"); - ret = -1; - goto end; - } - /* * Turn on dry run mode to prevent the creation and usage of Babeltrace * library objects (bt_field, bt_message_*, etc.). @@ -1534,7 +1545,7 @@ int decode_clock_snapshot_after_event(struct ctf_fs_trace *ctf_fs_trace, ctf_msg_iter_set_dry_run(msg_iter, true); /* Seek to the beginning of the target packet. */ - iter_status = ctf_msg_iter_seek(ds_file->msg_iter, index_entry->offset); + iter_status = ctf_msg_iter_seek(msg_iter, index_entry->offset); if (iter_status) { /* ctf_msg_iter_seek() logs errors. */ ret = -1; @@ -1549,12 +1560,12 @@ int decode_clock_snapshot_after_event(struct ctf_fs_trace *ctf_fs_trace, * snapshot. */ iter_status = ctf_msg_iter_curr_packet_first_event_clock_snapshot( - ds_file->msg_iter, cs); + msg_iter, cs); break; case LAST_EVENT: /* Decode the packet to extract the last event's clock snapshot. */ iter_status = ctf_msg_iter_curr_packet_last_event_clock_snapshot( - ds_file->msg_iter, cs); + msg_iter, cs); break; default: bt_common_abort(); -- 2.34.1