trace_processor: add Id wrapper type returned by Insert in all db tables
uint32_t was being used to represent both ids and row indices. This is
not a problem for root tables (as the id always equals the index) but
for child tables, this is not the case and can lead to bad out of
bounds errors.
Reduce the confusion by introducing a new Id type for each table
heirarchy. The Id is defined is the root table and is used as the return
type of Insert in all child tables. This ensures that there can be no
conversion between row indices and ids without explicitly converting
between the two.
Change-Id: Idff36200af7335a71ce5f620cb41befc73b59475
diff --git a/src/trace_processor/db/typed_column.h b/src/trace_processor/db/typed_column.h
index 2913865..99355d7 100644
--- a/src/trace_processor/db/typed_column.h
+++ b/src/trace_processor/db/typed_column.h
@@ -23,7 +23,13 @@
namespace trace_processor {
// Represents a column containing ids.
+template <typename Id>
struct IdColumn : public Column {
+ Id operator[](uint32_t row) const { return Id(row_map().Get(row)); }
+ base::Optional<uint32_t> IndexOf(Id id) const {
+ return row_map().IndexOf(id.value);
+ }
+
// Helper functions to create constraints for the given value.
Constraint eq(uint32_t v) const { return eq_value(SqlValue::Long(v)); }
Constraint gt(uint32_t v) const { return gt_value(SqlValue::Long(v)); }
diff --git a/src/trace_processor/event_tracker.cc b/src/trace_processor/event_tracker.cc
index c269268..8ee221a 100644
--- a/src/trace_processor/event_tracker.cc
+++ b/src/trace_processor/event_tracker.cc
@@ -35,25 +35,25 @@
EventTracker::~EventTracker() = default;
-base::Optional<uint32_t> EventTracker::PushProcessCounterForThread(
+base::Optional<CounterId> EventTracker::PushProcessCounterForThread(
int64_t timestamp,
double value,
StringId name_id,
UniqueTid utid) {
- auto opt_row = PushCounter(timestamp, value, kInvalidTrackId);
- if (opt_row) {
+ auto opt_id = PushCounter(timestamp, value, kInvalidTrackId);
+ if (opt_id) {
PendingUpidResolutionCounter pending;
- pending.row = *opt_row;
+ pending.row = *context_->storage->counter_table().id().IndexOf(*opt_id);
pending.utid = utid;
pending.name_id = name_id;
pending_upid_resolution_counter_.emplace_back(pending);
}
- return opt_row;
+ return opt_id;
}
-base::Optional<uint32_t> EventTracker::PushCounter(int64_t timestamp,
- double value,
- TrackId track_id) {
+base::Optional<CounterId> EventTracker::PushCounter(int64_t timestamp,
+ double value,
+ TrackId track_id) {
if (timestamp < max_timestamp_) {
PERFETTO_DLOG("counter event (ts: %" PRId64
") out of order by %.4f ms, skipping",
@@ -64,7 +64,7 @@
max_timestamp_ = timestamp;
auto* counter_values = context_->storage->mutable_counter_table();
- return counter_values->Insert({timestamp, track_id, value});
+ return counter_values->Insert({timestamp, track_id.value, value});
}
uint32_t EventTracker::PushInstant(int64_t timestamp,
@@ -94,10 +94,10 @@
// TODO(lalitm): having upid == 0 is probably not the correct approach here
// but it's unclear what may be better.
UniquePid upid = thread.upid.value_or(0);
- auto id = context_->track_tracker->InternProcessCounterTrack(
+ TrackId id = context_->track_tracker->InternProcessCounterTrack(
pending_counter.name_id, upid);
context_->storage->mutable_counter_table()->mutable_track_id()->Set(
- pending_counter.row, id);
+ pending_counter.row, id.value);
}
for (const auto& pending_instant : pending_upid_resolution_instant_) {
diff --git a/src/trace_processor/event_tracker.h b/src/trace_processor/event_tracker.h
index ac719de..7e27807 100644
--- a/src/trace_processor/event_tracker.h
+++ b/src/trace_processor/event_tracker.h
@@ -39,9 +39,9 @@
// Adds a counter event to the counters table returning the index of the
// newly added row.
- virtual base::Optional<uint32_t> PushCounter(int64_t timestamp,
- double value,
- TrackId track_id);
+ virtual base::Optional<CounterId> PushCounter(int64_t timestamp,
+ double value,
+ TrackId track_id);
// Adds a counter event to the counters table for counter events which
// should be associated with a process but only have a thread context
@@ -49,7 +49,7 @@
//
// This function will resolve the utid to a upid when the events are
// flushed (see |FlushPendingEvents()|).
- virtual base::Optional<uint32_t> PushProcessCounterForThread(
+ virtual base::Optional<CounterId> PushProcessCounterForThread(
int64_t timestamp,
double value,
StringId name_id,
diff --git a/src/trace_processor/export_json.cc b/src/trace_processor/export_json.cc
index 07fe227..4e08f1e 100644
--- a/src/trace_processor/export_json.cc
+++ b/src/trace_processor/export_json.cc
@@ -487,8 +487,12 @@
// or chrome tracks (i.e. TrackEvent slices). Slices on other tracks may
// also be present as raw events and handled by trace_to_text. Only add more
// track types here if they are not already covered by trace_to_text.
- auto track_id = slices.track_id()[i];
- auto track_args_id = storage->track_table().source_arg_set_id()[track_id];
+ uint32_t track_id = slices.track_id()[i];
+
+ const auto& track_table = storage->track_table();
+
+ uint32_t track_row = *track_table.id().IndexOf(TrackId{track_id});
+ auto track_args_id = track_table.source_arg_set_id()[track_row];
if (!track_args_id)
continue;
const auto& track_args = args_builder.GetArgs(*track_args_id);
@@ -535,8 +539,7 @@
}
}
- auto opt_thread_track_row =
- thread_track.id().IndexOf(SqlValue::Long(track_id));
+ auto opt_thread_track_row = thread_track.id().IndexOf(TrackId{track_id});
if (opt_thread_track_row) {
// Synchronous (thread) slice or instant event.
@@ -584,8 +587,7 @@
} else if (!legacy_chrome_track ||
(legacy_chrome_track && track_args.isMember("source_id"))) {
// Async event slice.
- auto opt_process_row =
- process_track.id().IndexOf(SqlValue::Long(track_id));
+ auto opt_process_row = process_track.id().IndexOf(TrackId{track_id});
if (legacy_chrome_track) {
// Legacy async tracks are always process-associated.
PERFETTO_DCHECK(opt_process_row);
@@ -671,8 +673,7 @@
// Use "I" instead of "i" phase for backwards-compat with old consumers.
event["ph"] = "I";
- auto opt_process_row =
- process_track.id().IndexOf(SqlValue::Long(track_id));
+ auto opt_process_row = process_track.id().IndexOf(TrackId{track_id});
if (opt_process_row.has_value()) {
uint32_t upid = process_track.upid()[*opt_process_row];
event["pid"] = static_cast<int32_t>(storage->GetProcess(upid).pid);
diff --git a/src/trace_processor/export_json_unittest.cc b/src/trace_processor/export_json_unittest.cc
index 1512285..c3bfabc 100644
--- a/src/trace_processor/export_json_unittest.cc
+++ b/src/trace_processor/export_json_unittest.cc
@@ -120,7 +120,7 @@
StringId cat_id = context_.storage->InternString(base::StringView(kCategory));
StringId name_id = context_.storage->InternString(base::StringView(kName));
context_.storage->mutable_slice_table()->Insert(
- {kTimestamp, kDuration, track, cat_id, name_id, 0, 0, 0});
+ {kTimestamp, kDuration, track.value, cat_id, name_id, 0, 0, 0});
context_.storage->mutable_thread_slices()->AddThreadSlice(
0, kThreadTimestamp, kThreadDuration, kThreadInstructionCount,
kThreadInstructionDelta);
@@ -167,7 +167,7 @@
StringId cat_id = context_.storage->InternString(base::StringView(kCategory));
StringId name_id = context_.storage->InternString(base::StringView(kName));
context_.storage->mutable_slice_table()->Insert(
- {kTimestamp, kDuration, track, cat_id, name_id, 0, 0, 0});
+ {kTimestamp, kDuration, track.value, cat_id, name_id, 0, 0, 0});
context_.storage->mutable_thread_slices()->AddThreadSlice(
0, kThreadTimestamp, kThreadDuration, kThreadInstructionCount,
kThreadInstructionDelta);
@@ -229,7 +229,7 @@
StringId cat_id = context_.storage->InternString("cat");
StringId name_id = context_.storage->InternString("name");
context_.storage->mutable_slice_table()->Insert(
- {0, 0, track, cat_id, name_id, 0, 0, 0});
+ {0, 0, track.value, cat_id, name_id, 0, 0, 0});
base::TempFile temp_file = base::TempFile::Create();
FILE* output = fopen(temp_file.path().c_str(), "w+");
@@ -401,7 +401,7 @@
StringId cat_id = context_.storage->InternString(base::StringView(kCategory));
StringId name_id = context_.storage->InternString(base::StringView(kName));
context_.storage->mutable_slice_table()->Insert(
- {0, 0, track, cat_id, name_id, 0, 0, 0});
+ {0, 0, track.value, cat_id, name_id, 0, 0, 0});
StringId arg_key_id = context_.storage->InternString(
base::StringView("task.posted_from.file_name"));
@@ -445,8 +445,9 @@
context_.args_tracker->Flush(); // Flush track args.
StringId cat_id = storage->InternString(base::StringView(kCategory));
StringId name_id = storage->InternString(base::StringView(kName));
- uint32_t row = storage->mutable_slice_table()->Insert(
- {0, 0, track, cat_id, name_id, 0, 0, 0});
+ SliceId id = storage->mutable_slice_table()->Insert(
+ {0, 0, track.value, cat_id, name_id, 0, 0, 0});
+ uint32_t row = *storage->slice_table().id().IndexOf(id);
auto add_arg = [&](const char* key, Variadic value) {
StringId key_id = storage->InternString(key);
@@ -495,7 +496,7 @@
StringId cat_id = context_.storage->InternString(base::StringView(kCategory));
StringId name_id = context_.storage->InternString(base::StringView(kName));
context_.storage->mutable_slice_table()->Insert(
- {0, 0, track, cat_id, name_id, 0, 0, 0});
+ {0, 0, track.value, cat_id, name_id, 0, 0, 0});
StringId arg_flat_key_id = context_.storage->InternString(
base::StringView("debug.draw_duration_ms"));
@@ -545,7 +546,7 @@
StringId cat_id = context_.storage->InternString(base::StringView(kCategory));
StringId name_id = context_.storage->InternString(base::StringView(kName));
context_.storage->mutable_slice_table()->Insert(
- {0, 0, track, cat_id, name_id, 0, 0, 0});
+ {0, 0, track.value, cat_id, name_id, 0, 0, 0});
StringId arg_key0_id =
context_.storage->InternString(base::StringView("arg0"));
@@ -591,7 +592,7 @@
StringId cat_id = context_.storage->InternString(base::StringView(kCategory));
StringId name_id = context_.storage->InternString(base::StringView(kName));
context_.storage->mutable_slice_table()->Insert(
- {0, 0, track, cat_id, name_id, 0, 0, 0});
+ {0, 0, track.value, cat_id, name_id, 0, 0, 0});
StringId arg_flat_key_id =
context_.storage->InternString(base::StringView("a.b"));
@@ -640,7 +641,7 @@
StringId cat_id = context_.storage->InternString(base::StringView(kCategory));
StringId name_id = context_.storage->InternString(base::StringView(kName));
context_.storage->mutable_slice_table()->Insert(
- {0, 0, track, cat_id, name_id, 0, 0, 0});
+ {0, 0, track.value, cat_id, name_id, 0, 0, 0});
StringId arg_flat_key_id =
context_.storage->InternString(base::StringView("a"));
@@ -689,7 +690,7 @@
StringId cat_id = context_.storage->InternString(base::StringView(kCategory));
StringId name_id = context_.storage->InternString(base::StringView(kName));
context_.storage->mutable_slice_table()->Insert(
- {0, 0, track, cat_id, name_id, 0, 0, 0});
+ {0, 0, track.value, cat_id, name_id, 0, 0, 0});
StringId arg_key_id = context_.storage->InternString(base::StringView("a"));
StringId arg_value_id =
@@ -727,7 +728,7 @@
StringId cat_id = context_.storage->InternString(base::StringView(kCategory));
StringId name_id = context_.storage->InternString(base::StringView(kName));
context_.storage->mutable_slice_table()->Insert(
- {kTimestamp, 0, track, cat_id, name_id, 0, 0, 0});
+ {kTimestamp, 0, track.value, cat_id, name_id, 0, 0, 0});
base::TempFile temp_file = base::TempFile::Create();
FILE* output = fopen(temp_file.path().c_str(), "w+");
@@ -759,7 +760,7 @@
StringId cat_id = context_.storage->InternString(base::StringView(kCategory));
StringId name_id = context_.storage->InternString(base::StringView(kName));
context_.storage->mutable_slice_table()->Insert(
- {kTimestamp, 0, track, cat_id, name_id, 0, 0, 0});
+ {kTimestamp, 0, track.value, cat_id, name_id, 0, 0, 0});
base::TempFile temp_file = base::TempFile::Create();
FILE* output = fopen(temp_file.path().c_str(), "w+");
@@ -799,7 +800,7 @@
context_.args_tracker->Flush(); // Flush track args.
context_.storage->mutable_slice_table()->Insert(
- {kTimestamp, kDuration, track, cat_id, name_id, 0, 0, 0});
+ {kTimestamp, kDuration, track.value, cat_id, name_id, 0, 0, 0});
StringId arg_key_id =
context_.storage->InternString(base::StringView(kArgName));
TraceStorage::Args::Arg arg;
@@ -862,9 +863,9 @@
context_.args_tracker->Flush(); // Flush track args.
auto slice_id = context_.storage->mutable_slice_table()->Insert(
- {kTimestamp, kDuration, track, cat_id, name_id, 0, 0, 0});
+ {kTimestamp, kDuration, track.value, cat_id, name_id, 0, 0, 0});
context_.storage->mutable_virtual_track_slices()->AddVirtualTrackSlice(
- slice_id, kThreadTimestamp, kThreadDuration, 0, 0);
+ slice_id.value, kThreadTimestamp, kThreadDuration, 0, 0);
base::TempFile temp_file = base::TempFile::Create();
FILE* output = fopen(temp_file.path().c_str(), "w+");
@@ -917,9 +918,9 @@
context_.args_tracker->Flush(); // Flush track args.
auto slice_id = context_.storage->mutable_slice_table()->Insert(
- {kTimestamp, kDuration, track, cat_id, name_id, 0, 0, 0});
+ {kTimestamp, kDuration, track.value, cat_id, name_id, 0, 0, 0});
context_.storage->mutable_virtual_track_slices()->AddVirtualTrackSlice(
- slice_id, kThreadTimestamp, kThreadDuration, 0, 0);
+ slice_id.value, kThreadTimestamp, kThreadDuration, 0, 0);
base::TempFile temp_file = base::TempFile::Create();
FILE* output = fopen(temp_file.path().c_str(), "w+");
@@ -960,7 +961,7 @@
context_.args_tracker->Flush(); // Flush track args.
context_.storage->mutable_slice_table()->Insert(
- {kTimestamp, 0, track, cat_id, name_id, 0, 0, 0});
+ {kTimestamp, 0, track.value, cat_id, name_id, 0, 0, 0});
StringId arg_key_id =
context_.storage->InternString(base::StringView("arg_name"));
TraceStorage::Args::Arg arg;
@@ -1137,48 +1138,46 @@
UniqueTid utid = storage->AddEmptyThread(kThreadID);
storage->GetMutableThread(utid)->upid = upid;
- uint32_t module_row_id_1 =
- storage->mutable_stack_profile_mapping_table()->Insert(
- {storage->InternString("foo_module_id"), 0, 0, 0, 0, 0,
- storage->InternString("foo_module_name")});
+ auto module_id_1 = storage->mutable_stack_profile_mapping_table()->Insert(
+ {storage->InternString("foo_module_id"), 0, 0, 0, 0, 0,
+ storage->InternString("foo_module_name")});
- uint32_t module_row_id_2 =
- storage->mutable_stack_profile_mapping_table()->Insert(
- {storage->InternString("bar_module_id"), 0, 0, 0, 0, 0,
- storage->InternString("bar_module_name")});
+ auto module_id_2 = storage->mutable_stack_profile_mapping_table()->Insert(
+ {storage->InternString("bar_module_id"), 0, 0, 0, 0, 0,
+ storage->InternString("bar_module_name")});
// TODO(140860736): Once we support null values for
// stack_profile_frame.symbol_set_id remove this hack
storage->mutable_symbol_table()->Insert({0, 0, 0, 0});
uint32_t frame_row_id_1 = storage->mutable_stack_profile_frames()->Insert(
- {/*name_id=*/0, module_row_id_1, 0x42});
+ {/*name_id=*/0, module_id_1.value, 0x42});
uint32_t symbol_set_id = storage->symbol_table().row_count();
storage->mutable_symbol_table()->Insert(
{symbol_set_id, storage->InternString("foo_func"),
storage->InternString("foo_file"), 66});
- storage->mutable_stack_profile_frames()->SetSymbolSetId(
- static_cast<size_t>(frame_row_id_1), symbol_set_id);
+ storage->mutable_stack_profile_frames()->SetSymbolSetId(frame_row_id_1,
+ symbol_set_id);
uint32_t frame_row_id_2 = storage->mutable_stack_profile_frames()->Insert(
- {/*name_id=*/0, module_row_id_2, 0x4242});
+ {/*name_id=*/0, module_id_2.value, 0x4242});
symbol_set_id = storage->symbol_table().row_count();
storage->mutable_symbol_table()->Insert(
{symbol_set_id, storage->InternString("bar_func"),
storage->InternString("bar_file"), 77});
- storage->mutable_stack_profile_frames()->SetSymbolSetId(
- static_cast<size_t>(frame_row_id_2), symbol_set_id);
+ storage->mutable_stack_profile_frames()->SetSymbolSetId(frame_row_id_2,
+ symbol_set_id);
- uint32_t frame_callsite_id_1 =
+ auto frame_callsite_id_1 =
storage->mutable_stack_profile_callsite_table()->Insert(
{0, -1, frame_row_id_1});
- uint32_t frame_callsite_id_2 =
+ auto frame_callsite_id_2 =
storage->mutable_stack_profile_callsite_table()->Insert(
- {1, frame_callsite_id_1, frame_row_id_2});
+ {1, frame_callsite_id_1.value, frame_row_id_2});
storage->mutable_cpu_profile_stack_sample_table()->Insert(
- {kTimestamp, frame_callsite_id_2, utid});
+ {kTimestamp, frame_callsite_id_2.value, utid});
base::TempFile temp_file = base::TempFile::Create();
FILE* output = fopen(temp_file.path().c_str(), "w+");
@@ -1217,13 +1216,14 @@
StringId arg2_id = context_.storage->InternString(base::StringView("arg2"));
StringId val_id = context_.storage->InternString(base::StringView("val"));
- std::array<uint32_t, 3> slice_ids;
+ std::array<uint32_t, 3> slice_rows;
for (size_t i = 0; i < name_ids.size(); i++) {
- slice_ids[i] = context_.storage->mutable_slice_table()->Insert(
- {0, 0, track, cat_id, name_ids[i], 0, 0, 0});
+ auto slice_id = context_.storage->mutable_slice_table()->Insert(
+ {0, 0, track.value, cat_id, name_ids[i], 0, 0, 0});
+ slice_rows[i] = *context_.storage->slice_table().id().IndexOf(slice_id);
}
- for (uint32_t row : slice_ids) {
+ for (uint32_t row : slice_rows) {
context_.args_tracker->AddArg(TableId::kNestableSlices, row, arg1_id,
arg1_id, Variadic::Integer(5));
context_.args_tracker->AddArg(TableId::kNestableSlices, row, arg2_id,
@@ -1323,9 +1323,9 @@
StringId name_id = context_.storage->InternString(base::StringView(kName));
context_.storage->mutable_slice_table()->Insert(
- {kTimestamp1, kDuration, track, cat_id, name_id, 0, 0, 0});
+ {kTimestamp1, kDuration, track.value, cat_id, name_id, 0, 0, 0});
context_.storage->mutable_slice_table()->Insert(
- {kTimestamp2, kDuration, track, cat_id, name_id, 0, 0, 0});
+ {kTimestamp2, kDuration, track.value, cat_id, name_id, 0, 0, 0});
auto label_filter = [](const char* label_name) {
return strcmp(label_name, "traceEvents") == 0;
diff --git a/src/trace_processor/importers/fuchsia/fuchsia_trace_parser.cc b/src/trace_processor/importers/fuchsia/fuchsia_trace_parser.cc
index bd118378..21349b2 100644
--- a/src/trace_processor/importers/fuchsia/fuchsia_trace_parser.cc
+++ b/src/trace_processor/importers/fuchsia/fuchsia_trace_parser.cc
@@ -375,7 +375,7 @@
TrackId track_id = context_->track_tracker->InternFuchsiaAsyncTrack(
name, correlation_id);
uint32_t row = context_->event_tracker->PushInstant(
- ts, name, 0, track_id, RefType::kRefTrack);
+ ts, name, 0, track_id.value, RefType::kRefTrack);
for (const Arg& arg : args) {
context_->args_tracker->AddArg(
TableId::kInstants, row, arg.name, arg.name,
diff --git a/src/trace_processor/importers/proto/graphics_event_parser.cc b/src/trace_processor/importers/proto/graphics_event_parser.cc
index 240c4d9..4bba933 100644
--- a/src/trace_processor/importers/proto/graphics_event_parser.cc
+++ b/src/trace_processor/importers/proto/graphics_event_parser.cc
@@ -478,10 +478,10 @@
void GraphicsEventParser::UpdateVulkanMemoryAllocationCounters(
UniquePid upid,
const VulkanMemoryEvent::Decoder& event) {
- StringId track_id = kNullStringId;
- TrackId track = UINT32_MAX;
+ StringId track_str_id = kNullStringId;
+ TrackId track = kInvalidTrackId;
auto allocation_scope = VulkanMemoryEvent::SCOPE_UNSPECIFIED;
- uint32_t memory_type = UINT32_MAX;
+ uint32_t memory_type = std::numeric_limits<uint32_t>::max();
switch (event.source()) {
case VulkanMemoryEvent::SOURCE_DRIVER:
allocation_scope = static_cast<VulkanMemoryEvent::AllocationScope>(
@@ -501,10 +501,10 @@
case VulkanMemoryEvent::OP_ANNOTATIONS:
return;
}
- track_id = vulkan_memory_tracker_.FindAllocationScopeCounterString(
+ track_str_id = vulkan_memory_tracker_.FindAllocationScopeCounterString(
allocation_scope);
- track =
- context_->track_tracker->InternProcessCounterTrack(track_id, upid);
+ track = context_->track_tracker->InternProcessCounterTrack(track_str_id,
+ upid);
context_->event_tracker->PushCounter(
event.timestamp(), vulkan_driver_memory_counters_[allocation_scope],
track);
@@ -527,11 +527,11 @@
case VulkanMemoryEvent::OP_ANNOTATIONS:
return;
}
- track_id = vulkan_memory_tracker_.FindMemoryTypeCounterString(
+ track_str_id = vulkan_memory_tracker_.FindMemoryTypeCounterString(
memory_type,
VulkanMemoryTracker::DeviceCounterType::kAllocationCounter);
- track =
- context_->track_tracker->InternProcessCounterTrack(track_id, upid);
+ track = context_->track_tracker->InternProcessCounterTrack(track_str_id,
+ upid);
context_->event_tracker->PushCounter(
event.timestamp(),
vulkan_device_memory_counters_allocate_[memory_type], track);
@@ -555,10 +555,10 @@
case VulkanMemoryEvent::OP_ANNOTATIONS:
return;
}
- track_id = vulkan_memory_tracker_.FindMemoryTypeCounterString(
+ track_str_id = vulkan_memory_tracker_.FindMemoryTypeCounterString(
memory_type, VulkanMemoryTracker::DeviceCounterType::kBindCounter);
- track =
- context_->track_tracker->InternProcessCounterTrack(track_id, upid);
+ track = context_->track_tracker->InternProcessCounterTrack(track_str_id,
+ upid);
context_->event_tracker->PushCounter(
event.timestamp(), vulkan_device_memory_counters_bind_[memory_type],
track);
@@ -620,9 +620,9 @@
UpdateVulkanMemoryAllocationCounters(vulkan_memory_event_row.upid.value(),
vulkan_memory_event);
- uint32_t row =
- context_->storage->mutable_vulkan_memory_allocations_table()->Insert(
- vulkan_memory_event_row);
+ auto* allocs = context_->storage->mutable_vulkan_memory_allocations_table();
+ auto id = allocs->Insert(vulkan_memory_event_row);
+ uint32_t row = *allocs->id().IndexOf(id);
if (vulkan_memory_event.has_annotations()) {
ArgsTracker::BoundInserter inserter(context_->args_tracker.get(),
diff --git a/src/trace_processor/importers/proto/heap_graph_tracker.cc b/src/trace_processor/importers/proto/heap_graph_tracker.cc
index 94f10f9..2610871 100644
--- a/src/trace_processor/importers/proto/heap_graph_tracker.cc
+++ b/src/trace_processor/importers/proto/heap_graph_tracker.cc
@@ -187,15 +187,18 @@
}
}
+ auto* mapping_table =
+ context_->storage->mutable_stack_profile_mapping_table();
+
tables::StackProfileMappingTable::Row mapping_row{};
mapping_row.name = context_->storage->InternString("JAVA");
- uint32_t mapping_id =
- context_->storage->mutable_stack_profile_mapping_table()->Insert(
- mapping_row);
+ MappingId mapping_id = mapping_table->Insert(mapping_row);
+
+ uint32_t mapping_idx = *mapping_table->id().IndexOf(mapping_id);
auto paths = sequence_state.walker.FindPathsFromRoot();
for (const auto& p : paths.children)
- WriteFlamegraph(sequence_state, p.second, -1, 0, mapping_id);
+ WriteFlamegraph(sequence_state, p.second, -1, 0, mapping_idx);
sequence_state_.erase(seq_id);
}
@@ -205,16 +208,16 @@
const HeapGraphWalker::PathFromRoot& path,
int32_t parent_id,
uint32_t depth,
- uint32_t mapping_id) {
+ uint32_t mapping_row) {
TraceStorage::StackProfileFrames::Row row{};
row.name_id = StringId(static_cast<uint32_t>(path.class_name));
- row.mapping_row = mapping_id;
+ row.mapping_row = mapping_row;
int32_t frame_id = static_cast<int32_t>(
context_->storage->mutable_stack_profile_frames()->Insert(row));
- parent_id = static_cast<int32_t>(
- context_->storage->mutable_stack_profile_callsite_table()->Insert(
- {depth, parent_id, frame_id}));
+ auto* callsites = context_->storage->mutable_stack_profile_callsite_table();
+ auto callsite_id = callsites->Insert({depth, parent_id, frame_id});
+ parent_id = static_cast<int32_t>(callsite_id.value);
depth++;
tables::HeapProfileAllocationTable::Row alloc_row{
@@ -224,7 +227,7 @@
context_->storage->mutable_heap_profile_allocation_table()->Insert(alloc_row);
for (const auto& p : path.children) {
const HeapGraphWalker::PathFromRoot& child = p.second;
- WriteFlamegraph(sequence_state, child, parent_id, depth, mapping_id);
+ WriteFlamegraph(sequence_state, child, parent_id, depth, mapping_row);
}
}
diff --git a/src/trace_processor/importers/proto/heap_graph_tracker.h b/src/trace_processor/importers/proto/heap_graph_tracker.h
index 219059b..56c9e7f 100644
--- a/src/trace_processor/importers/proto/heap_graph_tracker.h
+++ b/src/trace_processor/importers/proto/heap_graph_tracker.h
@@ -108,7 +108,7 @@
const HeapGraphWalker::PathFromRoot& path,
int32_t parent_id,
uint32_t depth,
- uint32_t mapping_id);
+ uint32_t mapping_row);
TraceProcessorContext* const context_;
std::map<uint32_t, SequenceState> sequence_state_;
diff --git a/src/trace_processor/importers/proto/proto_trace_parser.cc b/src/trace_processor/importers/proto/proto_trace_parser.cc
index e30f703..773ced4 100644
--- a/src/trace_processor/importers/proto/proto_trace_parser.cc
+++ b/src/trace_processor/importers/proto/proto_trace_parser.cc
@@ -637,7 +637,7 @@
for (const int64_t frame_row : frame_rows) {
PERFETTO_DCHECK(frame_row >= 0);
context_->storage->mutable_stack_profile_frames()->SetSymbolSetId(
- static_cast<size_t>(frame_row), symbol_set_id);
+ static_cast<uint32_t>(frame_row), symbol_set_id);
frame_found = true;
}
}
diff --git a/src/trace_processor/importers/proto/proto_trace_parser_unittest.cc b/src/trace_processor/importers/proto/proto_trace_parser_unittest.cc
index febb560..6026684 100644
--- a/src/trace_processor/importers/proto/proto_trace_parser_unittest.cc
+++ b/src/trace_processor/importers/proto/proto_trace_parser_unittest.cc
@@ -126,9 +126,9 @@
int32_t next_prio));
MOCK_METHOD3(PushCounter,
- base::Optional<uint32_t>(int64_t timestamp,
- double value,
- uint32_t track_id));
+ base::Optional<CounterId>(int64_t timestamp,
+ double value,
+ TrackId track_id));
MOCK_METHOD6(PushInstant,
uint32_t(int64_t timestamp,
@@ -567,7 +567,7 @@
cpu_freq->set_cpu_id(10);
cpu_freq->set_state(2000);
- EXPECT_CALL(*event_, PushCounter(1000, DoubleEq(2000), 0));
+ EXPECT_CALL(*event_, PushCounter(1000, DoubleEq(2000), TrackId{0}));
Tokenize();
EXPECT_EQ(context_.storage->cpu_counter_track_table().cpu()[0], 10u);
@@ -584,7 +584,7 @@
meminfo->set_value(value);
EXPECT_CALL(*event_, PushCounter(static_cast<int64_t>(ts),
- DoubleEq(value * 1024.0), 0u));
+ DoubleEq(value * 1024.0), TrackId{0u}));
Tokenize();
EXPECT_EQ(context_.storage->track_table().row_count(), 1u);
@@ -600,8 +600,8 @@
uint32_t value = 10;
meminfo->set_value(value);
- EXPECT_CALL(*event_,
- PushCounter(static_cast<int64_t>(ts), DoubleEq(value), 0u));
+ EXPECT_CALL(*event_, PushCounter(static_cast<int64_t>(ts), DoubleEq(value),
+ TrackId{0u}));
Tokenize();
EXPECT_EQ(context_.storage->track_table().row_count(), 1u);
@@ -768,7 +768,7 @@
MockBoundInserter inserter;
- constexpr TrackId track = 0u;
+ constexpr TrackId track{0u};
InSequence in_sequence; // Below slices should be sorted by timestamp.
EXPECT_CALL(*slice_,
Scoped(1005000, track, kNullStringId, kNullStringId, 23000, _))
@@ -851,7 +851,7 @@
MockBoundInserter inserter;
- constexpr TrackId track = 0u;
+ constexpr TrackId track{0u};
InSequence in_sequence; // Below slices should be sorted by timestamp.
EXPECT_CALL(*slice_, Begin(1010000, track, kNullStringId, kNullStringId, _))
.WillOnce(DoAll(InvokeArgument<4>(&inserter), Return(0u)));
@@ -986,8 +986,8 @@
EXPECT_CALL(*storage_, GetThread(1))
.WillRepeatedly(testing::ReturnRef(thread));
- constexpr TrackId thread_1_track = 0u;
- constexpr TrackId process_2_track = 1u;
+ constexpr TrackId thread_1_track{0u};
+ constexpr TrackId process_2_track{1u};
EXPECT_CALL(*storage_, InternString(base::StringView("cat2,cat3")))
.WillOnce(Return(1));
@@ -1167,13 +1167,16 @@
InSequence in_sequence; // Below slices should be sorted by timestamp.
- EXPECT_CALL(*slice_, Begin(1010000, 1, StringId(1), StringId(2), _))
+ EXPECT_CALL(*slice_, Begin(1010000, TrackId{1}, StringId(1), StringId(2), _))
.WillOnce(Return(0u));
- EXPECT_CALL(*slice_, Scoped(1015000, 1, StringId(1), StringId(4), 0, _));
- EXPECT_CALL(*slice_, Scoped(1018000, 2, StringId(3), StringId(4), 0, _));
- EXPECT_CALL(*slice_, End(1020000, 1, StringId(1), StringId(2), _))
+ EXPECT_CALL(*slice_,
+ Scoped(1015000, TrackId{1}, StringId(1), StringId(4), 0, _));
+ EXPECT_CALL(*slice_,
+ Scoped(1018000, TrackId{2}, StringId(3), StringId(4), 0, _));
+ EXPECT_CALL(*slice_, End(1020000, TrackId{1}, StringId(1), StringId(2), _))
.WillOnce(Return(0u));
- EXPECT_CALL(*slice_, Scoped(1030000, 3, StringId(3), StringId(4), 0, _));
+ EXPECT_CALL(*slice_,
+ Scoped(1030000, TrackId{3}, StringId(3), StringId(4), 0, _));
context_.sorter->ExtractEventsForced();
@@ -1357,24 +1360,26 @@
.WillOnce(Return(1));
EXPECT_CALL(*storage_, InternString(base::StringView("ev1")))
.WillOnce(Return(2));
- EXPECT_CALL(*slice_, Begin(1010000, 1, StringId(1), StringId(2), _))
+ EXPECT_CALL(*slice_, Begin(1010000, TrackId{1}, StringId(1), StringId(2), _))
.WillOnce(Return(0u));
EXPECT_CALL(*storage_, InternString(base::StringView("cat2")))
.WillOnce(Return(3));
EXPECT_CALL(*storage_, InternString(base::StringView("ev2")))
.WillOnce(Return(4));
- EXPECT_CALL(*slice_, Scoped(1015000, 0, StringId(3), StringId(4), 0, _))
+ EXPECT_CALL(*slice_,
+ Scoped(1015000, TrackId{0}, StringId(3), StringId(4), 0, _))
.WillOnce(Return(1u));
EXPECT_CALL(*storage_, InternString(base::StringView("cat3")))
.WillOnce(Return(5));
EXPECT_CALL(*storage_, InternString(base::StringView("ev3")))
.WillOnce(Return(6));
- EXPECT_CALL(*slice_, Scoped(1016000, 2, StringId(5), StringId(6), 0, _))
+ EXPECT_CALL(*slice_,
+ Scoped(1016000, TrackId{2}, StringId(5), StringId(6), 0, _))
.WillOnce(Return(2u));
- EXPECT_CALL(*slice_, End(1020000, 1, StringId(0), StringId(0), _))
+ EXPECT_CALL(*slice_, End(1020000, TrackId{1}, StringId(0), StringId(0), _))
.WillOnce(Return(0u));
context_.sorter->ExtractEventsForced();
@@ -1557,7 +1562,7 @@
.Times(2)
.WillRepeatedly(testing::ReturnRef(thread));
- constexpr TrackId track = 0u;
+ constexpr TrackId track{0u};
InSequence in_sequence; // Below slices should be sorted by timestamp.
EXPECT_CALL(*slice_, Begin(1010000, track, kNullStringId, kNullStringId, _));
EXPECT_CALL(*slice_, End(2010000, track, kNullStringId, kNullStringId, _));
@@ -1678,8 +1683,8 @@
EXPECT_CALL(*storage_, InternString(base::StringView("ev1")))
.WillRepeatedly(Return(3));
- constexpr TrackId thread_2_track = 0u;
- constexpr TrackId thread_1_track = 1u;
+ constexpr TrackId thread_2_track{0u};
+ constexpr TrackId thread_1_track{1u};
InSequence in_sequence; // Below slices should be sorted by timestamp.
EXPECT_CALL(*slice_,
@@ -1863,7 +1868,7 @@
EXPECT_CALL(*storage_, GetString(StringId(4))).WillOnce(Return("debug.an2"));
- constexpr TrackId track = 0u;
+ constexpr TrackId track{0u};
InSequence in_sequence; // Below slices should be sorted by timestamp.
EXPECT_CALL(*slice_, Begin(1010000, track, StringId(1), StringId(2), _))
@@ -1949,7 +1954,7 @@
thread.upid = 1u;
EXPECT_CALL(*storage_, GetThread(1)).WillOnce(testing::ReturnRef(thread));
- constexpr TrackId track = 0u;
+ constexpr TrackId track{0u};
InSequence in_sequence; // Below slices should be sorted by timestamp.
MockBoundInserter inserter;
@@ -2028,7 +2033,7 @@
thread.upid = 1u;
EXPECT_CALL(*storage_, GetThread(1)).WillOnce(testing::ReturnRef(thread));
- constexpr TrackId track = 0;
+ constexpr TrackId track{0};
InSequence in_sequence; // Below slices should be sorted by timestamp.
EXPECT_CALL(*storage_, InternString(base::StringView("cat1")))
@@ -2207,7 +2212,7 @@
thread.upid = 1u;
EXPECT_CALL(*storage_, GetThread(1)).WillOnce(testing::ReturnRef(thread));
- constexpr TrackId track = 0u;
+ constexpr TrackId track{0u};
InSequence in_sequence; // Below slices should be sorted by timestamp.
// Timestamp should be adjusted to trace time (BOOTTIME).
diff --git a/src/trace_processor/importers/proto/track_event_parser.cc b/src/trace_processor/importers/proto/track_event_parser.cc
index 37d47a0..7dff534 100644
--- a/src/trace_processor/importers/proto/track_event_parser.cc
+++ b/src/trace_processor/importers/proto/track_event_parser.cc
@@ -325,15 +325,13 @@
track_id = *opt_track_id;
auto thread_track_row =
- context_->storage->thread_track_table().id().IndexOf(
- SqlValue::Long(track_id));
+ context_->storage->thread_track_table().id().IndexOf(track_id);
if (thread_track_row) {
utid = storage->thread_track_table().utid()[*thread_track_row];
upid = storage->GetThread(*utid).upid;
} else {
auto process_track_row =
- context_->storage->process_track_table().id().IndexOf(
- SqlValue::Long(track_id));
+ context_->storage->process_track_table().id().IndexOf(track_id);
if (process_track_row)
upid = storage->process_track_table().upid()[*process_track_row];
}
diff --git a/src/trace_processor/slice_tracker.cc b/src/trace_processor/slice_tracker.cc
index d1ddbb2..290c8d5 100644
--- a/src/trace_processor/slice_tracker.cc
+++ b/src/trace_processor/slice_tracker.cc
@@ -91,9 +91,10 @@
int64_t parent_stack_id =
depth == 0 ? 0 : slices->stack_id()[stack->back().first];
- tables::SliceTable::Row row(timestamp, duration, track_id, category, name,
- depth, 0, parent_stack_id);
- uint32_t slice_idx = slices->Insert(row);
+ tables::SliceTable::Row row(timestamp, duration, track_id.value, category,
+ name, depth, 0, parent_stack_id);
+ auto id = slices->Insert(row);
+ uint32_t slice_idx = *slices->id().IndexOf(id);
stack->emplace_back(std::make_pair(slice_idx, ArgsTracker(context_)));
if (args_callback) {
diff --git a/src/trace_processor/slice_tracker_unittest.cc b/src/trace_processor/slice_tracker_unittest.cc
index bfdd44f..a38709e 100644
--- a/src/trace_processor/slice_tracker_unittest.cc
+++ b/src/trace_processor/slice_tracker_unittest.cc
@@ -55,7 +55,7 @@
context.storage.reset(new TraceStorage());
SliceTracker tracker(&context);
- constexpr TrackId track = 22u;
+ constexpr TrackId track{22u};
tracker.Begin(2 /*ts*/, track, 0 /*cat*/, 1 /*name*/);
tracker.End(10 /*ts*/, track, 0 /*cat*/, 1 /*name*/);
@@ -63,7 +63,7 @@
EXPECT_EQ(slices.row_count(), 1u);
EXPECT_EQ(slices.ts()[0], 2);
EXPECT_EQ(slices.dur()[0], 8);
- EXPECT_EQ(slices.track_id()[0], track);
+ EXPECT_EQ(slices.track_id()[0], track.value);
EXPECT_EQ(slices.category()[0], 0u);
EXPECT_EQ(slices.name()[0], 1u);
EXPECT_EQ(slices.depth()[0], 0u);
@@ -75,7 +75,7 @@
context.storage.reset(new TraceStorage());
SliceTracker tracker(&context);
- constexpr TrackId track = 22u;
+ constexpr TrackId track{22u};
tracker.Begin(2 /*ts*/, track, 0 /*cat*/, 1 /*name*/,
[](ArgsTracker::BoundInserter* inserter) {
inserter->AddArg(/*flat_key=*/1, /*key=*/2,
@@ -91,7 +91,7 @@
EXPECT_EQ(slices.row_count(), 1u);
EXPECT_EQ(slices.ts()[0], 2);
EXPECT_EQ(slices.dur()[0], 8);
- EXPECT_EQ(slices.track_id()[0], track);
+ EXPECT_EQ(slices.track_id()[0], track.value);
EXPECT_EQ(slices.category()[0], 0u);
EXPECT_EQ(slices.name()[0], 1u);
EXPECT_EQ(slices.depth()[0], 0u);
@@ -113,7 +113,7 @@
context.storage.reset(new TraceStorage());
SliceTracker tracker(&context);
- constexpr TrackId track = 22u;
+ constexpr TrackId track{22u};
tracker.Begin(2 /*ts*/, track, 0 /*cat*/, 1 /*name*/);
tracker.Begin(3 /*ts*/, track, 0 /*cat*/, 2 /*name*/);
tracker.End(5 /*ts*/, track);
@@ -126,14 +126,14 @@
uint32_t idx = 0;
EXPECT_EQ(slices.ts()[idx], 2);
EXPECT_EQ(slices.dur()[idx], 8);
- EXPECT_EQ(slices.track_id()[idx], track);
+ EXPECT_EQ(slices.track_id()[idx], track.value);
EXPECT_EQ(slices.category()[idx], 0u);
EXPECT_EQ(slices.name()[idx], 1u);
EXPECT_EQ(slices.depth()[idx++], 0u);
EXPECT_EQ(slices.ts()[idx], 3);
EXPECT_EQ(slices.dur()[idx], 2);
- EXPECT_EQ(slices.track_id()[idx], track);
+ EXPECT_EQ(slices.track_id()[idx], track.value);
EXPECT_EQ(slices.category()[idx], 0u);
EXPECT_EQ(slices.name()[idx], 2u);
EXPECT_EQ(slices.depth()[idx], 1u);
@@ -148,7 +148,7 @@
context.storage.reset(new TraceStorage());
SliceTracker tracker(&context);
- constexpr TrackId track = 22u;
+ constexpr TrackId track{22u};
tracker.Begin(0 /*ts*/, track, 0, 0);
tracker.Begin(1 /*ts*/, track, 0, 0);
tracker.Scoped(2 /*ts*/, track, 0, 0, 6);
@@ -165,7 +165,7 @@
context.storage.reset(new TraceStorage());
SliceTracker tracker(&context);
- constexpr TrackId track = 22u;
+ constexpr TrackId track{22u};
tracker.Begin(2 /*ts*/, track, 5 /*cat*/, 1 /*name*/);
tracker.End(3 /*ts*/, track, 1 /*cat*/, 1 /*name*/);
tracker.End(4 /*ts*/, track, 0 /*cat*/, 2 /*name*/);
@@ -183,7 +183,7 @@
// Bug scenario: the second zero-length scoped slice prevents the first slice
// from being closed, leading to an inconsistency when we try to insert the
// final slice and it doesn't intersect with the still pending first slice.
- constexpr TrackId track = 22u;
+ constexpr TrackId track{22u};
tracker.Scoped(2 /*ts*/, track, 0 /*cat*/, 1 /*name*/, 10 /* dur */);
tracker.Scoped(2 /*ts*/, track, 0 /*cat*/, 1 /*name*/, 0 /* dur */);
tracker.Scoped(12 /*ts*/, track, 0 /*cat*/, 1 /*name*/, 1 /* dur */);
@@ -199,8 +199,8 @@
context.storage.reset(new TraceStorage());
SliceTracker tracker(&context);
- constexpr TrackId track_a = 22u;
- constexpr TrackId track_b = 23u;
+ constexpr TrackId track_a{22u};
+ constexpr TrackId track_b{23u};
tracker.Begin(0 /*ts*/, track_a, 0, 0);
tracker.Scoped(2 /*ts*/, track_b, 0, 0, 6);
tracker.Scoped(3 /*ts*/, track_b, 0, 0, 4);
@@ -211,9 +211,9 @@
EXPECT_THAT(slices,
ElementsAre(SliceInfo{0, 10}, SliceInfo{2, 6}, SliceInfo{3, 4}));
- EXPECT_EQ(context.storage->slice_table().track_id()[0], track_a);
- EXPECT_EQ(context.storage->slice_table().track_id()[1], track_b);
- EXPECT_EQ(context.storage->slice_table().track_id()[2], track_b);
+ EXPECT_EQ(context.storage->slice_table().track_id()[0], track_a.value);
+ EXPECT_EQ(context.storage->slice_table().track_id()[1], track_b.value);
+ EXPECT_EQ(context.storage->slice_table().track_id()[2], track_b.value);
EXPECT_EQ(context.storage->slice_table().depth()[0], 0u);
EXPECT_EQ(context.storage->slice_table().depth()[1], 0u);
EXPECT_EQ(context.storage->slice_table().depth()[2], 1u);
@@ -224,7 +224,7 @@
context.storage.reset(new TraceStorage());
SliceTracker tracker(&context);
- constexpr TrackId track = 22u;
+ constexpr TrackId track{22u};
tracker.Scoped(50 /*ts*/, track, 11 /*cat*/, 21 /*name*/, 100 /*dur*/);
tracker.Begin(100 /*ts*/, track, 12 /*cat*/, 22 /*name*/);
tracker.Scoped(450 /*ts*/, track, 12 /*cat*/, 22 /*name*/, 100 /*dur*/);
diff --git a/src/trace_processor/stack_profile_tracker.cc b/src/trace_processor/stack_profile_tracker.cc
index 0629541..d3ec998 100644
--- a/src/trace_processor/stack_profile_tracker.cc
+++ b/src/trace_processor/stack_profile_tracker.cc
@@ -108,10 +108,10 @@
}
}
if (cur_row == -1) {
- cur_row =
- context_->storage->mutable_stack_profile_mapping_table()->Insert(row);
- context_->storage->InsertMappingRow(row.name, row.build_id,
- static_cast<uint32_t>(cur_row));
+ MappingId mapping_id = mappings->Insert(row);
+ uint32_t mapping_row = *mappings->id().IndexOf(mapping_id);
+ context_->storage->InsertMappingRow(row.name, row.build_id, mapping_row);
+ cur_row = mapping_row;
}
mapping_idx_.emplace(row, cur_row);
}
@@ -206,9 +206,10 @@
if (callsite_it != callsite_idx_.end()) {
self_id = callsite_it->second;
} else {
- self_id =
- context_->storage->mutable_stack_profile_callsite_table()->Insert(
- row);
+ auto* callsite =
+ context_->storage->mutable_stack_profile_callsite_table();
+ auto callsite_id = callsite->Insert(row);
+ self_id = callsite_id.value;
callsite_idx_.emplace(row, self_id);
}
parent_id = self_id;
diff --git a/src/trace_processor/stack_profile_tracker.h b/src/trace_processor/stack_profile_tracker.h
index 99d99de..1a0769c 100644
--- a/src/trace_processor/stack_profile_tracker.h
+++ b/src/trace_processor/stack_profile_tracker.h
@@ -56,6 +56,8 @@
class TraceProcessorContext;
+// TODO(lalitm): Overhaul this class to make row vs id consistent and use
+// base::Optional instead of int64_t.
class StackProfileTracker {
public:
using SourceStringId = uint64_t;
diff --git a/src/trace_processor/syscall_tracker_unittest.cc b/src/trace_processor/syscall_tracker_unittest.cc
index 1d9f513..f7a2eef 100644
--- a/src/trace_processor/syscall_tracker_unittest.cc
+++ b/src/trace_processor/syscall_tracker_unittest.cc
@@ -63,7 +63,7 @@
};
TEST_F(SyscallTrackerTest, ReportUnknownSyscalls) {
- constexpr TrackId track = 0u;
+ constexpr TrackId track{0u};
StringId begin_name = 0;
StringId end_name = 0;
EXPECT_CALL(*slice_tracker, Begin(100, track, kNullStringId, _, _))
@@ -89,7 +89,7 @@
}
TEST_F(SyscallTrackerTest, Aarch64) {
- constexpr TrackId track = 0u;
+ constexpr TrackId track{0u};
StringId begin_name = 0;
StringId end_name = 0;
EXPECT_CALL(*slice_tracker, Begin(100, track, kNullStringId, _, _))
@@ -106,7 +106,7 @@
}
TEST_F(SyscallTrackerTest, x8664) {
- constexpr TrackId track = 0u;
+ constexpr TrackId track{0u};
StringId begin_name = 0;
StringId end_name = 0;
EXPECT_CALL(*slice_tracker, Begin(100, track, kNullStringId, _, _))
diff --git a/src/trace_processor/tables/macros_internal.h b/src/trace_processor/tables/macros_internal.h
index ff172e7..37c09f1 100644
--- a/src/trace_processor/tables/macros_internal.h
+++ b/src/trace_processor/tables/macros_internal.h
@@ -43,6 +43,30 @@
uint32_t Insert(const Row&) { PERFETTO_FATAL("Should not be called"); }
};
+// IdHelper is used to figure out the Id type for a table.
+//
+// We do this using templates with the following algorithm:
+// 1. If the parent class is anything but RootParentTable, the Id of the
+// table is the same as the Id of the parent.
+// 2. If the parent class is RootParentTable (i.e. the table is a root
+// table), then the Id is the one defined in the table itself.
+// The net result of this is that all tables in the hierarchy get the
+// same type of Id - the one defined in the root table of that hierarchy.
+//
+// Reasoning: We do this because using uint32_t is very overloaded and
+// having a wrapper type for ids is very helpful to avoid confusion with
+// row indices (especially because ids and row indices often appear in
+// similar places in the codebase - that is at insertion in parsers and
+// in trackers).
+template <typename ParentClass, typename Class>
+struct IdHelper {
+ using Id = typename ParentClass::Id;
+};
+template <typename Class>
+struct IdHelper<RootParentTable, Class> {
+ using Id = typename Class::DefinedId;
+};
+
// The parent class for all macro generated tables.
// This class is used to extract common code from the macro tables to reduce
// code size.
@@ -218,7 +242,35 @@
#define PERFETTO_TP_TABLE_INTERNAL(table_name, class_name, parent_class_name, \
DEF) \
class class_name : public macros_internal::MacroTable { \
+ private: \
+ /* \
+ * Allows IdHelper to access DefinedId for root tables. \
+ * Needs to be defined here to allow the public using declaration of Id \
+ * below to work correctly. \
+ */ \
+ friend struct macros_internal::IdHelper<parent_class_name, class_name>; \
+ \
+ /* \
+ * Defines a new id type for a heirarchy of tables. \
+ * We define it here as we need this type to be visible for the public \
+ * using declaration of Id below. \
+ * Note: This type will only used if this table is a root table. \
+ */ \
+ struct DefinedId { \
+ DefinedId() = default; \
+ explicit constexpr DefinedId(uint32_t v) : value(v) {} \
+ \
+ bool operator==(const DefinedId& o) const { return o.value == value; } \
+ \
+ uint32_t value; \
+ }; \
+ \
public: \
+ /* \
+ * This defines the type of the id to be the type of the root table of \
+ * the hierarchy - see IdHelper for more details. \
+ */ \
+ using Id = macros_internal::IdHelper<parent_class_name, class_name>::Id; \
struct Row : parent_class_name::Row { \
/* \
* Expands to Row(col_type1 col1_c, base::Optional<col_type2> col2_c, \
@@ -231,7 +283,8 @@
PERFETTO_TP_PARENT_ROW_CONSTRUCTOR) nullptr) { \
type_ = table_name; \
\
- /* Expands to \
+ /* \
+ * Expands to \
* col1 = col1_c; \
* col2 = col2_c; \
* ... \
@@ -243,7 +296,8 @@
return PERFETTO_TP_TABLE_COLUMNS(DEF, PERFETTO_TP_ROW_EQUALS) true; \
} \
\
- /* Expands to \
+ /* \
+ * Expands to \
* col_type1 col1 = {}; \
* base::Optional<col_type2> col2 = {}; \
* ... \
@@ -254,7 +308,8 @@
class_name(StringPool* pool, parent_class_name* parent) \
: macros_internal::MacroTable(table_name, pool, parent), \
parent_(parent) { \
- /* Expands to \
+ /* \
+ * Expands to \
* columns_.emplace_back("col1", col1_, Column::kNoFlag, this, \
* columns_.size(), row_maps_.size() - 1); \
* columns_.emplace_back("col2", col2_, Column::kNoFlag, this, \
@@ -264,17 +319,18 @@
PERFETTO_TP_TABLE_COLUMNS(DEF, PERFETTO_TP_TABLE_CONSTRUCTOR_COLUMN); \
} \
\
- uint32_t Insert(const Row& row) { \
- uint32_t id; \
+ Id Insert(const Row& row) { \
+ Id id; \
if (parent_ == nullptr) { \
- id = row_count(); \
+ id = Id{row_count()}; \
type_.Append(string_pool_->InternString(row.type())); \
} else { \
- id = parent_->Insert(row); \
+ id = Id{parent_->Insert(row)}; \
} \
UpdateRowMapsAfterParentInsert(); \
\
- /* Expands to \
+ /* \
+ * Expands to \
* col1_.Append(row.col1); \
* col2_.Append(row.col2); \
* ... \
@@ -283,8 +339,8 @@
return id; \
} \
\
- const IdColumn& id() const { \
- return static_cast<const IdColumn&>( \
+ const IdColumn<Id>& id() const { \
+ return static_cast<const IdColumn<Id>&>( \
columns_[static_cast<uint32_t>(ColumnIndex::id)]); \
} \
\
@@ -293,7 +349,8 @@
columns_[static_cast<uint32_t>(ColumnIndex::type)]); \
} \
\
- /* Expands to \
+ /* \
+ * Expands to \
* const TypedColumn<col1_type>& col1() { return col1_; } \
* TypedColumn<col1_type>* mutable_col1() { return &col1_; } \
* const TypedColumn<col2_type>& col2() { return col2_; } \
@@ -311,7 +368,8 @@
\
parent_class_name* parent_; \
\
- /* Expands to \
+ /* \
+ * Expands to \
* SparseVector<col1_type> col1_; \
* SparseVector<col2_type> col2_; \
* ... \
diff --git a/src/trace_processor/tables/macros_unittest.cc b/src/trace_processor/tables/macros_unittest.cc
index c7c7e82..9bf0b75 100644
--- a/src/trace_processor/tables/macros_unittest.cc
+++ b/src/trace_processor/tables/macros_unittest.cc
@@ -67,14 +67,14 @@
}
TEST_F(TableMacrosUnittest, InsertParent) {
- uint32_t id = event_.Insert(TestEventTable::Row(100, 0));
- ASSERT_EQ(id, 0u);
+ auto id = event_.Insert(TestEventTable::Row(100, 0));
+ ASSERT_EQ(id.value, 0u);
ASSERT_EQ(event_.type().GetString(0), "event");
ASSERT_EQ(event_.ts()[0], 100);
ASSERT_EQ(event_.arg_set_id()[0], 0);
id = slice_.Insert(TestSliceTable::Row(200, 123, 10, 0));
- ASSERT_EQ(id, 1u);
+ ASSERT_EQ(id.value, 1u);
ASSERT_EQ(event_.type().GetString(1), "slice");
ASSERT_EQ(event_.ts()[1], 200);
@@ -86,7 +86,7 @@
ASSERT_EQ(slice_.depth()[0], 0);
id = slice_.Insert(TestSliceTable::Row(210, 456, base::nullopt, 0));
- ASSERT_EQ(id, 2u);
+ ASSERT_EQ(id.value, 2u);
ASSERT_EQ(event_.type().GetString(2), "slice");
ASSERT_EQ(event_.ts()[2], 210);
@@ -103,9 +103,9 @@
slice_.Insert(TestSliceTable::Row(200, 123, 10, 0));
auto reason = pool_.InternString("R");
- uint32_t id = cpu_slice_.Insert(
+ auto id = cpu_slice_.Insert(
TestCpuSliceTable::Row(205, 456, 5, 1, 4, 1024, reason));
- ASSERT_EQ(id, 2u);
+ ASSERT_EQ(id.value, 2u);
ASSERT_EQ(event_.type().GetString(2), "cpu_slice");
ASSERT_EQ(event_.ts()[2], 205);
ASSERT_EQ(event_.arg_set_id()[2], 456);
diff --git a/src/trace_processor/trace_storage.h b/src/trace_processor/trace_storage.h
index 37422cf..f12069d 100644
--- a/src/trace_processor/trace_storage.h
+++ b/src/trace_processor/trace_storage.h
@@ -74,11 +74,18 @@
using ArgSetId = uint32_t;
static const ArgSetId kInvalidArgSetId = 0;
-using TrackId = uint32_t;
+using TrackId = tables::TrackTable::Id;
+
+using CounterId = tables::CounterTable::Id;
+
+using SliceId = tables::SliceTable::Id;
+
+using MappingId = tables::StackProfileMappingTable::Id;
// TODO(lalitm): this is a temporary hack while migrating the counters table and
// will be removed when the migration is complete.
-static const TrackId kInvalidTrackId = std::numeric_limits<TrackId>::max();
+static const TrackId kInvalidTrackId =
+ TrackId(std::numeric_limits<TrackId>::max());
enum class RefType {
kRefNoRef = 0,
@@ -676,7 +683,7 @@
return it->second;
}
- void SetSymbolSetId(size_t row_idx, uint32_t symbol_set_id) {
+ void SetSymbolSetId(uint32_t row_idx, uint32_t symbol_set_id) {
PERFETTO_CHECK(row_idx < symbol_set_ids_.size());
symbol_set_ids_[row_idx] = symbol_set_id;
}
@@ -1162,6 +1169,16 @@
namespace std {
template <>
+struct hash<::perfetto::trace_processor::TrackId> {
+ using argument_type = ::perfetto::trace_processor::TrackId;
+ using result_type = size_t;
+
+ result_type operator()(const argument_type& r) const {
+ return std::hash<uint32_t>{}(r.value);
+ }
+};
+
+template <>
struct hash<
::perfetto::trace_processor::TraceStorage::StackProfileFrames::Row> {
using argument_type =
diff --git a/src/trace_processor/track_tracker.cc b/src/trace_processor/track_tracker.cc
index 1044101..944e674 100644
--- a/src/trace_processor/track_tracker.cc
+++ b/src/trace_processor/track_tracker.cc
@@ -22,7 +22,7 @@
namespace trace_processor {
// static
-constexpr TrackId TrackTracker::kDefaultDescriptorTrackUuid;
+constexpr uint64_t TrackTracker::kDefaultDescriptorTrackUuid;
TrackTracker::TrackTracker(TraceProcessorContext* context)
: source_key_(context->storage->InternString("source")),
@@ -61,7 +61,7 @@
fuchsia_async_tracks_[correlation_id] = id;
ArgsTracker::BoundInserter inserter(context_->args_tracker.get(),
- TableId::kTrack, id);
+ TableId::kTrack, id.value);
inserter.AddArg(source_key_, Variadic::String(fuchsia_source_));
inserter.AddArg(source_id_key_, Variadic::Integer(correlation_id));
return id;
@@ -103,7 +103,7 @@
chrome_tracks_[tuple] = id;
ArgsTracker::BoundInserter inserter(context_->args_tracker.get(),
- TableId::kTrack, id);
+ TableId::kTrack, id.value);
inserter.AddArg(source_key_, Variadic::String(chrome_source_));
inserter.AddArg(source_id_key_, Variadic::Integer(source_id));
inserter.AddArg(source_id_is_process_scoped_key_,
@@ -127,7 +127,7 @@
android_async_tracks_[tuple] = id;
ArgsTracker::BoundInserter inserter(context_->args_tracker.get(),
- TableId::kTrack, id);
+ TableId::kTrack, id.value);
inserter.AddArg(source_key_, Variadic::String(android_source_));
inserter.AddArg(source_id_key_, Variadic::Integer(cookie));
return id;
@@ -143,8 +143,8 @@
auto id = context_->storage->mutable_process_track_table()->Insert(row);
chrome_process_instant_tracks_[upid] = id;
- context_->args_tracker->AddArg(TableId::kTrack, id, source_key_, source_key_,
- Variadic::String(chrome_source_));
+ context_->args_tracker->AddArg(TableId::kTrack, id.value, source_key_,
+ source_key_, Variadic::String(chrome_source_));
return id;
}
@@ -154,7 +154,7 @@
context_->storage->mutable_track_table()->Insert({});
context_->args_tracker->AddArg(
- TableId::kTrack, *chrome_global_instant_track_id_, source_key_,
+ TableId::kTrack, chrome_global_instant_track_id_->value, source_key_,
source_key_, Variadic::String(chrome_source_));
}
return *chrome_global_instant_track_id_;
@@ -169,16 +169,16 @@
// Update existing track for |uuid|.
TrackId track_id = it->second;
if (name != kNullStringId) {
- context_->storage->mutable_track_table()->mutable_name()->Set(track_id,
- name);
+ auto* track = context_->storage->mutable_track_table();
+ auto row = *track->id().IndexOf(track_id);
+ track->mutable_name()->Set(row, name);
}
#if PERFETTO_DLOG_IS_ON()
if (upid) {
// Verify that upid didn't change.
auto process_track_row =
- context_->storage->process_track_table().id().IndexOf(
- SqlValue::Long(track_id));
+ context_->storage->process_track_table().id().IndexOf(track_id);
if (!process_track_row) {
PERFETTO_DLOG("Can't update non-scoped track with uuid %" PRIu64
" to a scoped track.",
@@ -197,8 +197,7 @@
if (utid) {
// Verify that utid didn't change.
auto thread_track_row =
- context_->storage->thread_track_table().id().IndexOf(
- SqlValue::Long(track_id));
+ context_->storage->thread_track_table().id().IndexOf(track_id);
if (!thread_track_row) {
PERFETTO_DLOG("Can't update non-thread track with uuid %" PRIu64
" to a thread track.",
@@ -236,8 +235,8 @@
if (descriptor_it == descriptor_tracks_.end()) {
descriptor_tracks_[uuid] = candidate_track_id;
context_->args_tracker->AddArg(
- TableId::kTrack, candidate_track_id, source_id_key_, source_id_key_,
- Variadic::Integer(static_cast<int64_t>(uuid)));
+ TableId::kTrack, candidate_track_id.value, source_id_key_,
+ source_id_key_, Variadic::Integer(static_cast<int64_t>(uuid)));
return candidate_track_id;
}
@@ -265,7 +264,7 @@
descriptor_tracks_[uuid] = track_id;
ArgsTracker::BoundInserter inserter(context_->args_tracker.get(),
- TableId::kTrack, track_id);
+ TableId::kTrack, track_id.value);
inserter.AddArg(source_key_, Variadic::String(descriptor_source_));
inserter.AddArg(source_id_key_,
Variadic::Integer(static_cast<int64_t>(uuid)));
@@ -291,7 +290,7 @@
context_->storage->mutable_thread_track_table()->Insert(row);
descriptor_tracks_by_utid_[utid] = track_id;
- context_->args_tracker->AddArg(TableId::kTrack, track_id, source_key_,
+ context_->args_tracker->AddArg(TableId::kTrack, track_id.value, source_key_,
source_key_,
Variadic::String(descriptor_source_));
return track_id;
diff --git a/src/trace_processor/track_tracker.h b/src/trace_processor/track_tracker.h
index 8ef379a..3a2b0ba 100644
--- a/src/trace_processor/track_tracker.h
+++ b/src/trace_processor/track_tracker.h
@@ -136,7 +136,7 @@
}
};
- static constexpr TrackId kDefaultDescriptorTrackUuid = 0u;
+ static constexpr uint64_t kDefaultDescriptorTrackUuid = 0u;
std::map<UniqueTid /* utid */, TrackId> thread_tracks_;
std::map<int64_t /* correlation_id */, TrackId> fuchsia_async_tracks_;
