Merge "trace_processor: add copy/move constructors to potentially fix compile"
diff --git a/include/perfetto/base/compiler.h b/include/perfetto/base/compiler.h
index 0b2efbf..0f08b42 100644
--- a/include/perfetto/base/compiler.h
+++ b/include/perfetto/base/compiler.h
@@ -38,25 +38,6 @@
#define PERFETTO_NO_INLINE
#endif
-// TODO(lalitm): is_trivially_constructible is currently not available
-// in some environments we build in. Reenable when that environment supports
-// this.
-#if defined(__GLIBCXX__)
-#define PERFETTO_IS_TRIVIALLY_CONSTRUCTIBLE(T) true
-#else
-#define PERFETTO_IS_TRIVIALLY_CONSTRUCTIBLE(T) \
- std::is_trivially_constructible<T>::value
-#endif
-
-// TODO(lalitm): is_trivially_copyable is currently not available
-// in some environments we build in. Reenable when that environment supports
-// this.
-#if defined(__GLIBCXX__)
-#define PERFETTO_IS_TRIVIALLY_COPYABLE(T) true
-#else
-#define PERFETTO_IS_TRIVIALLY_COPYABLE(T) std::is_trivially_copyable<T>::value
-#endif
-
#if defined(__GNUC__) || defined(__clang__)
#define PERFETTO_DEBUG_FUNCTION_IDENTIFIER() __PRETTY_FUNCTION__
#elif defined(_MSC_VER)
diff --git a/include/perfetto/protozero/proto_decoder.h b/include/perfetto/protozero/proto_decoder.h
index 5794cc9..b58e64e 100644
--- a/include/perfetto/protozero/proto_decoder.h
+++ b/include/perfetto/protozero/proto_decoder.h
@@ -332,7 +332,7 @@
// implicit initializers on all the ~1000 entries. We need it to initialize
// only on the first |max_field_id| fields, the remaining capacity doesn't
// require initialization.
- static_assert(PERFETTO_IS_TRIVIALLY_CONSTRUCTIBLE(Field) &&
+ static_assert(std::is_trivially_constructible<Field>::value &&
std::is_trivially_destructible<Field>::value &&
std::is_trivial<Field>::value,
"Field must be a trivial aggregate type");
diff --git a/protos/perfetto/trace/perfetto_trace.proto b/protos/perfetto/trace/perfetto_trace.proto
index 5cd950a..5b5d5b7 100644
--- a/protos/perfetto/trace/perfetto_trace.proto
+++ b/protos/perfetto/trace/perfetto_trace.proto
@@ -3049,7 +3049,7 @@
ROOT_JNI_MONITOR = 14;
};
// Objects retained by this root.
- repeated uint64 object_ids = 1;
+ repeated uint64 object_ids = 1 [packed = true];
optional Type root_type = 2;
}
@@ -3065,10 +3065,10 @@
// Indices for InternedData.field_names for the name of the field referring
// to the object.
- repeated uint64 reference_field_id = 4;
+ repeated uint64 reference_field_id = 4 [packed = true];
// Ids of the Object that is referred to.
- repeated uint64 reference_object_id = 5;
+ repeated uint64 reference_object_id = 5 [packed = true];
}
message HeapGraph {
diff --git a/protos/perfetto/trace/profiling/heap_graph.proto b/protos/perfetto/trace/profiling/heap_graph.proto
index d29c292..7e2d5d4 100644
--- a/protos/perfetto/trace/profiling/heap_graph.proto
+++ b/protos/perfetto/trace/profiling/heap_graph.proto
@@ -43,7 +43,7 @@
ROOT_JNI_MONITOR = 14;
};
// Objects retained by this root.
- repeated uint64 object_ids = 1;
+ repeated uint64 object_ids = 1 [packed = true];
optional Type root_type = 2;
}
@@ -59,10 +59,10 @@
// Indices for InternedData.field_names for the name of the field referring
// to the object.
- repeated uint64 reference_field_id = 4;
+ repeated uint64 reference_field_id = 4 [packed = true];
// Ids of the Object that is referred to.
- repeated uint64 reference_object_id = 5;
+ repeated uint64 reference_object_id = 5 [packed = true];
}
message HeapGraph {
diff --git a/src/protozero/proto_decoder.cc b/src/protozero/proto_decoder.cc
index bd563fb..a5f1b95 100644
--- a/src/protozero/proto_decoder.cc
+++ b/src/protozero/proto_decoder.cc
@@ -208,7 +208,7 @@
PERFETTO_CHECK(new_capacity > size_);
std::unique_ptr<Field[]> new_storage(new Field[new_capacity]);
- static_assert(PERFETTO_IS_TRIVIALLY_COPYABLE(Field),
+ static_assert(std::is_trivially_copyable<Field>::value,
"Field must be trivially copyable");
memcpy(&new_storage[0], fields_, sizeof(Field) * size_);
diff --git a/src/trace_processor/importers/proto/heap_graph_module.cc b/src/trace_processor/importers/proto/heap_graph_module.cc
index 50bba45..7216d90 100644
--- a/src/trace_processor/importers/proto/heap_graph_module.cc
+++ b/src/trace_processor/importers/proto/heap_graph_module.cc
@@ -65,6 +65,28 @@
}
}
+// Iterate over a repeated field of varints, independent of whether it is
+// packed or not.
+template <int32_t field_no, typename T, typename F>
+bool ForEachVarInt(const T& decoder, F fn) {
+ auto field = decoder.template at<field_no>();
+ bool parse_error = false;
+ if (field.type() == protozero::proto_utils::ProtoWireType::kLengthDelimited) {
+ // packed repeated
+ auto it = decoder.template GetPackedRepeated<
+ ::protozero::proto_utils::ProtoWireType::kVarInt, uint64_t>(
+ field_no, &parse_error);
+ for (; it; ++it)
+ fn(*it);
+ } else {
+ // non-packed repeated
+ auto it = decoder.template GetRepeated<uint64_t>(field_no);
+ for (; it; ++it)
+ fn(*it);
+ }
+ return parse_error;
+}
+
} // namespace
void HeapGraphModule::ParseHeapGraph(int64_t ts, protozero::ConstBytes blob) {
@@ -78,21 +100,37 @@
obj.object_id = object.id();
obj.self_size = object.self_size();
obj.type_id = object.type_id();
- auto ref_field_ids_it = object.reference_field_id();
- auto ref_object_ids_it = object.reference_object_id();
- for (; ref_field_ids_it && ref_object_ids_it;
- ++ref_field_ids_it, ++ref_object_ids_it) {
- HeapGraphTracker::SourceObject::Reference ref;
- ref.field_name_id = *ref_field_ids_it;
- ref.owned_object_id = *ref_object_ids_it;
- obj.references.emplace_back(std::move(ref));
+
+ std::vector<uint64_t> field_ids;
+ std::vector<uint64_t> object_ids;
+
+ bool parse_error = ForEachVarInt<
+ protos::pbzero::HeapGraphObject::kReferenceFieldIdFieldNumber>(
+ object, [&field_ids](uint64_t value) { field_ids.push_back(value); });
+
+ if (!parse_error) {
+ parse_error = ForEachVarInt<
+ protos::pbzero::HeapGraphObject::kReferenceObjectIdFieldNumber>(
+ object,
+ [&object_ids](uint64_t value) { object_ids.push_back(value); });
}
- if (ref_field_ids_it || ref_object_ids_it) {
- context_->storage->IncrementIndexedStats(stats::heap_graph_missing_packet,
- static_cast<int>(upid));
+ if (parse_error) {
+ context_->storage->IncrementIndexedStats(
+ stats::heap_graph_malformed_packet, static_cast<int>(upid));
+ break;
+ }
+ if (field_ids.size() != object_ids.size()) {
+ context_->storage->IncrementIndexedStats(
+ stats::heap_graph_malformed_packet, static_cast<int>(upid));
continue;
}
+ for (size_t i = 0; i < field_ids.size(); ++i) {
+ HeapGraphTracker::SourceObject::Reference ref;
+ ref.field_name_id = field_ids[i];
+ ref.owned_object_id = object_ids[i];
+ obj.references.emplace_back(std::move(ref));
+ }
context_->heap_graph_tracker->AddObject(upid, ts, std::move(obj));
}
for (auto it = heap_graph.type_names(); it; ++it) {
@@ -118,8 +156,16 @@
HeapGraphTracker::SourceRoot src_root;
src_root.root_type = context_->storage->InternString(str_view);
- for (auto obj_it = entry.object_ids(); obj_it; ++obj_it)
- src_root.object_ids.emplace_back(*obj_it);
+ bool parse_error =
+ ForEachVarInt<protos::pbzero::HeapGraphRoot::kObjectIdsFieldNumber>(
+ entry, [&src_root](uint64_t value) {
+ src_root.object_ids.emplace_back(value);
+ });
+ if (parse_error) {
+ context_->storage->IncrementIndexedStats(
+ stats::heap_graph_malformed_packet, static_cast<int>(upid));
+ break;
+ }
context_->heap_graph_tracker->AddRoot(upid, ts, std::move(src_root));
}
if (!heap_graph.continued()) {
diff --git a/src/trace_processor/importers/proto/system_probes_parser.cc b/src/trace_processor/importers/proto/system_probes_parser.cc
index 14afab1..e93578c 100644
--- a/src/trace_processor/importers/proto/system_probes_parser.cc
+++ b/src/trace_processor/importers/proto/system_probes_parser.cc
@@ -212,7 +212,12 @@
} else {
auto args = proc.cmdline();
base::StringView argv0 = args ? *args : base::StringView();
- context_->process_tracker->SetProcessMetadata(pid, ppid, argv0);
+ UniquePid upid =
+ context_->process_tracker->SetProcessMetadata(pid, ppid, argv0);
+ if (proc.has_uid()) {
+ context_->process_tracker->SetProcessUid(
+ upid, static_cast<uint32_t>(proc.uid()));
+ }
}
}
diff --git a/src/trace_processor/process_table.cc b/src/trace_processor/process_table.cc
index adc9d1e..65d29f9 100644
--- a/src/trace_processor/process_table.cc
+++ b/src/trace_processor/process_table.cc
@@ -47,6 +47,7 @@
SqliteTable::Column(Column::kEndTs, "end_ts", SqlValue::Type::kLong),
SqliteTable::Column(Column::kParentUpid, "parent_upid",
SqlValue::Type::kLong),
+ SqliteTable::Column(Column::kUid, "uid", SqlValue::Type::kLong),
},
{Column::kUpid});
return util::OkStatus();
@@ -147,6 +148,14 @@
}
break;
}
+ case Column::kUid: {
+ if (process.uid.has_value()) {
+ sqlite3_result_int64(context, process.uid.value());
+ } else {
+ sqlite3_result_null(context);
+ }
+ break;
+ }
default:
PERFETTO_FATAL("Unknown column %d", N);
break;
diff --git a/src/trace_processor/process_table.h b/src/trace_processor/process_table.h
index c4c1ed5..74b0386 100644
--- a/src/trace_processor/process_table.h
+++ b/src/trace_processor/process_table.h
@@ -36,7 +36,8 @@
kPid = 2,
kStartTs = 3,
kEndTs = 4,
- kParentUpid = 5
+ kParentUpid = 5,
+ kUid = 6
};
class Cursor : public SqliteTable::Cursor {
public:
diff --git a/src/trace_processor/process_tracker.cc b/src/trace_processor/process_tracker.cc
index e9c0335..1ccc4b7 100644
--- a/src/trace_processor/process_tracker.cc
+++ b/src/trace_processor/process_tracker.cc
@@ -194,6 +194,10 @@
return upid;
}
+void ProcessTracker::SetProcessUid(UniquePid upid, uint32_t uid) {
+ context_->storage->GetMutableProcess(upid)->uid = uid;
+}
+
void ProcessTracker::SetProcessNameIfUnset(UniquePid upid,
StringId process_name_id) {
TraceStorage::Process* process = context_->storage->GetMutableProcess(upid);
diff --git a/src/trace_processor/process_tracker.h b/src/trace_processor/process_tracker.h
index 7c7ea55..985212b 100644
--- a/src/trace_processor/process_tracker.h
+++ b/src/trace_processor/process_tracker.h
@@ -91,6 +91,9 @@
base::Optional<uint32_t> ppid,
base::StringView name);
+ // Sets the process user id.
+ void SetProcessUid(UniquePid upid, uint32_t uid);
+
// Assigns the given name to the process identified by |upid| if it does not
// have a name yet.
void SetProcessNameIfUnset(UniquePid upid, StringId process_name_id);
diff --git a/src/trace_processor/span_join_operator_table.cc b/src/trace_processor/span_join_operator_table.cc
index 94c8ac1..24077d7 100644
--- a/src/trace_processor/span_join_operator_table.cc
+++ b/src/trace_processor/span_join_operator_table.cc
@@ -193,8 +193,7 @@
continue;
if (col_name == kTsColumnName || col_name == kDurColumnName) {
- // We don't support constraints on ts or duration in the child tables.
- PERFETTO_DFATAL("ts or duration constraints on child tables");
+ // Allow SQLite handle any constraints on ts or duration.
continue;
}
auto op = sqlite_utils::OpToString(cs.op);
diff --git a/src/trace_processor/trace_storage.h b/src/trace_processor/trace_storage.h
index 7243b42..2d40985 100644
--- a/src/trace_processor/trace_storage.h
+++ b/src/trace_processor/trace_storage.h
@@ -115,6 +115,7 @@
StringId name_id = 0;
uint32_t pid = 0;
base::Optional<UniquePid> parent_upid;
+ base::Optional<uint32_t> uid;
};
// Information about a unique thread seen in a trace.
diff --git a/test/synth_common.py b/test/synth_common.py
index 3383b81..142e76e 100644
--- a/test/synth_common.py
+++ b/test/synth_common.py
@@ -173,11 +173,13 @@
if ts is not None:
self.packet.timestamp = ts
- def add_process(self, pid, ppid, cmdline):
+ def add_process(self, pid, ppid, cmdline, uid=None):
process = self.packet.process_tree.processes.add()
process.pid = pid
process.ppid = ppid
process.cmdline.append(cmdline)
+ if uid is not None:
+ process.uid = uid
self.proc_map[pid] = cmdline
def add_thread(self, tid, tgid, cmdline):
diff --git a/test/trace_processor/index b/test/trace_processor/index
index 958b11e..808b2cf 100644
--- a/test/trace_processor/index
+++ b/test/trace_processor/index
@@ -12,6 +12,7 @@
# Test for the process<>thread tracking logic.
synth_process_tracking.py process_tracking.sql process_tracking.out
+synth_process_tracking.py process_tracking_uid.sql process_tracking_uid.out
process_tracking_short_lived_1.py process_tracking.sql process_tracking_process_tracking_short_lived_1.out
process_tracking_short_lived_2.py process_tracking.sql process_tracking_process_tracking_short_lived_2.out
process_tracking_exec.py process_tracking.sql process_tracking_process_tracking_exec.out
diff --git a/test/trace_processor/process_tracking_uid.out b/test/trace_processor/process_tracking_uid.out
new file mode 100644
index 0000000..ae90e8d
--- /dev/null
+++ b/test/trace_processor/process_tracking_uid.out
@@ -0,0 +1,6 @@
+"pid","uid"
+0,"[NULL]"
+10,1001
+20,1002
+30,"[NULL]"
+40,"[NULL]"
diff --git a/test/trace_processor/process_tracking_uid.sql b/test/trace_processor/process_tracking_uid.sql
new file mode 100644
index 0000000..de06399
--- /dev/null
+++ b/test/trace_processor/process_tracking_uid.sql
@@ -0,0 +1,3 @@
+select pid, uid
+from process
+order by pid;
diff --git a/test/trace_processor/synth_process_tracking.py b/test/trace_processor/synth_process_tracking.py
index 6686d89..a7a4d2d 100644
--- a/test/trace_processor/synth_process_tracking.py
+++ b/test/trace_processor/synth_process_tracking.py
@@ -40,7 +40,7 @@
# SQL level we should be able to tell that p1-t0 and p1-t2 belong to 'process1'
# but p1-t1 should be left unjoinable.
trace.add_process_tree_packet(ts=5)
-trace.add_process(10, 0, "process1")
+trace.add_process(10, 0, "process1", 1001)
trace.add_thread(12, 10, "p1-t2")
# Now create another process (pid=20) with three threads(tids=20,21,22).
@@ -58,7 +58,7 @@
# From the process tracker viewpoint we pretend we only scraped tids=20,21.
trace.add_process_tree_packet(ts=15)
-trace.add_process(20, 0, "process_2")
+trace.add_process(20, 0, "process_2", 1002)
trace.add_thread(21, 20, "p2-t1")
# Finally the very complex case: a third process (pid=30) which spawns threads
