processor: Refactor TimestampedTracePiece
Chrome wants to add a few more counters to TrackEvents, which would
eventually also need a place in TimestampedTracePiece. Since TTP has
been growing a lot (and this contributed to increased memory usage in
trace processor), this patch refactors TimestampedTracePiece to make it
easier to add new data to only a subset of sorted events, while keeping
the TimestampedTracePiece of other events small.
Each TTP now has a type, and each type can store either inline or
heap-allocated supplemental data.
Bug: 123864183, 142557489
Change-Id: I050183c39bb24fadaad360886fee083577479328
diff --git a/Android.bp b/Android.bp
index 95b241f..9a0c021 100644
--- a/Android.bp
+++ b/Android.bp
@@ -5807,7 +5807,7 @@
"src/trace_processor/gzip_trace_parser.cc",
"src/trace_processor/heap_profile_tracker.cc",
"src/trace_processor/importers/ftrace/ftrace_module.cc",
- "src/trace_processor/importers/fuchsia/fuchsia_provider_view.cc",
+ "src/trace_processor/importers/fuchsia/fuchsia_record.cc",
"src/trace_processor/importers/fuchsia/fuchsia_trace_parser.cc",
"src/trace_processor/importers/fuchsia/fuchsia_trace_tokenizer.cc",
"src/trace_processor/importers/fuchsia/fuchsia_trace_utils.cc",
diff --git a/BUILD b/BUILD
index 504f825..b9f8000 100644
--- a/BUILD
+++ b/BUILD
@@ -897,8 +897,8 @@
"src/trace_processor/heap_profile_tracker.h",
"src/trace_processor/importers/ftrace/ftrace_module.cc",
"src/trace_processor/importers/ftrace/ftrace_module.h",
- "src/trace_processor/importers/fuchsia/fuchsia_provider_view.cc",
- "src/trace_processor/importers/fuchsia/fuchsia_provider_view.h",
+ "src/trace_processor/importers/fuchsia/fuchsia_record.cc",
+ "src/trace_processor/importers/fuchsia/fuchsia_record.h",
"src/trace_processor/importers/fuchsia/fuchsia_trace_parser.cc",
"src/trace_processor/importers/fuchsia/fuchsia_trace_parser.h",
"src/trace_processor/importers/fuchsia/fuchsia_trace_tokenizer.cc",
diff --git a/src/trace_processor/BUILD.gn b/src/trace_processor/BUILD.gn
index f7b6b64..097fd06 100644
--- a/src/trace_processor/BUILD.gn
+++ b/src/trace_processor/BUILD.gn
@@ -92,7 +92,7 @@
"heap_profile_tracker.h",
"importers/ftrace/ftrace_module.cc",
"importers/ftrace/ftrace_module.h",
- "importers/fuchsia/fuchsia_provider_view.h",
+ "importers/fuchsia/fuchsia_record.h",
"importers/proto/args_table_utils.cc",
"importers/proto/args_table_utils.h",
"importers/proto/chrome_compositor_scheduler_state.descriptor.h",
@@ -176,7 +176,7 @@
if (enable_perfetto_trace_processor_fuchsia) {
sources += [
"ftrace_utils.cc",
- "importers/fuchsia/fuchsia_provider_view.cc",
+ "importers/fuchsia/fuchsia_record.cc",
"importers/fuchsia/fuchsia_trace_parser.cc",
"importers/fuchsia/fuchsia_trace_parser.h",
"importers/fuchsia/fuchsia_trace_tokenizer.cc",
diff --git a/src/trace_processor/importers/ftrace/ftrace_parser.cc b/src/trace_processor/importers/ftrace/ftrace_parser.cc
index 3739fcc..af72c18 100644
--- a/src/trace_processor/importers/ftrace/ftrace_parser.cc
+++ b/src/trace_processor/importers/ftrace/ftrace_parser.cc
@@ -192,8 +192,8 @@
SchedEventTracker* sched_tracker = SchedEventTracker::GetOrCreate(context_);
// Handle the (optional) alternative encoding format for sched_switch.
- if (ttp.inline_event.type == InlineEvent::Type::kSchedSwitch) {
- const auto& event = ttp.inline_event.sched_switch;
+ if (ttp.type == TimestampedTracePiece::Type::kInlineSchedSwitch) {
+ const auto& event = ttp.sched_switch;
sched_tracker->PushSchedSwitchCompact(cpu, ts, event.prev_state,
static_cast<uint32_t>(event.next_pid),
event.next_prio, event.next_comm);
@@ -201,15 +201,16 @@
}
// Handle the (optional) alternative encoding format for sched_waking.
- if (ttp.inline_event.type == InlineEvent::Type::kSchedWaking) {
- const auto& event = ttp.inline_event.sched_waking;
+ if (ttp.type == TimestampedTracePiece::Type::kInlineSchedWaking) {
+ const auto& event = ttp.sched_waking;
sched_tracker->PushSchedWakingCompact(
cpu, ts, static_cast<uint32_t>(event.pid), event.target_cpu, event.prio,
event.comm);
return util::OkStatus();
}
- const TraceBlobView& event = ttp.blob_view;
+ PERFETTO_DCHECK(ttp.type == TimestampedTracePiece::Type::kFtraceEvent);
+ const TraceBlobView& event = ttp.ftrace_event;
ProtoDecoder decoder(event.data(), event.length());
uint64_t raw_pid = 0;
if (auto pid_field = decoder.FindField(FtraceEvent::kPidFieldNumber)) {
diff --git a/src/trace_processor/importers/ftrace/ftrace_tokenizer.cc b/src/trace_processor/importers/ftrace/ftrace_tokenizer.cc
index e94d604..341eb1d 100644
--- a/src/trace_processor/importers/ftrace/ftrace_tokenizer.cc
+++ b/src/trace_processor/importers/ftrace/ftrace_tokenizer.cc
@@ -153,8 +153,7 @@
event.next_pid = *npid_it;
event.next_prio = *nprio_it;
- context_->sorter->PushInlineFtraceEvent(cpu, event_timestamp,
- InlineEvent::SchedSwitch(event));
+ context_->sorter->PushInlineFtraceEvent(cpu, event_timestamp, event);
}
// Check that all packed buffers were decoded correctly, and fully.
@@ -197,8 +196,7 @@
event.target_cpu = *tcpu_it;
event.prio = *prio_it;
- context_->sorter->PushInlineFtraceEvent(cpu, event_timestamp,
- InlineEvent::SchedWaking(event));
+ context_->sorter->PushInlineFtraceEvent(cpu, event_timestamp, event);
}
// Check that all packed buffers were decoded correctly, and fully.
diff --git a/src/trace_processor/importers/fuchsia/fuchsia_provider_view.cc b/src/trace_processor/importers/fuchsia/fuchsia_record.cc
similarity index 74%
rename from src/trace_processor/importers/fuchsia/fuchsia_provider_view.cc
rename to src/trace_processor/importers/fuchsia/fuchsia_record.cc
index 6c69f6b..dfe70a6 100644
--- a/src/trace_processor/importers/fuchsia/fuchsia_provider_view.cc
+++ b/src/trace_processor/importers/fuchsia/fuchsia_record.cc
@@ -14,12 +14,12 @@
* limitations under the License.
*/
-#include "src/trace_processor/importers/fuchsia/fuchsia_provider_view.h"
+#include "src/trace_processor/importers/fuchsia/fuchsia_record.h"
namespace perfetto {
namespace trace_processor {
-void FuchsiaProviderView::InsertString(uint32_t index, StringId string_id) {
+void FuchsiaRecord::InsertString(uint32_t index, StringId string_id) {
StringTableEntry entry;
entry.index = index;
entry.string_id = string_id;
@@ -27,7 +27,7 @@
string_entries_.push_back(entry);
}
-StringId FuchsiaProviderView::GetString(uint32_t index) {
+StringId FuchsiaRecord::GetString(uint32_t index) {
for (const auto& entry : string_entries_) {
if (entry.index == index)
return entry.string_id;
@@ -35,8 +35,8 @@
return StringId();
}
-void FuchsiaProviderView::InsertThread(uint32_t index,
- fuchsia_trace_utils::ThreadInfo info) {
+void FuchsiaRecord::InsertThread(uint32_t index,
+ fuchsia_trace_utils::ThreadInfo info) {
ThreadTableEntry entry;
entry.index = index;
entry.info = info;
@@ -44,7 +44,7 @@
thread_entries_.push_back(entry);
}
-fuchsia_trace_utils::ThreadInfo FuchsiaProviderView::GetThread(uint32_t index) {
+fuchsia_trace_utils::ThreadInfo FuchsiaRecord::GetThread(uint32_t index) {
for (const auto& entry : thread_entries_) {
if (entry.index == index)
return entry.info;
diff --git a/src/trace_processor/importers/fuchsia/fuchsia_provider_view.h b/src/trace_processor/importers/fuchsia/fuchsia_record.h
similarity index 71%
rename from src/trace_processor/importers/fuchsia/fuchsia_provider_view.h
rename to src/trace_processor/importers/fuchsia/fuchsia_record.h
index a334ee4..c1fb0ca 100644
--- a/src/trace_processor/importers/fuchsia/fuchsia_provider_view.h
+++ b/src/trace_processor/importers/fuchsia/fuchsia_record.h
@@ -14,10 +14,11 @@
* limitations under the License.
*/
-#ifndef SRC_TRACE_PROCESSOR_IMPORTERS_FUCHSIA_FUCHSIA_PROVIDER_VIEW_H_
-#define SRC_TRACE_PROCESSOR_IMPORTERS_FUCHSIA_FUCHSIA_PROVIDER_VIEW_H_
+#ifndef SRC_TRACE_PROCESSOR_IMPORTERS_FUCHSIA_FUCHSIA_RECORD_H_
+#define SRC_TRACE_PROCESSOR_IMPORTERS_FUCHSIA_FUCHSIA_RECORD_H_
#include "src/trace_processor/importers/fuchsia/fuchsia_trace_utils.h"
+#include "src/trace_processor/trace_blob_view.h"
#include "src/trace_processor/trace_storage.h"
#include <vector>
@@ -26,11 +27,14 @@
namespace trace_processor {
// Data from a trace provider that is necessary for interpreting a binary
-// record. Namely, the entries of the string table and the thread table that are
-// referenced by the record. This enables understanding the binary record after
-// arbitrary reordering.
-class FuchsiaProviderView {
+// record. Namely, the record itself and the entries of the string table and the
+// thread table that are referenced by the record. This enables understanding
+// the binary record after arbitrary reordering.
+class FuchsiaRecord {
public:
+ FuchsiaRecord(TraceBlobView record_view)
+ : record_view_(std::move(record_view)) {}
+
struct StringTableEntry {
uint32_t index;
StringId string_id;
@@ -53,7 +57,11 @@
uint64_t get_ticks_per_second() { return ticks_per_second_; }
+ TraceBlobView* record_view() { return &record_view_; }
+
private:
+ TraceBlobView record_view_;
+
std::vector<StringTableEntry> string_entries_;
std::vector<ThreadTableEntry> thread_entries_;
@@ -63,4 +71,4 @@
} // namespace trace_processor
} // namespace perfetto
-#endif // SRC_TRACE_PROCESSOR_IMPORTERS_FUCHSIA_FUCHSIA_PROVIDER_VIEW_H_
+#endif // SRC_TRACE_PROCESSOR_IMPORTERS_FUCHSIA_FUCHSIA_RECORD_H_
diff --git a/src/trace_processor/importers/fuchsia/fuchsia_trace_parser.cc b/src/trace_processor/importers/fuchsia/fuchsia_trace_parser.cc
index bd118378..67dafce 100644
--- a/src/trace_processor/importers/fuchsia/fuchsia_trace_parser.cc
+++ b/src/trace_processor/importers/fuchsia/fuchsia_trace_parser.cc
@@ -68,12 +68,15 @@
}
void FuchsiaTraceParser::ParseTracePacket(int64_t, TimestampedTracePiece ttp) {
- PERFETTO_DCHECK(ttp.fuchsia_provider_view != nullptr);
+ PERFETTO_DCHECK(ttp.type == TimestampedTracePiece::Type::kFuchsiaRecord);
+ PERFETTO_DCHECK(ttp.fuchsia_record != nullptr);
// The timestamp is also present in the record, so we'll ignore the one passed
// as an argument.
- fuchsia_trace_utils::RecordCursor cursor(&ttp.blob_view);
- FuchsiaProviderView* provider_view = ttp.fuchsia_provider_view.get();
+ fuchsia_trace_utils::RecordCursor cursor(
+ ttp.fuchsia_record->record_view()->data(),
+ ttp.fuchsia_record->record_view()->length());
+ FuchsiaRecord* record = ttp.fuchsia_record.get();
ProcessTracker* procs = context_->process_tracker.get();
SliceTracker* slices = context_->slice_tracker.get();
@@ -97,7 +100,7 @@
fuchsia_trace_utils::ReadField<uint32_t>(header, 48, 63);
int64_t ts;
- if (!cursor.ReadTimestamp(provider_view->get_ticks_per_second(), &ts)) {
+ if (!cursor.ReadTimestamp(record->get_ticks_per_second(), &ts)) {
context_->storage->IncrementStats(stats::fuchsia_invalid_event);
return;
}
@@ -108,7 +111,7 @@
return;
}
} else {
- tinfo = provider_view->GetThread(thread_ref);
+ tinfo = record->GetThread(thread_ref);
}
StringId cat;
if (fuchsia_trace_utils::IsInlineString(cat_ref)) {
@@ -119,7 +122,7 @@
}
cat = context_->storage->InternString(cat_string_view);
} else {
- cat = provider_view->GetString(cat_ref);
+ cat = record->GetString(cat_ref);
}
StringId name;
if (fuchsia_trace_utils::IsInlineString(name_ref)) {
@@ -130,7 +133,7 @@
}
name = context_->storage->InternString(name_string_view);
} else {
- name = provider_view->GetString(name_ref);
+ name = record->GetString(name_ref);
}
// Read arguments
@@ -157,7 +160,7 @@
}
arg.name = context_->storage->InternString(arg_name_view);
} else {
- arg.name = provider_view->GetString(arg_name_ref);
+ arg.name = record->GetString(arg_name_ref);
}
switch (arg_type) {
@@ -211,7 +214,7 @@
}
value = context_->storage->InternString(arg_value_view);
} else {
- value = provider_view->GetString(arg_value_ref);
+ value = record->GetString(arg_value_ref);
}
arg.value = fuchsia_trace_utils::ArgValue::String(value);
break;
@@ -335,8 +338,7 @@
}
case kDurationComplete: {
int64_t end_ts;
- if (!cursor.ReadTimestamp(provider_view->get_ticks_per_second(),
- &end_ts)) {
+ if (!cursor.ReadTimestamp(record->get_ticks_per_second(), &end_ts)) {
context_->storage->IncrementStats(stats::fuchsia_invalid_event);
return;
}
diff --git a/src/trace_processor/importers/fuchsia/fuchsia_trace_parser.h b/src/trace_processor/importers/fuchsia/fuchsia_trace_parser.h
index bc54d47..581704d 100644
--- a/src/trace_processor/importers/fuchsia/fuchsia_trace_parser.h
+++ b/src/trace_processor/importers/fuchsia/fuchsia_trace_parser.h
@@ -17,7 +17,7 @@
#ifndef SRC_TRACE_PROCESSOR_IMPORTERS_FUCHSIA_FUCHSIA_TRACE_PARSER_H_
#define SRC_TRACE_PROCESSOR_IMPORTERS_FUCHSIA_FUCHSIA_TRACE_PARSER_H_
-#include "src/trace_processor/importers/fuchsia/fuchsia_provider_view.h"
+#include "src/trace_processor/importers/fuchsia/fuchsia_record.h"
#include "src/trace_processor/timestamped_trace_piece.h"
#include "src/trace_processor/trace_parser.h"
diff --git a/src/trace_processor/importers/fuchsia/fuchsia_trace_tokenizer.cc b/src/trace_processor/importers/fuchsia/fuchsia_trace_tokenizer.cc
index 9f58cc9..ac6b064 100644
--- a/src/trace_processor/importers/fuchsia/fuchsia_trace_tokenizer.cc
+++ b/src/trace_processor/importers/fuchsia/fuchsia_trace_tokenizer.cc
@@ -22,7 +22,7 @@
#include "perfetto/base/logging.h"
#include "perfetto/ext/base/string_view.h"
#include "src/trace_processor/ftrace_utils.h"
-#include "src/trace_processor/importers/fuchsia/fuchsia_provider_view.h"
+#include "src/trace_processor/importers/fuchsia/fuchsia_record.h"
#include "src/trace_processor/process_tracker.h"
#include "src/trace_processor/slice_tracker.h"
#include "src/trace_processor/trace_processor_context.h"
@@ -187,7 +187,7 @@
ProcessTracker* procs = context_->process_tracker.get();
TraceSorter* sorter = context_->sorter.get();
- fuchsia_trace_utils::RecordCursor cursor(&tbv);
+ fuchsia_trace_utils::RecordCursor cursor(tbv.data(), tbv.length());
uint64_t header;
if (!cursor.ReadUint64(&header)) {
context_->storage->IncrementStats(stats::fuchsia_invalid_event);
@@ -278,13 +278,12 @@
uint32_t name_ref =
fuchsia_trace_utils::ReadField<uint32_t>(header, 48, 63);
- // Build the minimal FuchsiaProviderView needed by
- // the record. This means the thread information if not inline, and any
- // non-inline strings (name, category for now, arg names and string values
- // in the future.
- auto provider_view =
- std::unique_ptr<FuchsiaProviderView>(new FuchsiaProviderView());
- provider_view->set_ticks_per_second(current_provider_->ticks_per_second);
+ // Build the FuchsiaRecord for the event, i.e. extract the thread
+ // information if not inline, and any non-inline strings (name, category
+ // for now, arg names and string values in the future).
+ auto record =
+ std::unique_ptr<FuchsiaRecord>(new FuchsiaRecord(std::move(tbv)));
+ record->set_ticks_per_second(current_provider_->ticks_per_second);
uint64_t ticks;
if (!cursor.ReadUint64(&ticks)) {
@@ -302,24 +301,23 @@
// Skip over inline thread
cursor.ReadInlineThread(nullptr);
} else {
- provider_view->InsertThread(
- thread_ref, current_provider_->thread_table[thread_ref]);
+ record->InsertThread(thread_ref,
+ current_provider_->thread_table[thread_ref]);
}
if (fuchsia_trace_utils::IsInlineString(cat_ref)) {
// Skip over inline string
cursor.ReadInlineString(cat_ref, nullptr);
} else {
- provider_view->InsertString(cat_ref,
- current_provider_->string_table[cat_ref]);
+ record->InsertString(cat_ref, current_provider_->string_table[cat_ref]);
}
if (fuchsia_trace_utils::IsInlineString(name_ref)) {
// Skip over inline string
cursor.ReadInlineString(name_ref, nullptr);
} else {
- provider_view->InsertString(name_ref,
- current_provider_->string_table[name_ref]);
+ record->InsertString(name_ref,
+ current_provider_->string_table[name_ref]);
}
uint32_t n_args =
@@ -342,8 +340,8 @@
// Skip over inline string
cursor.ReadInlineString(arg_name_ref, nullptr);
} else {
- provider_view->InsertString(
- arg_name_ref, current_provider_->string_table[arg_name_ref]);
+ record->InsertString(arg_name_ref,
+ current_provider_->string_table[arg_name_ref]);
}
if (arg_type == kArgString) {
@@ -353,7 +351,7 @@
// Skip over inline string
cursor.ReadInlineString(arg_value_ref, nullptr);
} else {
- provider_view->InsertString(
+ record->InsertString(
arg_value_ref, current_provider_->string_table[arg_value_ref]);
}
}
@@ -361,8 +359,7 @@
cursor.SetWordIndex(arg_base + arg_size_words);
}
- sorter->PushFuchsiaRecord(ts, std::move(tbv), std::move(provider_view));
-
+ sorter->PushFuchsiaRecord(ts, std::move(record));
break;
}
case kKernelObject: {
diff --git a/src/trace_processor/importers/fuchsia/fuchsia_trace_utils.cc b/src/trace_processor/importers/fuchsia/fuchsia_trace_utils.cc
index 1edc8ba..8b02c87 100644
--- a/src/trace_processor/importers/fuchsia/fuchsia_trace_utils.cc
+++ b/src/trace_processor/importers/fuchsia/fuchsia_trace_utils.cc
@@ -174,13 +174,12 @@
}
bool RecordCursor::ReadWords(size_t num_words, const uint8_t** data_out) {
- const uint8_t* end = tbv_.data() + tbv_.length();
- const uint8_t* data = tbv_.data() + sizeof(uint64_t) * word_index_;
+ const uint8_t* data = begin_ + sizeof(uint64_t) * word_index_;
// This addition is unconditional so that callers with data_out == nullptr do
// not necessarily have to check the return value, as future calls will fail
// due to attempting to read out of bounds.
word_index_ += num_words;
- if (data + sizeof(uint64_t) * num_words <= end) {
+ if (data + sizeof(uint64_t) * num_words <= end_) {
if (data_out != nullptr) {
*data_out = data;
}
diff --git a/src/trace_processor/importers/fuchsia/fuchsia_trace_utils.h b/src/trace_processor/importers/fuchsia/fuchsia_trace_utils.h
index c67b77f..2ccc7a6 100644
--- a/src/trace_processor/importers/fuchsia/fuchsia_trace_utils.h
+++ b/src/trace_processor/importers/fuchsia/fuchsia_trace_utils.h
@@ -195,7 +195,8 @@
// https://fuchsia.googlesource.com/fuchsia/+/refs/heads/master/docs/development/tracing/trace-format/
class RecordCursor {
public:
- RecordCursor(const TraceBlobView* tbv) : tbv_(*tbv), word_index_(0) {}
+ RecordCursor(const uint8_t* begin, size_t length)
+ : begin_(begin), end_(begin + length), word_index_(0) {}
size_t WordIndex();
void SetWordIndex(size_t index);
@@ -212,7 +213,8 @@
private:
bool ReadWords(size_t num_words, const uint8_t** data_out);
- const TraceBlobView& tbv_;
+ const uint8_t* begin_;
+ const uint8_t* end_;
size_t word_index_;
};
diff --git a/src/trace_processor/importers/proto/graphics_event_module.cc b/src/trace_processor/importers/proto/graphics_event_module.cc
index 4c9bfc2..e9fda0a 100644
--- a/src/trace_processor/importers/proto/graphics_event_module.cc
+++ b/src/trace_processor/importers/proto/graphics_event_module.cc
@@ -52,9 +52,11 @@
decoder.graphics_frame_event());
return;
case TracePacket::kVulkanMemoryEventFieldNumber:
- parser_.ParseVulkanMemoryEvent(ttp.packet_sequence_state,
- ttp.packet_sequence_state_generation,
- decoder.vulkan_memory_event());
+ PERFETTO_DCHECK(ttp.type == TimestampedTracePiece::Type::kTracePacket);
+ parser_.ParseVulkanMemoryEvent(
+ ttp.packet_data.packet_sequence_state,
+ ttp.packet_data.packet_sequence_state_generation,
+ decoder.vulkan_memory_event());
return;
case TracePacket::kVulkanApiEventFieldNumber:
parser_.ParseVulkanApiEvent(decoder.vulkan_api_event());
diff --git a/src/trace_processor/importers/proto/proto_trace_parser.cc b/src/trace_processor/importers/proto/proto_trace_parser.cc
index e30f703..eb32e46 100644
--- a/src/trace_processor/importers/proto/proto_trace_parser.cc
+++ b/src/trace_processor/importers/proto/proto_trace_parser.cc
@@ -181,12 +181,18 @@
ProtoTraceParser::~ProtoTraceParser() = default;
void ProtoTraceParser::ParseTracePacket(int64_t ts, TimestampedTracePiece ttp) {
- PERFETTO_DCHECK(ttp.json_value == nullptr);
+ const TracePacketData* data = nullptr;
+ if (ttp.type == TimestampedTracePiece::Type::kTracePacket) {
+ data = &ttp.packet_data;
+ } else {
+ PERFETTO_DCHECK(ttp.type == TimestampedTracePiece::Type::kTrackEvent);
+ data = ttp.track_event_data.get();
+ }
- const TraceBlobView& blob = ttp.blob_view;
+ const TraceBlobView& blob = data->packet;
protos::pbzero::TracePacket::Decoder packet(blob.data(), blob.length());
- ParseTracePacketImpl(ts, std::move(ttp), packet);
+ ParseTracePacketImpl(ts, std::move(ttp), data, packet);
// TODO(lalitm): maybe move this to the flush method in the trace processor
// once we have it. This may reduce performance in the ArgsTracker though so
@@ -198,6 +204,7 @@
void ProtoTraceParser::ParseTracePacketImpl(
int64_t ts,
TimestampedTracePiece ttp,
+ const TracePacketData* data,
const protos::pbzero::TracePacket::Decoder& packet) {
// TODO(eseckler): Propagate statuses from modules.
auto& modules = context_->modules_by_field;
@@ -213,13 +220,13 @@
if (packet.has_profile_packet()) {
ParseProfilePacket(
- ts, ttp.packet_sequence_state, ttp.packet_sequence_state_generation,
+ ts, data->packet_sequence_state, data->packet_sequence_state_generation,
packet.trusted_packet_sequence_id(), packet.profile_packet());
}
if (packet.has_streaming_profile_packet()) {
- ParseStreamingProfilePacket(ttp.packet_sequence_state,
- ttp.packet_sequence_state_generation,
+ ParseStreamingProfilePacket(data->packet_sequence_state,
+ data->packet_sequence_state_generation,
packet.streaming_profile_packet());
}
@@ -247,7 +254,9 @@
void ProtoTraceParser::ParseFtracePacket(uint32_t cpu,
int64_t /*ts*/,
TimestampedTracePiece ttp) {
- PERFETTO_DCHECK(ttp.json_value == nullptr);
+ PERFETTO_DCHECK(ttp.type == TimestampedTracePiece::Type::kFtraceEvent ||
+ ttp.type == TimestampedTracePiece::Type::kInlineSchedSwitch ||
+ ttp.type == TimestampedTracePiece::Type::kInlineSchedWaking);
PERFETTO_DCHECK(context_->ftrace_module);
context_->ftrace_module->ParseFtracePacket(cpu, ttp);
diff --git a/src/trace_processor/importers/proto/proto_trace_parser.h b/src/trace_processor/importers/proto/proto_trace_parser.h
index 4827ebf..25b4367 100644
--- a/src/trace_processor/importers/proto/proto_trace_parser.h
+++ b/src/trace_processor/importers/proto/proto_trace_parser.h
@@ -58,6 +58,7 @@
void ParseTracePacketImpl(int64_t ts,
TimestampedTracePiece,
+ const TracePacketData*,
const protos::pbzero::TracePacket_Decoder&);
void ParseTraceStats(ConstBytes);
diff --git a/src/trace_processor/importers/proto/track_event_module.cc b/src/trace_processor/importers/proto/track_event_module.cc
index 8e80a80..a96cf53 100644
--- a/src/trace_processor/importers/proto/track_event_module.cc
+++ b/src/trace_processor/importers/proto/track_event_module.cc
@@ -65,9 +65,12 @@
const TimestampedTracePiece& ttp,
uint32_t field_id) {
if (field_id == TracePacket::kTrackEventFieldNumber) {
+ PERFETTO_DCHECK(ttp.type == TimestampedTracePiece::Type::kTrackEvent);
parser_.ParseTrackEvent(
- ttp.timestamp, ttp.thread_timestamp, ttp.thread_instruction_count,
- ttp.packet_sequence_state, ttp.packet_sequence_state_generation,
+ ttp.timestamp, ttp.track_event_data->thread_timestamp,
+ ttp.track_event_data->thread_instruction_count,
+ ttp.track_event_data->packet_sequence_state,
+ ttp.track_event_data->packet_sequence_state_generation,
decoder.track_event());
}
}
diff --git a/src/trace_processor/timestamped_trace_piece.h b/src/trace_processor/timestamped_trace_piece.h
index 83b4932..464864c 100644
--- a/src/trace_processor/timestamped_trace_piece.h
+++ b/src/trace_processor/timestamped_trace_piece.h
@@ -19,7 +19,7 @@
#include "perfetto/base/build_config.h"
#include "perfetto/trace_processor/basic_types.h"
-#include "src/trace_processor/importers/fuchsia/fuchsia_provider_view.h"
+#include "src/trace_processor/importers/fuchsia/fuchsia_record.h"
#include "src/trace_processor/importers/proto/packet_sequence_state.h"
#include "src/trace_processor/trace_blob_view.h"
#include "src/trace_processor/trace_processor_context.h"
@@ -34,6 +34,15 @@
} // namespace Json
#endif // PERFETTO_BUILDFLAG(PERFETTO_TP_JSON_IMPORT)
+// GCC can't figure out the relationship between TimestampedTracePiece's type
+// and the union, and thus thinks that we may be moving or destroying
+// uninitialized data in the move constructors / destructors. Disable those
+// warnings for TimestampedTracePiece and the types it contains.
+#if PERFETTO_BUILDFLAG(PERFETTO_COMPILER_GCC)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wmaybe-uninitialized"
+#endif
+
namespace perfetto {
namespace trace_processor {
@@ -51,143 +60,167 @@
StringId comm;
};
-// Discriminated union of events that are cannot be easily read from the
-// mapped trace.
-struct InlineEvent {
- enum class Type { kInvalid = 0, kSchedSwitch, kSchedWaking };
+struct TracePacketData {
+ TraceBlobView packet;
- static InlineEvent SchedSwitch(InlineSchedSwitch content) {
- InlineEvent evt;
- evt.type = Type::kSchedSwitch;
- evt.sched_switch = content;
- return evt;
- }
+ // TODO(eseckler): Refactor this into a single pointer to a PSSGeneration.
+ PacketSequenceState* packet_sequence_state;
+ size_t packet_sequence_state_generation;
+};
- static InlineEvent SchedWaking(InlineSchedWaking content) {
- InlineEvent evt;
- evt.type = Type::kSchedWaking;
- evt.sched_waking = content;
- return evt;
- }
+struct TrackEventData : public TracePacketData {
+ TrackEventData(TraceBlobView pv,
+ PacketSequenceState* pss,
+ size_t pss_generation,
+ int64_t thread_ts,
+ int64_t thread_ic)
+ : TracePacketData{std::move(pv), pss, pss_generation},
+ thread_timestamp(thread_ts),
+ thread_instruction_count(thread_ic) {}
- Type type = Type::kInvalid;
- union {
- InlineSchedSwitch sched_switch;
- InlineSchedWaking sched_waking;
- };
+ int64_t thread_timestamp;
+ int64_t thread_instruction_count;
};
// A TimestampedTracePiece is (usually a reference to) a piece of a trace that
// is sorted by TraceSorter.
struct TimestampedTracePiece {
+ enum class Type {
+ kInvalid = 0,
+ kFtraceEvent,
+ kTracePacket,
+ kInlineSchedSwitch,
+ kInlineSchedWaking,
+ kJsonValue,
+ kFuchsiaRecord,
+ kTrackEvent
+ };
+
TimestampedTracePiece(int64_t ts,
uint64_t idx,
TraceBlobView tbv,
PacketSequenceState* sequence_state)
- : TimestampedTracePiece(ts,
- /*thread_ts=*/0,
- /*thread_instructions=*/0,
- idx,
- std::move(tbv),
- /*value=*/nullptr,
- /*fpv=*/nullptr,
- /*sequence_state=*/sequence_state,
- InlineEvent{}) {}
+ : packet_data{std::move(tbv), sequence_state,
+ sequence_state ? sequence_state->current_generation() : 0},
+ timestamp(ts),
+ packet_idx(idx),
+ type(Type::kTracePacket) {}
TimestampedTracePiece(int64_t ts, uint64_t idx, TraceBlobView tbv)
- : TimestampedTracePiece(ts,
- /*thread_ts=*/0,
- /*thread_instructions=*/0,
- idx,
- std::move(tbv),
- /*value=*/nullptr,
- /*fpv=*/nullptr,
- /*sequence_state=*/nullptr,
- InlineEvent{}) {}
+ : ftrace_event(std::move(tbv)),
+ timestamp(ts),
+ packet_idx(idx),
+ type(Type::kFtraceEvent) {}
TimestampedTracePiece(int64_t ts,
uint64_t idx,
std::unique_ptr<Json::Value> value)
- : TimestampedTracePiece(ts,
- /*thread_ts=*/0,
- /*thread_instructions=*/0,
- idx,
- // TODO(dproy): Stop requiring TraceBlobView in
- // TimestampedTracePiece.
- TraceBlobView(nullptr, 0, 0),
- std::move(value),
- /*fpv=*/nullptr,
- /*sequence_state=*/nullptr,
- InlineEvent{}) {}
-
- TimestampedTracePiece(int64_t ts,
- uint64_t idx,
- TraceBlobView tbv,
- std::unique_ptr<FuchsiaProviderView> fpv)
- : TimestampedTracePiece(ts,
- /*thread_ts=*/0,
- /*thread_instructions=*/0,
- idx,
- std::move(tbv),
- /*value=*/nullptr,
- std::move(fpv),
- /*sequence_state=*/nullptr,
- InlineEvent{}) {}
-
- TimestampedTracePiece(int64_t ts,
- int64_t thread_ts,
- int64_t thread_instructions,
- uint64_t idx,
- TraceBlobView tbv,
- PacketSequenceState* sequence_state)
- : TimestampedTracePiece(ts,
- thread_ts,
- thread_instructions,
- idx,
- std::move(tbv),
- /*value=*/nullptr,
- /*fpv=*/nullptr,
- sequence_state,
- InlineEvent{}) {}
-
- // TODO(rsavitski): each "empty" TraceBlobView created by this constructor
- // still allocates ref-counting structures for the nonexistent memory.
- // It's not a significant overhead, but consider making the class have a
- // legitimate empty state.
- TimestampedTracePiece(int64_t ts, uint64_t idx, InlineEvent inline_evt)
- : TimestampedTracePiece(ts,
- /*thread_ts=*/0,
- /*thread_instructions=*/0,
- idx,
- TraceBlobView(nullptr, 0, 0),
- /*value=*/nullptr,
- /*fpv=*/nullptr,
- /*sequence_state=*/nullptr,
- inline_evt) {}
-
- TimestampedTracePiece(int64_t ts,
- int64_t thread_ts,
- int64_t thread_instructions,
- uint64_t idx,
- TraceBlobView tbv,
- std::unique_ptr<Json::Value> value,
- std::unique_ptr<FuchsiaProviderView> fpv,
- PacketSequenceState* sequence_state,
- InlineEvent inline_evt)
: json_value(std::move(value)),
- fuchsia_provider_view(std::move(fpv)),
- packet_sequence_state(sequence_state),
- packet_sequence_state_generation(
- sequence_state ? sequence_state->current_generation() : 0),
timestamp(ts),
- thread_timestamp(thread_ts),
- thread_instruction_count(thread_instructions),
- packet_idx_(idx),
- blob_view(std::move(tbv)),
- inline_event(inline_evt) {}
+ packet_idx(idx),
+ type(Type::kJsonValue) {}
- TimestampedTracePiece(TimestampedTracePiece&&) noexcept = default;
- TimestampedTracePiece& operator=(TimestampedTracePiece&&) = default;
+ TimestampedTracePiece(int64_t ts,
+ uint64_t idx,
+ std::unique_ptr<FuchsiaRecord> fr)
+ : fuchsia_record(std::move(fr)),
+ timestamp(ts),
+ packet_idx(idx),
+ type(Type::kFuchsiaRecord) {}
+
+ TimestampedTracePiece(int64_t ts,
+ uint64_t idx,
+ std::unique_ptr<TrackEventData> ted)
+ : track_event_data(std::move(ted)),
+ timestamp(ts),
+ packet_idx(idx),
+ type(Type::kTrackEvent) {}
+
+ TimestampedTracePiece(int64_t ts, uint64_t idx, InlineSchedSwitch iss)
+ : sched_switch(std::move(iss)),
+ timestamp(ts),
+ packet_idx(idx),
+ type(Type::kInlineSchedSwitch) {}
+
+ TimestampedTracePiece(int64_t ts, uint64_t idx, InlineSchedWaking isw)
+ : sched_waking(std::move(isw)),
+ timestamp(ts),
+ packet_idx(idx),
+ type(Type::kInlineSchedWaking) {}
+
+ TimestampedTracePiece(TimestampedTracePiece&& ttp) noexcept {
+ // Adopt |ttp|'s data. We have to use placement-new to fill the fields
+ // because their original values may be uninitialized and thus
+ // move-assignment won't work correctly.
+ switch (ttp.type) {
+ case Type::kInvalid:
+ break;
+ case Type::kFtraceEvent:
+ new (&ftrace_event) TraceBlobView(std::move(ttp.ftrace_event));
+ break;
+ case Type::kTracePacket:
+ new (&packet_data) TracePacketData(std::move(ttp.packet_data));
+ break;
+ case Type::kInlineSchedSwitch:
+ new (&sched_switch) InlineSchedSwitch(std::move(ttp.sched_switch));
+ break;
+ case Type::kInlineSchedWaking:
+ new (&sched_waking) InlineSchedWaking(std::move(ttp.sched_waking));
+ break;
+ case Type::kJsonValue:
+ new (&json_value)
+ std::unique_ptr<Json::Value>(std::move(ttp.json_value));
+ break;
+ case Type::kFuchsiaRecord:
+ new (&fuchsia_record)
+ std::unique_ptr<FuchsiaRecord>(std::move(ttp.fuchsia_record));
+ break;
+ case Type::kTrackEvent:
+ new (&track_event_data)
+ std::unique_ptr<TrackEventData>(std::move(ttp.track_event_data));
+ break;
+ }
+ timestamp = ttp.timestamp;
+ packet_idx = ttp.packet_idx;
+ type = ttp.type;
+
+ // Invalidate |ttp|.
+ ttp.type = Type::kInvalid;
+ }
+
+ TimestampedTracePiece& operator=(TimestampedTracePiece&& ttp) {
+ if (this != &ttp) {
+ // First invoke the destructor and then invoke the move constructor
+ // inline via placement-new to implement move-assignment.
+ this->~TimestampedTracePiece();
+ new (this) TimestampedTracePiece(std::move(ttp));
+ }
+ return *this;
+ }
+
+ ~TimestampedTracePiece() {
+ switch (type) {
+ case Type::kInvalid:
+ case Type::kInlineSchedSwitch:
+ case Type::kInlineSchedWaking:
+ break;
+ case Type::kFtraceEvent:
+ ftrace_event.~TraceBlobView();
+ break;
+ case Type::kTracePacket:
+ packet_data.~TracePacketData();
+ break;
+ case Type::kJsonValue:
+ json_value.~unique_ptr();
+ break;
+ case Type::kFuchsiaRecord:
+ fuchsia_record.~unique_ptr();
+ break;
+ case Type::kTrackEvent:
+ track_event_data.~unique_ptr();
+ break;
+ }
+ }
// For std::lower_bound().
static inline bool Compare(const TimestampedTracePiece& x, int64_t ts) {
@@ -197,23 +230,32 @@
// For std::sort().
inline bool operator<(const TimestampedTracePiece& o) const {
return timestamp < o.timestamp ||
- (timestamp == o.timestamp && packet_idx_ < o.packet_idx_);
+ (timestamp == o.timestamp && packet_idx < o.packet_idx);
}
- std::unique_ptr<Json::Value> json_value;
- std::unique_ptr<FuchsiaProviderView> fuchsia_provider_view;
- PacketSequenceState* packet_sequence_state;
- size_t packet_sequence_state_generation;
+ // Fields ordered for packing.
+
+ // Data for different types of TimestampedTracePiece.
+ union {
+ TraceBlobView ftrace_event;
+ TracePacketData packet_data;
+ InlineSchedSwitch sched_switch;
+ InlineSchedWaking sched_waking;
+ std::unique_ptr<Json::Value> json_value;
+ std::unique_ptr<FuchsiaRecord> fuchsia_record;
+ std::unique_ptr<TrackEventData> track_event_data;
+ };
int64_t timestamp;
- int64_t thread_timestamp;
- int64_t thread_instruction_count;
- uint64_t packet_idx_;
- TraceBlobView blob_view;
- InlineEvent inline_event;
+ uint64_t packet_idx;
+ Type type;
};
} // namespace trace_processor
} // namespace perfetto
+#if PERFETTO_BUILDFLAG(PERFETTO_COMPILER_GCC)
+#pragma GCC diagnostic pop
+#endif
+
#endif // SRC_TRACE_PROCESSOR_TIMESTAMPED_TRACE_PIECE_H_
diff --git a/src/trace_processor/trace_sorter.h b/src/trace_processor/trace_sorter.h
index dcf65b4..0b8502a 100644
--- a/src/trace_processor/trace_sorter.h
+++ b/src/trace_processor/trace_sorter.h
@@ -85,14 +85,12 @@
MaybeExtractEvents(queue);
}
- inline void PushFuchsiaRecord(
- int64_t timestamp,
- TraceBlobView record,
- std::unique_ptr<FuchsiaProviderView> provider_view) {
+ inline void PushFuchsiaRecord(int64_t timestamp,
+ std::unique_ptr<FuchsiaRecord> record) {
DCHECK_ftrace_batch_cpu(kNoBatch);
auto* queue = GetQueue(0);
- queue->Append(TimestampedTracePiece(
- timestamp, packet_idx_++, std::move(record), std::move(provider_view)));
+ queue->Append(
+ TimestampedTracePiece(timestamp, packet_idx_++, std::move(record)));
MaybeExtractEvents(queue);
}
@@ -118,10 +116,17 @@
// instead.
inline void PushInlineFtraceEvent(uint32_t cpu,
int64_t timestamp,
- InlineEvent inline_event) {
+ InlineSchedSwitch inline_sched_switch) {
set_ftrace_batch_cpu_for_DCHECK(cpu);
GetQueue(cpu + 1)->Append(
- TimestampedTracePiece(timestamp, packet_idx_++, inline_event));
+ TimestampedTracePiece(timestamp, packet_idx_++, inline_sched_switch));
+ }
+ inline void PushInlineFtraceEvent(uint32_t cpu,
+ int64_t timestamp,
+ InlineSchedWaking inline_sched_waking) {
+ set_ftrace_batch_cpu_for_DCHECK(cpu);
+ GetQueue(cpu + 1)->Append(
+ TimestampedTracePiece(timestamp, packet_idx_++, inline_sched_waking));
}
inline void PushTrackEventPacket(int64_t timestamp,
@@ -130,9 +135,11 @@
PacketSequenceState* state,
TraceBlobView packet) {
auto* queue = GetQueue(0);
- queue->Append(TimestampedTracePiece(timestamp, thread_time,
- thread_instruction_count, packet_idx_++,
- std::move(packet), state));
+ std::unique_ptr<TrackEventData> data(new TrackEventData{
+ std::move(packet), state, state->current_generation(), thread_time,
+ thread_instruction_count});
+ queue->Append(
+ TimestampedTracePiece(timestamp, packet_idx_++, std::move(data)));
MaybeExtractEvents(queue);
}
diff --git a/src/trace_processor/trace_sorter_unittest.cc b/src/trace_processor/trace_sorter_unittest.cc
index fa0a34a..8f4ea0b 100644
--- a/src/trace_processor/trace_sorter_unittest.cc
+++ b/src/trace_processor/trace_sorter_unittest.cc
@@ -48,15 +48,17 @@
void ParseFtracePacket(uint32_t cpu,
int64_t timestamp,
TimestampedTracePiece ttp) override {
- TraceBlobView& tbv = ttp.blob_view;
- MOCK_ParseFtracePacket(cpu, timestamp, tbv.data(), tbv.length());
+ bool isNonCompact = ttp.type == TimestampedTracePiece::Type::kFtraceEvent;
+ MOCK_ParseFtracePacket(cpu, timestamp,
+ isNonCompact ? ttp.ftrace_event.data() : nullptr,
+ isNonCompact ? ttp.ftrace_event.length() : 0);
}
MOCK_METHOD3(MOCK_ParseTracePacket,
void(int64_t ts, const uint8_t* data, size_t length));
void ParseTracePacket(int64_t ts, TimestampedTracePiece ttp) override {
- TraceBlobView& tbv = ttp.blob_view;
+ TraceBlobView& tbv = ttp.packet_data.packet;
MOCK_ParseTracePacket(ts, tbv.data(), tbv.length());
}
};