From 0011731e4365bc8c2ec0b45dcc90baabe63be97b Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Wed, 6 Dec 2023 16:15:18 +0000 Subject: [PATCH] src.ctf.fs: store index entry objects instead of pointers Store `ctf_fs_ds_index_entry` objects in a flag vector, instead of a vector of pointers. Change-Id: I4fea078cddf97292cc1f040ca998acef465d6808 Reviewed-on: https://review.lttng.org/c/babeltrace/+/8348 Reviewed-by: Philippe Proulx Signed-off-by: Simon Marchi Reviewed-on: https://review.lttng.org/c/babeltrace/+/12343 Tested-by: jenkins --- src/plugins/ctf/fs-src/data-stream-file.cpp | 70 ++++++--------- src/plugins/ctf/fs-src/data-stream-file.hpp | 4 +- src/plugins/ctf/fs-src/fs.cpp | 94 ++++++++++----------- src/plugins/ctf/fs-src/query.cpp | 8 +- 4 files changed, 76 insertions(+), 100 deletions(-) diff --git a/src/plugins/ctf/fs-src/data-stream-file.cpp b/src/plugins/ctf/fs-src/data-stream-file.cpp index 10c21ec7..41950683 100644 --- a/src/plugins/ctf/fs-src/data-stream-file.cpp +++ b/src/plugins/ctf/fs-src/data-stream-file.cpp @@ -310,7 +310,6 @@ ctf_fs_ds_group_medops_set_file(struct ctf_fs_ds_group_medops_data *data, static enum ctf_msg_iter_medium_status medop_group_switch_packet(void *void_data) { struct ctf_fs_ds_group_medops_data *data = (struct ctf_fs_ds_group_medops_data *) void_data; - struct ctf_fs_ds_index_entry *index_entry; /* If we have gone through all index entries, we are done. */ if (data->next_index_entry_index >= data->ds_file_group->index->entries.size()) { @@ -321,9 +320,8 @@ 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 = data->ds_file_group->index->entries[data->next_index_entry_index].get(); - - ctf_msg_iter_medium_status status = ctf_fs_ds_group_medops_set_file(data, index_entry); + ctf_msg_iter_medium_status status = ctf_fs_ds_group_medops_set_file( + data, &data->ds_file_group->index->entries[data->next_index_entry_index]); if (status != CTF_MSG_ITER_MEDIUM_STATUS_OK) { return status; } @@ -399,7 +397,6 @@ static ctf_fs_ds_index::UP build_index_from_idx_file(struct ctf_fs_ds_file *ds_f const char *mmap_begin = NULL, *file_pos = NULL; const struct ctf_packet_index_file_hdr *header = NULL; ctf_fs_ds_index::UP index; - ctf_fs_ds_index_entry::UP index_entry; ctf_fs_ds_index_entry *prev_index_entry = NULL; auto totalPacketsSize = bt2c::DataLen::fromBytes(0); size_t file_index_entry_size; @@ -523,38 +520,33 @@ static ctf_fs_ds_index::UP build_index_from_idx_file(struct ctf_fs_ds_file *ds_f return nullptr; } - index_entry = bt2s::make_unique(offset, packetSize); - if (!index_entry) { - BT_CPPLOGE_APPEND_CAUSE_SPEC(ds_file->logger, - "Failed to create a ctf_fs_ds_index_entry."); - return nullptr; - } + ctf_fs_ds_index_entry index_entry {offset, packetSize}; /* Set path to stream file. */ - index_entry->path = file_info->path.c_str(); + index_entry.path = file_info->path.c_str(); - index_entry->timestamp_begin = be64toh(file_index->timestamp_begin); - index_entry->timestamp_end = be64toh(file_index->timestamp_end); - if (index_entry->timestamp_end < index_entry->timestamp_begin) { + index_entry.timestamp_begin = be64toh(file_index->timestamp_begin); + index_entry.timestamp_end = be64toh(file_index->timestamp_end); + if (index_entry.timestamp_end < index_entry.timestamp_begin) { BT_CPPLOGW_SPEC( ds_file->logger, "Invalid packet time bounds encountered in LTTng trace index file (begin > end): " "timestamp_begin={}, timestamp_end={}", - index_entry->timestamp_begin, index_entry->timestamp_end); + index_entry.timestamp_begin, index_entry.timestamp_end); return nullptr; } /* Convert the packet's bound to nanoseconds since Epoch. */ - ret = convert_cycles_to_ns(sc->default_clock_class, index_entry->timestamp_begin, - &index_entry->timestamp_begin_ns); + ret = convert_cycles_to_ns(sc->default_clock_class, index_entry.timestamp_begin, + &index_entry.timestamp_begin_ns); if (ret) { BT_CPPLOGI_STR_SPEC( ds_file->logger, "Failed to convert raw timestamp to nanoseconds since Epoch during index parsing"); return nullptr; } - ret = convert_cycles_to_ns(sc->default_clock_class, index_entry->timestamp_end, - &index_entry->timestamp_end_ns); + ret = convert_cycles_to_ns(sc->default_clock_class, index_entry.timestamp_end, + &index_entry.timestamp_end_ns); if (ret) { BT_CPPLOGI_STR_SPEC( ds_file->logger, @@ -563,15 +555,15 @@ static ctf_fs_ds_index::UP build_index_from_idx_file(struct ctf_fs_ds_file *ds_f } if (version_minor >= 1) { - index_entry->packet_seq_num = be64toh(file_index->packet_seq_num); + index_entry.packet_seq_num = be64toh(file_index->packet_seq_num); } totalPacketsSize += packetSize; file_pos += file_index_entry_size; - prev_index_entry = index_entry.get(); + index->entries.emplace_back(index_entry); - index->entries.emplace_back(std::move(index_entry)); + prev_index_entry = &index->entries.back(); } /* Validate that the index addresses the complete stream. */ @@ -586,7 +578,7 @@ static ctf_fs_ds_index::UP build_index_from_idx_file(struct ctf_fs_ds_file *ds_f return index; } -static int init_index_entry(struct ctf_fs_ds_index_entry *entry, struct ctf_fs_ds_file *ds_file, +static int init_index_entry(ctf_fs_ds_index_entry& entry, struct ctf_fs_ds_file *ds_file, struct ctf_msg_iter_packet_properties *props) { struct ctf_stream_class *sc; @@ -595,35 +587,35 @@ static int init_index_entry(struct ctf_fs_ds_index_entry *entry, struct ctf_fs_d BT_ASSERT(sc); if (props->snapshots.beginning_clock != UINT64_C(-1)) { - entry->timestamp_begin = props->snapshots.beginning_clock; + entry.timestamp_begin = props->snapshots.beginning_clock; /* Convert the packet's bound to nanoseconds since Epoch. */ int ret = convert_cycles_to_ns(sc->default_clock_class, props->snapshots.beginning_clock, - &entry->timestamp_begin_ns); + &entry.timestamp_begin_ns); if (ret) { BT_CPPLOGI_STR_SPEC(ds_file->logger, "Failed to convert raw timestamp to nanoseconds since Epoch."); return ret; } } else { - entry->timestamp_begin = UINT64_C(-1); - entry->timestamp_begin_ns = UINT64_C(-1); + entry.timestamp_begin = UINT64_C(-1); + entry.timestamp_begin_ns = UINT64_C(-1); } if (props->snapshots.end_clock != UINT64_C(-1)) { - entry->timestamp_end = props->snapshots.end_clock; + entry.timestamp_end = props->snapshots.end_clock; /* Convert the packet's bound to nanoseconds since Epoch. */ int ret = convert_cycles_to_ns(sc->default_clock_class, props->snapshots.end_clock, - &entry->timestamp_end_ns); + &entry.timestamp_end_ns); if (ret) { BT_CPPLOGI_STR_SPEC(ds_file->logger, "Failed to convert raw timestamp to nanoseconds since Epoch."); return ret; } } else { - entry->timestamp_end = UINT64_C(-1); - entry->timestamp_end_ns = UINT64_C(-1); + entry.timestamp_end = UINT64_C(-1); + entry.timestamp_end_ns = UINT64_C(-1); } return 0; @@ -682,23 +674,17 @@ static ctf_fs_ds_index::UP build_index_from_stream_file(struct ctf_fs_ds_file *d return nullptr; } - auto index_entry = - bt2s::make_unique(currentPacketOffset, currentPacketSize); - if (!index_entry) { - BT_CPPLOGE_APPEND_CAUSE_SPEC(ds_file->logger, - "Failed to create a ctf_fs_ds_index_entry."); - return nullptr; - } + ctf_fs_ds_index_entry index_entry {currentPacketOffset, currentPacketSize}; /* Set path to stream file. */ - index_entry->path = file_info->path.c_str(); + index_entry.path = file_info->path.c_str(); - ret = init_index_entry(index_entry.get(), ds_file, &props); + ret = init_index_entry(index_entry, ds_file, &props); if (ret) { return nullptr; } - index->entries.emplace_back(std::move(index_entry)); + index->entries.emplace_back(index_entry); currentPacketOffset += currentPacketSize; BT_CPPLOGD_SPEC(ds_file->logger, diff --git a/src/plugins/ctf/fs-src/data-stream-file.hpp b/src/plugins/ctf/fs-src/data-stream-file.hpp index 10c97a5e..d99760bb 100644 --- a/src/plugins/ctf/fs-src/data-stream-file.hpp +++ b/src/plugins/ctf/fs-src/data-stream-file.hpp @@ -78,8 +78,6 @@ struct ctf_fs_ds_file struct ctf_fs_ds_index_entry { - using UP = std::unique_ptr; - explicit ctf_fs_ds_index_entry(const bt2c::DataLen offsetParam, const bt2c::DataLen packetSizeParam) noexcept : offset(offsetParam), @@ -118,7 +116,7 @@ struct ctf_fs_ds_index { using UP = std::unique_ptr; - std::vector entries; + std::vector entries; }; struct ctf_fs_ds_file_group diff --git a/src/plugins/ctf/fs-src/fs.cpp b/src/plugins/ctf/fs-src/fs.cpp index d78aa3db..900db23f 100644 --- a/src/plugins/ctf/fs-src/fs.cpp +++ b/src/plugins/ctf/fs-src/fs.cpp @@ -355,22 +355,22 @@ static void ds_file_group_insert_ds_file_info_sorted(struct ctf_fs_ds_file_group ds_file_group->ds_file_infos.insert(it, std::move(ds_file_info)); } -static bool ds_index_entries_equal(const struct ctf_fs_ds_index_entry *left, - const struct ctf_fs_ds_index_entry *right) +static bool ds_index_entries_equal(const ctf_fs_ds_index_entry& left, + const ctf_fs_ds_index_entry& right) { - if (left->packetSize != right->packetSize) { + if (left.packetSize != right.packetSize) { return false; } - if (left->timestamp_begin != right->timestamp_begin) { + if (left.timestamp_begin != right.timestamp_begin) { return false; } - if (left->timestamp_end != right->timestamp_end) { + if (left.timestamp_end != right.timestamp_end) { return false; } - if (left->packet_seq_num != right->packet_seq_num) { + if (left.packet_seq_num != right.packet_seq_num) { return false; } @@ -381,18 +381,15 @@ static bool ds_index_entries_equal(const struct ctf_fs_ds_index_entry *left, * Insert `entry` into `index`, without duplication. * * The entry is inserted only if there isn't an identical entry already. - * - * In any case, the ownership of `entry` is transferred to this function. So if - * the entry is not inserted, it is freed. */ static void ds_index_insert_ds_index_entry_sorted(struct ctf_fs_ds_index *index, - ctf_fs_ds_index_entry::UP entry) + const ctf_fs_ds_index_entry& entry) { /* Find the spot where to insert this index entry. */ auto otherEntry = index->entries.begin(); for (; otherEntry != index->entries.end(); ++otherEntry) { - if (entry->timestamp_begin_ns <= (*otherEntry)->timestamp_begin_ns) { + if (entry.timestamp_begin_ns <= otherEntry->timestamp_begin_ns) { break; } } @@ -404,16 +401,15 @@ 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 (otherEntry == index->entries.end() || - !ds_index_entries_equal(entry.get(), otherEntry->get())) { - index->entries.insert(otherEntry, std::move(entry)); + if (otherEntry == index->entries.end() || !ds_index_entries_equal(entry, *otherEntry)) { + index->entries.emplace(otherEntry, entry); } } -static void merge_ctf_fs_ds_indexes(struct ctf_fs_ds_index *dest, ctf_fs_ds_index::UP src) +static void merge_ctf_fs_ds_indexes(struct ctf_fs_ds_index *dest, const ctf_fs_ds_index& src) { - for (auto& entry : src->entries) { - ds_index_insert_ds_index_entry_sorted(dest, std::move(entry)); + for (const auto& entry : src.entries) { + ds_index_insert_ds_index_entry_sorted(dest, entry); } } @@ -539,7 +535,7 @@ static int add_ds_file_to_ds_file_group(struct ctf_fs_trace *ctf_fs_trace, const ds_file_group = new_ds_file_group.get(); ctf_fs_trace->ds_file_groups.emplace_back(std::move(new_ds_file_group)); } else { - merge_ctf_fs_ds_indexes(ds_file_group->index.get(), std::move(index)); + merge_ctf_fs_ds_indexes(ds_file_group->index.get(), *index); } ds_file_group_insert_ds_file_info_sorted(ds_file_group, std::move(ds_file_info)); @@ -775,7 +771,7 @@ static void merge_ctf_fs_ds_file_groups(struct ctf_fs_ds_file_group *dest, } /* Merge both indexes. */ - merge_ctf_fs_ds_indexes(dest->index.get(), std::move(src->index)); + merge_ctf_fs_ds_indexes(dest->index.get(), *src->index); } /* Merge src_trace's data stream file groups into dest_trace's. */ @@ -921,7 +917,7 @@ enum target_event static int decode_clock_snapshot_after_event(struct ctf_fs_trace *ctf_fs_trace, struct ctf_clock_class *default_cc, - struct ctf_fs_ds_index_entry *index_entry, + const ctf_fs_ds_index_entry& index_entry, enum target_event target_event, uint64_t *cs, int64_t *ts_ns) { @@ -929,11 +925,10 @@ static int decode_clock_snapshot_after_event(struct ctf_fs_trace *ctf_fs_trace, ctf_msg_iter_up msg_iter; BT_ASSERT(ctf_fs_trace); - BT_ASSERT(index_entry); - BT_ASSERT(index_entry->path); + BT_ASSERT(index_entry.path); const auto ds_file = ctf_fs_ds_file_create(ctf_fs_trace, bt2::Stream::Shared {}, - index_entry->path, ctf_fs_trace->logger); + index_entry.path, ctf_fs_trace->logger); if (!ds_file) { BT_CPPLOGE_APPEND_CAUSE_SPEC(ctf_fs_trace->logger, "Failed to create a ctf_fs_ds_file"); return -1; @@ -958,7 +953,7 @@ static int decode_clock_snapshot_after_event(struct ctf_fs_trace *ctf_fs_trace, ctf_msg_iter_set_dry_run(msg_iter.get(), true); /* Seek to the beginning of the target packet. */ - iter_status = ctf_msg_iter_seek(msg_iter.get(), index_entry->offset.bytes()); + iter_status = ctf_msg_iter_seek(msg_iter.get(), index_entry.offset.bytes()); if (iter_status) { /* ctf_msg_iter_seek() logs errors. */ return -1; @@ -998,7 +993,7 @@ static int decode_clock_snapshot_after_event(struct ctf_fs_trace *ctf_fs_trace, static int decode_packet_first_event_timestamp(struct ctf_fs_trace *ctf_fs_trace, struct ctf_clock_class *default_cc, - struct ctf_fs_ds_index_entry *index_entry, + const ctf_fs_ds_index_entry& index_entry, uint64_t *cs, int64_t *ts_ns) { return decode_clock_snapshot_after_event(ctf_fs_trace, default_cc, index_entry, FIRST_EVENT, cs, @@ -1007,7 +1002,7 @@ static int decode_packet_first_event_timestamp(struct ctf_fs_trace *ctf_fs_trace static int decode_packet_last_event_timestamp(struct ctf_fs_trace *ctf_fs_trace, struct ctf_clock_class *default_cc, - struct ctf_fs_ds_index_entry *index_entry, + const ctf_fs_ds_index_entry& index_entry, uint64_t *cs, int64_t *ts_ns) { return decode_clock_snapshot_after_event(ctf_fs_trace, default_cc, index_entry, LAST_EVENT, cs, @@ -1048,23 +1043,22 @@ static int fix_index_lttng_event_after_packet_bug(struct ctf_fs_trace *trace) * fixed differently after. */ 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(); + auto& curr_entry = index->entries[entry_i]; + const auto& next_entry = index->entries[entry_i + 1]; /* * 1. Set the current index entry `end` timestamp to * the next index entry `begin` timestamp. */ - curr_entry->timestamp_end = next_entry->timestamp_begin; - curr_entry->timestamp_end_ns = next_entry->timestamp_begin_ns; + curr_entry.timestamp_end = next_entry.timestamp_begin; + curr_entry.timestamp_end_ns = next_entry.timestamp_begin_ns; } /* * 2. Fix the last entry by decoding the last event of the last * packet. */ - const auto last_entry = index->entries.back().get(); - BT_ASSERT(last_entry); + auto& last_entry = index->entries.back(); BT_ASSERT(ds_file_group->sc->default_clock_class); default_cc = ds_file_group->sc->default_clock_class; @@ -1073,9 +1067,8 @@ static int fix_index_lttng_event_after_packet_bug(struct ctf_fs_trace *trace) * Decode packet to read the timestamp of the last event of the * entry. */ - int ret = decode_packet_last_event_timestamp(trace, default_cc, last_entry, - &last_entry->timestamp_end, - &last_entry->timestamp_end_ns); + int ret = decode_packet_last_event_timestamp( + trace, default_cc, last_entry, &last_entry.timestamp_end, &last_entry.timestamp_end_ns); if (ret) { BT_CPPLOGE_APPEND_CAUSE_SPEC( trace->logger, @@ -1118,15 +1111,15 @@ static int fix_index_barectf_event_before_packet_bug(struct ctf_fs_trace *trace) * (index = 1). */ 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(); + auto& prev_entry = index->entries[entry_i - 1]; + auto& curr_entry = index->entries[entry_i]; /* * 2. Set the current entry `begin` timestamp to the * timestamp of the first event of the current packet. */ int ret = decode_packet_first_event_timestamp(trace, default_cc, curr_entry, - &curr_entry->timestamp_begin, - &curr_entry->timestamp_begin_ns); + &curr_entry.timestamp_begin, + &curr_entry.timestamp_begin_ns); if (ret) { BT_CPPLOGE_APPEND_CAUSE_SPEC(trace->logger, "Failed to decode first event's clock snapshot"); @@ -1137,8 +1130,8 @@ static int fix_index_barectf_event_before_packet_bug(struct ctf_fs_trace *trace) * 3. Set the previous entry `end` timestamp to the * timestamp of the first event of the current packet. */ - prev_entry->timestamp_end = curr_entry->timestamp_begin; - prev_entry->timestamp_end_ns = curr_entry->timestamp_begin_ns; + prev_entry.timestamp_end = curr_entry.timestamp_begin; + prev_entry.timestamp_end_ns = curr_entry.timestamp_begin_ns; } } @@ -1176,18 +1169,17 @@ static int fix_index_lttng_crash_quirk(struct ctf_fs_trace *trace) BT_ASSERT(index); BT_ASSERT(!index->entries.empty()); - const auto last_entry = index->entries.back().get(); - BT_ASSERT(last_entry); + auto& last_entry = index->entries.back(); /* 1. Fix the last entry first. */ - if (last_entry->timestamp_end == 0 && last_entry->timestamp_begin != 0) { + if (last_entry.timestamp_end == 0 && last_entry.timestamp_begin != 0) { /* * Decode packet to read the timestamp of the * last event of the stream file. */ int ret = decode_packet_last_event_timestamp(trace, default_cc, last_entry, - &last_entry->timestamp_end, - &last_entry->timestamp_end_ns); + &last_entry.timestamp_end, + &last_entry.timestamp_end_ns); if (ret) { BT_CPPLOGE_APPEND_CAUSE_SPEC(trace->logger, "Failed to decode last event's clock snapshot"); @@ -1197,16 +1189,16 @@ static int fix_index_lttng_crash_quirk(struct ctf_fs_trace *trace) /* Iterate over all entries but the last one. */ 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(); + auto& curr_entry = index->entries[entry_idx]; + const auto& next_entry = index->entries[entry_idx + 1]; - if (curr_entry->timestamp_end == 0 && curr_entry->timestamp_begin != 0) { + if (curr_entry.timestamp_end == 0 && curr_entry.timestamp_begin != 0) { /* * 2. Set the current index entry `end` timestamp to * the next index entry `begin` timestamp. */ - curr_entry->timestamp_end = next_entry->timestamp_begin; - curr_entry->timestamp_end_ns = next_entry->timestamp_begin_ns; + curr_entry.timestamp_end = next_entry.timestamp_begin; + curr_entry.timestamp_end_ns = next_entry.timestamp_begin_ns; } } } diff --git a/src/plugins/ctf/fs-src/query.cpp b/src/plugins/ctf/fs-src/query.cpp index b4b63908..890a3445 100644 --- a/src/plugins/ctf/fs-src/query.cpp +++ b/src/plugins/ctf/fs-src/query.cpp @@ -127,13 +127,13 @@ static void populate_stream_info(struct ctf_fs_ds_file_group *group, const bt2:: BT_ASSERT(!group->index->entries.empty()); /* First entry. */ - const auto first_ds_index_entry = group->index->entries.front().get(); + const auto& first_ds_index_entry = group->index->entries.front(); /* Last entry. */ - const auto last_ds_index_entry = group->index->entries.back().get(); + const auto& last_ds_index_entry = group->index->entries.back(); - stream_range->begin_ns = first_ds_index_entry->timestamp_begin_ns; - stream_range->end_ns = last_ds_index_entry->timestamp_end_ns; + stream_range->begin_ns = first_ds_index_entry.timestamp_begin_ns; + stream_range->end_ns = last_ds_index_entry.timestamp_end_ns; /* * If any of the begin and end timestamps is not set it means that -- 2.34.1