Merge "Separate the actual trace to pprof conversion"
diff --git a/BUILD b/BUILD
index aca32e3..c5bdbca 100644
--- a/BUILD
+++ b/BUILD
@@ -115,8 +115,8 @@
"src/protozero/protoc_plugin/protozero_plugin.cc",
],
deps = [
- "//third_party/protobuf",
- "//third_party/protobuf:libprotoc",
+ "//third_party/protobuf:libprotoc_legacy",
+ "//third_party/protobuf:protobuf_legacy",
],
)
@@ -671,8 +671,8 @@
"//third_party/perfetto/protos:trace_sys_stats_zero_cc_proto",
"//third_party/perfetto/protos:trace_track_event_zero_cc_proto",
"//third_party/perfetto/protos:trace_zero_cc_proto",
- "//third_party/protobuf",
- "//third_party/protobuf:libprotoc",
+ "//third_party/protobuf:libprotoc_legacy",
+ "//third_party/protobuf:protobuf_legacy",
"//third_party/sqlite",
"//third_party/sqlite:sqlite_ext_percentile",
"//third_party/zlib:zlibsystem",
@@ -938,8 +938,8 @@
"//third_party/perfetto/protos:trace_track_event_cc_proto",
"//third_party/perfetto/protos:trace_track_event_zero_cc_proto",
"//third_party/perfetto/protos:trace_zero_cc_proto",
- "//third_party/protobuf",
- "//third_party/protobuf:libprotoc",
+ "//third_party/protobuf:libprotoc_legacy",
+ "//third_party/protobuf:protobuf_legacy",
"//third_party/sqlite",
"//third_party/sqlite:sqlite_ext_percentile",
"//third_party/zlib:zlibsystem",
diff --git a/protos/perfetto/trace/interned_data/interned_data.proto b/protos/perfetto/trace/interned_data/interned_data.proto
index 2ad06b2..cfad464 100644
--- a/protos/perfetto/trace/interned_data/interned_data.proto
+++ b/protos/perfetto/trace/interned_data/interned_data.proto
@@ -51,7 +51,8 @@
// refers to them (since the last reset of interning state). They may also be
// emitted proactively in advance of referring to them in later packets.
//
-// Next id: 8.
+// Next reserved id: 8 (up to 15).
+// Next id: 20.
message InternedData {
// Each field's message type needs to specify an |iid| field, which is the ID
// of the entry in the field's interning index. Each field constructs its own
@@ -73,8 +74,6 @@
repeated InternedString mapping_paths = 17;
// Names of functions used in frames below.
repeated InternedString function_names = 5;
- // Symbol tables added to the end of the trace, for the current sequence.
- repeated InternedModuleSymbol module_symbols = 20;
// Executable files mapped into processes.
repeated Mapping mappings = 19;
diff --git a/protos/perfetto/trace/perfetto_trace.proto b/protos/perfetto/trace/perfetto_trace.proto
index 86b59e9..6f3843a 100644
--- a/protos/perfetto/trace/perfetto_trace.proto
+++ b/protos/perfetto/trace/perfetto_trace.proto
@@ -2602,7 +2602,8 @@
// refers to them (since the last reset of interning state). They may also be
// emitted proactively in advance of referring to them in later packets.
//
-// Next id: 8.
+// Next reserved id: 8 (up to 15).
+// Next id: 20.
message InternedData {
// Each field's message type needs to specify an |iid| field, which is the ID
// of the entry in the field's interning index. Each field constructs its own
@@ -2624,8 +2625,6 @@
repeated InternedString mapping_paths = 17;
// Names of functions used in frames below.
repeated InternedString function_names = 5;
- // Symbol tables added to the end of the trace, for the current sequence.
- repeated InternedModuleSymbol module_symbols = 20;
// Executable files mapped into processes.
repeated Mapping mappings = 19;
@@ -2732,9 +2731,9 @@
// A symbol field that is emitted after the trace is written. These tables would
// be appended as the last packets in the trace that the profiler will use, so
// that the actual trace need not be rewritten to symbolize the profiles.
-message InternedModuleSymbol {
+message ProfiledFrameSymbols {
// Use the frame id as the interning key for the symbols.
- optional int64 frame_id = 1;
+ optional int64 frame_iid = 1;
optional int64 function_name_id = 2; // key to InternedString
optional int64 file_name_id = 3; // key to InternedString
@@ -3060,7 +3059,7 @@
// TracePacket(s).
//
// Next reserved id: 13 (up to 15).
-// Next id: 54.
+// Next id: 56.
message TracePacket {
// TODO(primiano): in future we should add a timestamp_clock_domain field to
// allow mixing timestamps from different clock domains.
@@ -3096,6 +3095,9 @@
GpuRenderStageEvent gpu_render_stage_event = 53;
StreamingProfilePacket streaming_profile_packet = 54;
+ // Only used in profile packets.
+ ProfiledFrameSymbols profiled_frame_symbols = 55;
+
// Only used by TrackEvent.
ProcessDescriptor process_descriptor = 43;
ThreadDescriptor thread_descriptor = 44;
diff --git a/protos/perfetto/trace/profiling/profile_common.proto b/protos/perfetto/trace/profiling/profile_common.proto
index d0983e1..42c13db 100644
--- a/protos/perfetto/trace/profiling/profile_common.proto
+++ b/protos/perfetto/trace/profiling/profile_common.proto
@@ -34,9 +34,9 @@
// A symbol field that is emitted after the trace is written. These tables would
// be appended as the last packets in the trace that the profiler will use, so
// that the actual trace need not be rewritten to symbolize the profiles.
-message InternedModuleSymbol {
+message ProfiledFrameSymbols {
// Use the frame id as the interning key for the symbols.
- optional int64 frame_id = 1;
+ optional int64 frame_iid = 1;
optional int64 function_name_id = 2; // key to InternedString
optional int64 file_name_id = 3; // key to InternedString
diff --git a/protos/perfetto/trace/trace_packet.proto b/protos/perfetto/trace/trace_packet.proto
index 756fa51..c2a138e 100644
--- a/protos/perfetto/trace/trace_packet.proto
+++ b/protos/perfetto/trace/trace_packet.proto
@@ -34,6 +34,7 @@
import "perfetto/trace/perfetto/perfetto_metatrace.proto";
import "perfetto/trace/power/battery_counters.proto";
import "perfetto/trace/power/power_rails.proto";
+import "perfetto/trace/profiling/profile_common.proto";
import "perfetto/trace/profiling/profile_packet.proto";
import "perfetto/trace/ps/process_stats.proto";
import "perfetto/trace/ps/process_tree.proto";
@@ -51,7 +52,7 @@
// TracePacket(s).
//
// Next reserved id: 13 (up to 15).
-// Next id: 54.
+// Next id: 56.
message TracePacket {
// TODO(primiano): in future we should add a timestamp_clock_domain field to
// allow mixing timestamps from different clock domains.
@@ -87,6 +88,9 @@
GpuRenderStageEvent gpu_render_stage_event = 53;
StreamingProfilePacket streaming_profile_packet = 54;
+ // Only used in profile packets.
+ ProfiledFrameSymbols profiled_frame_symbols = 55;
+
// Only used by TrackEvent.
ProcessDescriptor process_descriptor = 43;
ThreadDescriptor thread_descriptor = 44;
diff --git a/src/tracing/core/tracing_service_impl.cc b/src/tracing/core/tracing_service_impl.cc
index 11e042f..bd887ea 100644
--- a/src/tracing/core/tracing_service_impl.cc
+++ b/src/tracing/core/tracing_service_impl.cc
@@ -1426,8 +1426,13 @@
base::TimeMillis now = base::GetWallTimeMs();
if (now >= tracing_session->last_snapshot_time + kSnapshotsInterval) {
tracing_session->last_snapshot_time = now;
+ // Don't emit the stats immediately, but instead wait until no more trace
+ // data is available to read. That way, any problems that occur while
+ // reading from the buffers are reflected in the emitted stats. This is
+ // particularly important for use cases where ReadBuffers is only ever
+ // called after the tracing session is stopped.
+ tracing_session->should_emit_stats = true;
SnapshotSyncMarker(&packets);
- SnapshotStats(tracing_session, &packets);
if (!tracing_session->config.builtin_data_sources()
.disable_clock_snapshotting()) {
@@ -1530,6 +1535,19 @@
} // for(packets...)
} // for(buffers...)
+ const bool has_more = did_hit_threshold;
+ if (!has_more && tracing_session->should_emit_stats) {
+ size_t prev_packets_size = packets.size();
+ SnapshotStats(tracing_session, &packets);
+ tracing_session->should_emit_stats = false;
+
+ // Add sizes of packets emitted by SnapshotStats.
+ for (size_t i = prev_packets_size; i < packets.size(); ++i) {
+ packets_bytes += packets[i].size();
+ total_slices += packets[i].slices().size();
+ }
+ }
+
// If the caller asked us to write into a file by setting
// |write_into_file| == true in the trace config, drain the packets read
// (if any) into the given file descriptor.
@@ -1614,7 +1632,6 @@
return;
} // if (tracing_session->write_into_file)
- const bool has_more = did_hit_threshold;
if (has_more) {
auto weak_consumer = consumer->GetWeakPtr();
auto weak_this = weak_ptr_factory_.GetWeakPtr();
diff --git a/src/tracing/core/tracing_service_impl.h b/src/tracing/core/tracing_service_impl.h
index 0146f8b..9f009a0 100644
--- a/src/tracing/core/tracing_service_impl.h
+++ b/src/tracing/core/tracing_service_impl.h
@@ -452,6 +452,10 @@
// the output stream.
base::TimeMillis last_snapshot_time = {};
+ // Whether we should emit the trace stats next time we reach EOF while
+ // performing ReadBuffers.
+ bool should_emit_stats = false;
+
// Whether we mirrored the trace config back to the trace output yet.
bool did_emit_config = false;
diff --git a/src/tracing/core/tracing_service_impl_unittest.cc b/src/tracing/core/tracing_service_impl_unittest.cc
index 24e2988..547c9f9 100644
--- a/src/tracing/core/tracing_service_impl_unittest.cc
+++ b/src/tracing/core/tracing_service_impl_unittest.cc
@@ -1347,8 +1347,7 @@
// SystemInfo
// Trace read clocksnapshot
// Trace synchronisation
- // Trace stats
- static const int kNumPreamblePackets = 6;
+ static const int kNumPreamblePackets = 5;
static const int kNumTestPackets = 9;
static const char kPayload[] = "1234567890abcdef-";
diff --git a/tools/gen_bazel b/tools/gen_bazel
index 010f69d..6f7d5ca 100755
--- a/tools/gen_bazel
+++ b/tools/gen_bazel
@@ -95,8 +95,8 @@
def enable_protobuf_full(module):
- module.deps.add(Label('//third_party/protobuf:libprotoc'))
- module.deps.add(Label('//third_party/protobuf'))
+ module.deps.add(Label('//third_party/protobuf:libprotoc_legacy'))
+ module.deps.add(Label('//third_party/protobuf:protobuf_legacy'))
def enable_perfetto_version(module):
diff --git a/tools/trace_to_text/trace_to_systrace.cc b/tools/trace_to_text/trace_to_systrace.cc
index ffb503e..53a873b 100644
--- a/tools/trace_to_text/trace_to_systrace.cc
+++ b/tools/trace_to_text/trace_to_systrace.cc
@@ -229,8 +229,11 @@
auto p_callback = [](Iterator* it, base::StringWriter* writer) {
uint32_t pid = static_cast<uint32_t>(it->Get(0 /* col */).long_value);
uint32_t ppid = static_cast<uint32_t>(it->Get(1 /* col */).long_value);
- const char* name = it->Get(2 /* col */).string_value;
- FormatProcess(pid, ppid, name, writer);
+ const auto& name_col = it->Get(2 /* col */);
+ auto name_view = name_col.type == trace_processor::SqlValue::kString
+ ? base::StringView(name_col.string_value)
+ : base::StringView();
+ FormatProcess(pid, ppid, name_view, writer);
};
if (!q_writer.RunQuery(kPSql, p_callback))
return 1;
@@ -240,12 +243,15 @@
// Write out all the threads in the trace.
static const char kTSql[] =
"select tid, COALESCE(upid, 0), thread.name "
- "from thread inner join process using (upid)";
+ "from thread left join process using (upid)";
auto t_callback = [](Iterator* it, base::StringWriter* writer) {
uint32_t tid = static_cast<uint32_t>(it->Get(0 /* col */).long_value);
uint32_t tgid = static_cast<uint32_t>(it->Get(1 /* col */).long_value);
- const char* name = it->Get(2 /* col */).string_value;
- FormatThread(tid, tgid, name, writer);
+ const auto& name_col = it->Get(2 /* col */);
+ auto name_view = name_col.type == trace_processor::SqlValue::kString
+ ? base::StringView(name_col.string_value)
+ : base::StringView();
+ FormatThread(tid, tgid, name_view, writer);
};
if (!q_writer.RunQuery(kTSql, t_callback))
return 1;
