From 2fb7af12b8423f1bf00cbbbc7b8766dcc8d3ec15 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Tue, 9 Apr 2024 17:07:50 -0400 Subject: [PATCH] src.ctf.fs: make ctf_fs_ds_index::entries a vector of ctf_fs_ds_index_entry::UP Change-Id: If56c5ab6c35b6d93bf38b264f384496d2d06f4eb Signed-off-by: Simon Marchi Reviewed-on: https://review.lttng.org/c/babeltrace/+/8254 Reviewed-by: Philippe Proulx Reviewed-on: https://review.lttng.org/c/babeltrace/+/12292 --- src/plugins/ctf/fs-src/data-stream-file.cpp | 43 ++------- src/plugins/ctf/fs-src/data-stream-file.hpp | 5 +- src/plugins/ctf/fs-src/fs.cpp | 96 ++++++--------------- src/plugins/ctf/fs-src/query.cpp | 9 +- 4 files changed, 38 insertions(+), 115 deletions(-) diff --git a/src/plugins/ctf/fs-src/data-stream-file.cpp b/src/plugins/ctf/fs-src/data-stream-file.cpp index 63140e45..f62bbd29 100644 --- a/src/plugins/ctf/fs-src/data-stream-file.cpp +++ b/src/plugins/ctf/fs-src/data-stream-file.cpp @@ -359,7 +359,7 @@ static enum ctf_msg_iter_medium_status medop_group_switch_packet(void *void_data enum ctf_msg_iter_medium_status status; /* If we have gone through all index entries, we are done. */ - if (data->next_index_entry_index >= data->ds_file_group->index->entries->len) { + if (data->next_index_entry_index >= data->ds_file_group->index->entries.size()) { status = CTF_MSG_ITER_MEDIUM_STATUS_EOF; goto end; } @@ -368,8 +368,7 @@ static enum ctf_msg_iter_medium_status medop_group_switch_packet(void *void_data * Otherwise, look up the next index entry / packet and prepare it * for reading. */ - index_entry = (struct ctf_fs_ds_index_entry *) g_ptr_array_index( - data->ds_file_group->index->entries, data->next_index_entry_index); + index_entry = data->ds_file_group->index->entries[data->next_index_entry_index].get(); status = ctf_fs_ds_group_medops_set_file(data, index_entry); if (status != CTF_MSG_ITER_MEDIUM_STATUS_OK) { @@ -404,7 +403,7 @@ enum ctf_msg_iter_medium_status ctf_fs_ds_group_medops_data_create( BT_ASSERT(self_msg_iter); BT_ASSERT(ds_file_group); BT_ASSERT(ds_file_group->index); - BT_ASSERT(ds_file_group->index->entries->len > 0); + BT_ASSERT(!ds_file_group->index->entries.empty()); ctf_fs_ds_group_medops_data *data = new ctf_fs_ds_group_medops_data {parentLogger}; data->ds_file_group = ds_file_group; @@ -438,11 +437,6 @@ struct ctf_msg_iter_medium_ops ctf_fs_ds_group_medops = { .borrow_stream = medop_group_borrow_stream, }; -static void ctf_fs_ds_index_entry_destroy(ctf_fs_ds_index_entry *entry) -{ - delete entry; -} - static ctf_fs_ds_index_entry::UP ctf_fs_ds_index_entry_create(const bt2c::DataLen offset, const bt2c::DataLen packetSize) { @@ -581,7 +575,7 @@ static ctf_fs_ds_index::UP build_index_from_idx_file(struct ctf_fs_ds_file *ds_f goto error; } - index = ctf_fs_ds_index_create(ds_file->logger); + index = ctf_fs_ds_index_create(); if (!index) { goto error; } @@ -655,8 +649,7 @@ static ctf_fs_ds_index::UP build_index_from_idx_file(struct ctf_fs_ds_file *ds_f prev_index_entry = index_entry.get(); - /* Give ownership of `index_entry` to `index->entries`. */ - g_ptr_array_add(index->entries, index_entry.release()); + index->entries.emplace_back(std::move(index_entry)); } /* Validate that the index addresses the complete stream. */ @@ -739,7 +732,7 @@ static ctf_fs_ds_index::UP build_index_from_stream_file(struct ctf_fs_ds_file *d BT_CPPLOGI_SPEC(ds_file->logger, "Indexing stream file {}", ds_file->file->path->str); - index = ctf_fs_ds_index_create(ds_file->logger); + index = ctf_fs_ds_index_create(); if (!index) { goto error; } @@ -800,7 +793,7 @@ static ctf_fs_ds_index::UP build_index_from_stream_file(struct ctf_fs_ds_file *d goto error; } - g_ptr_array_add(index->entries, index_entry.release()); + index->entries.emplace_back(std::move(index_entry)); currentPacketOffset += currentPacketSize; BT_CPPLOGD_SPEC(ds_file->logger, @@ -873,23 +866,9 @@ end: return index; } -ctf_fs_ds_index::UP ctf_fs_ds_index_create(const bt2c::Logger& logger) +ctf_fs_ds_index::UP ctf_fs_ds_index_create() { - ctf_fs_ds_index::UP index {new ctf_fs_ds_index}; - - index->entries = g_ptr_array_new_with_free_func((GDestroyNotify) ctf_fs_ds_index_entry_destroy); - if (!index->entries) { - BT_CPPLOGE_SPEC(logger, "Failed to allocate index entries."); - goto error; - } - - goto end; - -error: - index.reset(); - -end: - return index; + return ctf_fs_ds_index::UP {new ctf_fs_ds_index}; } void ctf_fs_ds_file_destroy(struct ctf_fs_ds_file *ds_file) @@ -914,10 +893,6 @@ void ctf_fs_ds_index_destroy(struct ctf_fs_ds_index *index) return; } - if (index->entries) { - g_ptr_array_free(index->entries, TRUE); - } - delete index; } diff --git a/src/plugins/ctf/fs-src/data-stream-file.hpp b/src/plugins/ctf/fs-src/data-stream-file.hpp index df0c0c78..3059933c 100644 --- a/src/plugins/ctf/fs-src/data-stream-file.hpp +++ b/src/plugins/ctf/fs-src/data-stream-file.hpp @@ -117,8 +117,7 @@ struct ctf_fs_ds_index { using UP = std::unique_ptr; - /* Array of pointer to struct ctf_fs_ds_index_entry. */ - GPtrArray *entries = nullptr; + std::vector entries; }; struct ctf_fs_ds_file_group_deleter @@ -163,7 +162,7 @@ ctf_fs_ds_index::UP ctf_fs_ds_file_build_index(struct ctf_fs_ds_file *ds_file, struct ctf_fs_ds_file_info *ds_file_info, struct ctf_msg_iter *msg_iter); -ctf_fs_ds_index::UP ctf_fs_ds_index_create(const bt2c::Logger& logger); +ctf_fs_ds_index::UP ctf_fs_ds_index_create(); void ctf_fs_ds_index_destroy(struct ctf_fs_ds_index *index); diff --git a/src/plugins/ctf/fs-src/fs.cpp b/src/plugins/ctf/fs-src/fs.cpp index b7d2d855..40e50085 100644 --- a/src/plugins/ctf/fs-src/fs.cpp +++ b/src/plugins/ctf/fs-src/fs.cpp @@ -421,24 +421,6 @@ end: return ret; } -/* Replace by g_ptr_array_insert when we depend on glib >= 2.40. */ -static void array_insert(GPtrArray *array, gpointer element, size_t pos) -{ - size_t original_array_len = array->len; - - /* Allocate an unused element at the end of the array. */ - g_ptr_array_add(array, NULL); - - /* If we are not inserting at the end, move the elements by one. */ - if (pos < original_array_len) { - memmove(&(array->pdata[pos + 1]), &(array->pdata[pos]), - (original_array_len - pos) * sizeof(gpointer)); - } - - /* Insert the value. */ - array->pdata[pos] = element; -} - /* * Insert ds_file_info in ds_file_group's list of ds_file_infos at the right * place to keep it sorted. @@ -493,16 +475,12 @@ static bool ds_index_entries_equal(const struct ctf_fs_ds_index_entry *left, */ static void ds_index_insert_ds_index_entry_sorted(struct ctf_fs_ds_index *index, - struct ctf_fs_ds_index_entry *entry) + ctf_fs_ds_index_entry::UP entry) { - guint i; - struct ctf_fs_ds_index_entry *other_entry = NULL; - /* Find the spot where to insert this index entry. */ - for (i = 0; i < index->entries->len; i++) { - other_entry = (struct ctf_fs_ds_index_entry *) g_ptr_array_index(index->entries, i); - - if (entry->timestamp_begin_ns <= other_entry->timestamp_begin_ns) { + auto otherEntry = index->entries.begin(); + for (; otherEntry != index->entries.end(); ++otherEntry) { + if (entry->timestamp_begin_ns <= (*otherEntry)->timestamp_begin_ns) { break; } } @@ -514,27 +492,16 @@ static void ds_index_insert_ds_index_entry_sorted(struct ctf_fs_ds_index *index, * snapshots of the same trace. We then want the index to contain * a reference to only one copy of that packet. */ - if (i == index->entries->len || !ds_index_entries_equal(entry, other_entry)) { - array_insert(index->entries, entry, i); - } else { - delete entry; + if (otherEntry == index->entries.end() || + !ds_index_entries_equal(entry.get(), otherEntry->get())) { + index->entries.insert(otherEntry, std::move(entry)); } } static void merge_ctf_fs_ds_indexes(struct ctf_fs_ds_index *dest, ctf_fs_ds_index::UP src) { - guint i; - - for (i = 0; i < src->entries->len; i++) { - struct ctf_fs_ds_index_entry *entry = - (struct ctf_fs_ds_index_entry *) g_ptr_array_index(src->entries, i); - - /* - * Ownership of the ctf_fs_ds_index_entry is transferred to - * ds_index_insert_ds_index_entry_sorted. - */ - g_ptr_array_index(src->entries, i) = NULL; - ds_index_insert_ds_index_entry_sorted(dest, entry); + for (auto& entry : src->entries) { + ds_index_insert_ds_index_entry_sorted(dest, std::move(entry)); } } @@ -1059,7 +1026,7 @@ static int merge_matching_ctf_fs_ds_file_groups(struct ctf_fs_trace *dest_trace, src_group->sc->id); BT_ASSERT(sc); - auto index = ctf_fs_ds_index_create(dest_trace->logger); + auto index = ctf_fs_ds_index_create(); if (!index) { ret = -1; goto end; @@ -1284,26 +1251,21 @@ static int fix_index_lttng_event_after_packet_bug(struct ctf_fs_trace *trace) int ret = 0; for (const auto& ds_file_group : trace->ds_file_groups) { - guint entry_i; struct ctf_clock_class *default_cc; - struct ctf_fs_ds_index_entry *last_entry; BT_ASSERT(ds_file_group); const auto index = ds_file_group->index.get(); BT_ASSERT(index); - BT_ASSERT(index->entries); - BT_ASSERT(index->entries->len > 0); + BT_ASSERT(!index->entries.empty()); /* * Iterate over all entries but the last one. The last one is * fixed differently after. */ - for (entry_i = 0; entry_i < index->entries->len - 1; entry_i++) { - struct ctf_fs_ds_index_entry *curr_entry, *next_entry; - - curr_entry = (ctf_fs_ds_index_entry *) g_ptr_array_index(index->entries, entry_i); - next_entry = (ctf_fs_ds_index_entry *) g_ptr_array_index(index->entries, entry_i + 1); + for (size_t entry_i = 0; entry_i < index->entries.size() - 1; ++entry_i) { + ctf_fs_ds_index_entry *curr_entry = index->entries[entry_i].get(); + ctf_fs_ds_index_entry *next_entry = index->entries[entry_i + 1].get(); /* * 1. Set the current index entry `end` timestamp to @@ -1317,8 +1279,7 @@ static int fix_index_lttng_event_after_packet_bug(struct ctf_fs_trace *trace) * 2. Fix the last entry by decoding the last event of the last * packet. */ - last_entry = - (ctf_fs_ds_index_entry *) g_ptr_array_index(index->entries, index->entries->len - 1); + const auto last_entry = index->entries.back().get(); BT_ASSERT(last_entry); BT_ASSERT(ds_file_group->sc->default_clock_class); @@ -1362,13 +1323,11 @@ static int fix_index_barectf_event_before_packet_bug(struct ctf_fs_trace *trace) int ret = 0; for (const auto& ds_file_group : trace->ds_file_groups) { - guint entry_i; struct ctf_clock_class *default_cc; const auto index = ds_file_group->index.get(); BT_ASSERT(index); - BT_ASSERT(index->entries); - BT_ASSERT(index->entries->len > 0); + BT_ASSERT(!index->entries.empty()); BT_ASSERT(ds_file_group->sc->default_clock_class); default_cc = ds_file_group->sc->default_clock_class; @@ -1377,11 +1336,9 @@ static int fix_index_barectf_event_before_packet_bug(struct ctf_fs_trace *trace) * 1. Iterate over the index, starting from the second entry * (index = 1). */ - for (entry_i = 1; entry_i < index->entries->len; entry_i++) { - ctf_fs_ds_index_entry *prev_entry = - (ctf_fs_ds_index_entry *) g_ptr_array_index(index->entries, entry_i - 1); - ctf_fs_ds_index_entry *curr_entry = - (ctf_fs_ds_index_entry *) g_ptr_array_index(index->entries, entry_i); + for (size_t entry_i = 1; entry_i < index->entries.size(); ++entry_i) { + ctf_fs_ds_index_entry *prev_entry = index->entries[entry_i - 1].get(); + ctf_fs_ds_index_entry *curr_entry = index->entries[entry_i].get(); /* * 2. Set the current entry `begin` timestamp to the * timestamp of the first event of the current packet. @@ -1429,7 +1386,6 @@ static int fix_index_lttng_crash_quirk(struct ctf_fs_trace *trace) int ret = 0; for (const auto& ds_file_group : trace->ds_file_groups) { - guint entry_idx; struct ctf_clock_class *default_cc; BT_ASSERT(ds_file_group); @@ -1439,11 +1395,9 @@ static int fix_index_lttng_crash_quirk(struct ctf_fs_trace *trace) default_cc = ds_file_group->sc->default_clock_class; BT_ASSERT(index); - BT_ASSERT(index->entries); - BT_ASSERT(index->entries->len > 0); + BT_ASSERT(!index->entries.empty()); - ctf_fs_ds_index_entry *last_entry = - (ctf_fs_ds_index_entry *) g_ptr_array_index(index->entries, index->entries->len - 1); + const auto last_entry = index->entries.back().get(); BT_ASSERT(last_entry); /* 1. Fix the last entry first. */ @@ -1463,11 +1417,9 @@ static int fix_index_lttng_crash_quirk(struct ctf_fs_trace *trace) } /* Iterate over all entries but the last one. */ - for (entry_idx = 0; entry_idx < index->entries->len - 1; entry_idx++) { - ctf_fs_ds_index_entry *curr_entry = - (ctf_fs_ds_index_entry *) g_ptr_array_index(index->entries, entry_idx); - ctf_fs_ds_index_entry *next_entry = - (ctf_fs_ds_index_entry *) g_ptr_array_index(index->entries, entry_idx + 1); + for (size_t entry_idx = 0; entry_idx < index->entries.size() - 1; ++entry_idx) { + ctf_fs_ds_index_entry *curr_entry = index->entries[entry_idx].get(); + ctf_fs_ds_index_entry *next_entry = index->entries[entry_idx + 1].get(); if (curr_entry->timestamp_end == 0 && curr_entry->timestamp_begin != 0) { /* diff --git a/src/plugins/ctf/fs-src/query.cpp b/src/plugins/ctf/fs-src/query.cpp index 4f27127b..2a6057db 100644 --- a/src/plugins/ctf/fs-src/query.cpp +++ b/src/plugins/ctf/fs-src/query.cpp @@ -124,16 +124,13 @@ static void populate_stream_info(struct ctf_fs_ds_file_group *group, const bt2:: * of the last index entry. */ BT_ASSERT(group->index); - BT_ASSERT(group->index->entries); - BT_ASSERT(group->index->entries->len > 0); + BT_ASSERT(!group->index->entries.empty()); /* First entry. */ - ctf_fs_ds_index_entry *first_ds_index_entry = - (struct ctf_fs_ds_index_entry *) g_ptr_array_index(group->index->entries, 0); + const auto first_ds_index_entry = group->index->entries.front().get(); /* Last entry. */ - ctf_fs_ds_index_entry *last_ds_index_entry = (struct ctf_fs_ds_index_entry *) g_ptr_array_index( - group->index->entries, group->index->entries->len - 1); + const auto last_ds_index_entry = group->index->entries.back().get(); stream_range->begin_ns = first_ds_index_entry->timestamp_begin_ns; stream_range->end_ns = last_ds_index_entry->timestamp_end_ns; -- 2.34.1