Set timestamp for proto-format memory snapshots

Timestamps for corresponding OS and chrome dumps need to be equal
so that they can be matched afterwards.

Bug: 1144103
Change-Id: Ib4d4375563b6ac6f983773bc85416c597186f1c2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2521152Reviewed-by: ssid <ssid@chromium.org>
Reviewed-by: Eric Seckler <eseckler@chromium.org>
Commit-Queue: Eric Seckler <eseckler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826771}

services/resource_coordinator/memory_instrumentation/queued_request_dispatcher.cc

@@ -471,6 +471,9 @@ void QueuedRequestDispatcher::Finalize(QueuedRequest* request,
   GraphProcessor::AddOverheadsAndPropagateEntries(global_graph.get());
   GraphProcessor::CalculateSizesForGraph(global_graph.get());
 
+  // The same timestamp needs to be set for all dumps in the memory snapshot.
+  base::TimeTicks timestamp = TRACE_TIME_TICKS_NOW();
+
   // Build up the global dump by iterating on the |valid| process dumps.
   mojom::GlobalMemoryDumpPtr global_dump(mojom::GlobalMemoryDump::New());
   global_dump->start_time = request->start_time;
@@ -520,7 +523,8 @@ void QueuedRequestDispatcher::Finalize(QueuedRequest* request,
     if (request->args.add_to_trace) {
       if (raw_os_dump) {
         bool trace_os_success = tracing_observer->AddOsDumpToTraceIfEnabled(
-            request->GetRequestArgs(), pid, *os_dump, raw_os_dump->memory_maps);
+            request->GetRequestArgs(), pid, *os_dump, raw_os_dump->memory_maps,
+            timestamp);
         if (!trace_os_success)
           request->failed_memory_dump_count++;
       }
@@ -528,7 +532,7 @@ void QueuedRequestDispatcher::Finalize(QueuedRequest* request,
       if (raw_chrome_dump) {
         bool trace_chrome_success = AddChromeMemoryDumpToTrace(
             request->GetRequestArgs(), pid, *raw_chrome_dump, *global_graph,
-            pid_to_process_type, tracing_observer, use_proto_writer);
+            pid_to_process_type, tracing_observer, use_proto_writer, timestamp);
         if (!trace_chrome_success)
           request->failed_memory_dump_count++;
       }
@@ -611,20 +615,21 @@ bool QueuedRequestDispatcher::AddChromeMemoryDumpToTrace(
     const GlobalNodeGraph& global_graph,
     const std::map<base::ProcessId, mojom::ProcessType>& pid_to_process_type,
     TracingObserver* tracing_observer,
-    bool use_proto_writer) {
+    bool use_proto_writer,
+    const base::TimeTicks& timestamp) {
   bool is_chrome_tracing_enabled =
       base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableChromeTracingComputation);
   if (!is_chrome_tracing_enabled) {
-    return tracing_observer->AddChromeDumpToTraceIfEnabled(args, pid,
-                                                           &raw_chrome_dump);
+    return tracing_observer->AddChromeDumpToTraceIfEnabled(
+        args, pid, &raw_chrome_dump, timestamp);
   }
   if (!tracing_observer->ShouldAddToTrace(args))
     return false;
 
   if (use_proto_writer) {
-    return tracing_observer->AddChromeDumpToTraceIfEnabled(args, pid,
-                                                           &raw_chrome_dump);
+    return tracing_observer->AddChromeDumpToTraceIfEnabled(
+        args, pid, &raw_chrome_dump, timestamp);
   }
 
   const GlobalNodeGraph::Process& process =

services/resource_coordinator/memory_instrumentation/queued_request_dispatcher.h

@@ -80,7 +80,8 @@ class QueuedRequestDispatcher {
       const perfetto::trace_processor::GlobalNodeGraph& global_graph,
       const std::map<base::ProcessId, mojom::ProcessType>& pid_to_process_type,
       TracingObserver* tracing_observer,
-      bool use_proto_writer);
+      bool use_proto_writer,
+      const base::TimeTicks& timestamp);
 };
 
 }  // namespace memory_instrumentation

services/resource_coordinator/public/cpp/memory_instrumentation/tracing_observer.cc

@@ -77,7 +77,8 @@ void TracingObserver::OnTraceLogDisabled() {
 bool TracingObserver::AddChromeDumpToTraceIfEnabled(
     const base::trace_event::MemoryDumpRequestArgs&,
     const base::ProcessId pid,
-    const base::trace_event::ProcessMemoryDump*) {
+    const base::trace_event::ProcessMemoryDump*,
+    const base::TimeTicks& timestamp) {
   return false;
 }
 
@@ -85,7 +86,8 @@ bool TracingObserver::AddOsDumpToTraceIfEnabled(
     const base::trace_event::MemoryDumpRequestArgs& args,
     const base::ProcessId pid,
     const mojom::OSMemDump& os_dump,
-    const std::vector<mojom::VmRegionPtr>& memory_maps) {
+    const std::vector<mojom::VmRegionPtr>& memory_maps,
+    const base::TimeTicks& timestamp) {
   return false;
 }
 

services/resource_coordinator/public/cpp/memory_instrumentation/tracing_observer.h

@@ -31,13 +31,15 @@ class COMPONENT_EXPORT(RESOURCE_COORDINATOR_PUBLIC_MEMORY_INSTRUMENTATION)
   virtual bool AddChromeDumpToTraceIfEnabled(
       const base::trace_event::MemoryDumpRequestArgs&,
       const base::ProcessId pid,
-      const base::trace_event::ProcessMemoryDump*);
+      const base::trace_event::ProcessMemoryDump*,
+      const base::TimeTicks& timestamp);
 
   virtual bool AddOsDumpToTraceIfEnabled(
       const base::trace_event::MemoryDumpRequestArgs& args,
       const base::ProcessId pid,
       const mojom::OSMemDump& os_dump,
-      const std::vector<mojom::VmRegionPtr>& memory_maps);
+      const std::vector<mojom::VmRegionPtr>& memory_maps,
+      const base::TimeTicks& timestamp);
 
   // TODO(lalitm): make these private again after TracingObserver is moved
   // to private space.

services/resource_coordinator/public/cpp/memory_instrumentation/tracing_observer_proto.cc

@@ -12,6 +12,7 @@
 #include "build/build_config.h"
 #include "services/tracing/public/cpp/perfetto/perfetto_producer.h"
 #include "services/tracing/public/cpp/perfetto/perfetto_traced_process.h"
+#include "services/tracing/public/cpp/perfetto/trace_time.h"
 #include "third_party/perfetto/protos/perfetto/trace/memory_graph.pbzero.h"
 #include "third_party/perfetto/protos/perfetto/trace/profiling/smaps.pbzero.h"
 #include "third_party/perfetto/protos/perfetto/trace/ps/process_stats.pbzero.h"
@@ -61,7 +62,8 @@ TracingObserverProto::~TracingObserverProto() = default;
 bool TracingObserverProto::AddChromeDumpToTraceIfEnabled(
     const base::trace_event::MemoryDumpRequestArgs& args,
     const base::ProcessId pid,
-    const ProcessMemoryDump* process_memory_dump) {
+    const ProcessMemoryDump* process_memory_dump,
+    const base::TimeTicks& timestamp) {
   if (!ShouldAddToTrace(args))
     return false;
 
@@ -72,6 +74,8 @@ bool TracingObserverProto::AddChromeDumpToTraceIfEnabled(
 
   perfetto::TraceWriter::TracePacketHandle handle =
       trace_writer_->NewTracePacket();
+  handle->set_timestamp(timestamp.since_origin().InNanoseconds());
+  handle->set_timestamp_clock_id(tracing::kTraceClockId);
   perfetto::protos::pbzero::MemoryTrackerSnapshot* memory_snapshot =
       handle->set_memory_tracker_snapshot();
   memory_snapshot->set_level_of_detail(
@@ -85,7 +89,8 @@ bool TracingObserverProto::AddOsDumpToTraceIfEnabled(
     const base::trace_event::MemoryDumpRequestArgs& args,
     const base::ProcessId pid,
     const mojom::OSMemDump& os_dump,
-    const std::vector<mojom::VmRegionPtr>& memory_maps) {
+    const std::vector<mojom::VmRegionPtr>& memory_maps,
+    const base::TimeTicks& timestamp) {
   if (!ShouldAddToTrace(args))
     return false;
 
@@ -96,6 +101,8 @@ bool TracingObserverProto::AddOsDumpToTraceIfEnabled(
 
   perfetto::TraceWriter::TracePacketHandle process_stats_packet =
       trace_writer_->NewTracePacket();
+  process_stats_packet->set_timestamp(timestamp.since_origin().InNanoseconds());
+  process_stats_packet->set_timestamp_clock_id(tracing::kTraceClockId);
   perfetto::protos::pbzero::ProcessStats* process_stats =
       process_stats_packet->set_process_stats();
   perfetto::protos::pbzero::ProcessStats::Process* process =
@@ -109,6 +116,8 @@ bool TracingObserverProto::AddOsDumpToTraceIfEnabled(
   if (memory_maps.size()) {
     perfetto::TraceWriter::TracePacketHandle smaps_packet =
         trace_writer_->NewTracePacket();
+    smaps_packet->set_timestamp(timestamp.since_origin().InNanoseconds());
+    smaps_packet->set_timestamp_clock_id(tracing::kTraceClockId);
     perfetto::protos::pbzero::SmapsPacket* smaps =
         smaps_packet->set_smaps_packet();
 

services/resource_coordinator/public/cpp/memory_instrumentation/tracing_observer_proto.h

@@ -37,12 +37,14 @@ class COMPONENT_EXPORT(RESOURCE_COORDINATOR_PUBLIC_MEMORY_INSTRUMENTATION)
   bool AddChromeDumpToTraceIfEnabled(
       const base::trace_event::MemoryDumpRequestArgs&,
       const base::ProcessId pid,
-      const base::trace_event::ProcessMemoryDump*) override;
+      const base::trace_event::ProcessMemoryDump*,
+      const base::TimeTicks& timestamp) override;
   bool AddOsDumpToTraceIfEnabled(
       const base::trace_event::MemoryDumpRequestArgs&,
       const base::ProcessId,
       const mojom::OSMemDump&,
-      const std::vector<mojom::VmRegionPtr>&) override;
+      const std::vector<mojom::VmRegionPtr>&,
+      const base::TimeTicks& timestamp) override;
 
   void StartTracing(
       tracing::PerfettoProducer* producer,

services/resource_coordinator/public/cpp/memory_instrumentation/tracing_observer_proto_unittest.cc

@@ -20,6 +20,7 @@
 #include "services/tracing/public/cpp/perfetto/perfetto_producer.h"
 #include "services/tracing/public/cpp/perfetto/perfetto_traced_process.h"
 #include "services/tracing/public/cpp/perfetto/producer_test_utils.h"
+#include "services/tracing/public/cpp/perfetto/trace_time.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/perfetto/protos/perfetto/trace/memory_graph.pbzero.h"
 #include "third_party/perfetto/protos/perfetto/trace/profiling/smaps.pbzero.h"
@@ -99,6 +100,10 @@ const uint32_t kResidentSetKb = 1;
 const uint32_t kPrivateFootprintKb = 2;
 const uint32_t kSharedFootprintKb = 3;
 
+const base::TimeTicks kTimestamp =
+    base::TimeTicks() + base::TimeDelta::FromMicroseconds(100000);
+const uint64_t kTimestampProto = kTimestamp.since_origin().InNanoseconds();
+
 uint64_t GetFakeAddrForVmRegion(int pid, int region_index) {
   return 0x100000ul * pid * (region_index + 1);
 }
@@ -149,13 +154,13 @@ TEST_F(TracingObserverProtoTest,
 
   base::trace_event::ProcessMemoryDump pmd = FillSamplePmd();
 
-  EXPECT_FALSE(
-      tracing_observer->AddChromeDumpToTraceIfEnabled(args, kTestPid, &pmd));
+  EXPECT_FALSE(tracing_observer->AddChromeDumpToTraceIfEnabled(
+      args, kTestPid, &pmd, kTimestamp));
 
   EnableTraceLog();
 
-  EXPECT_TRUE(
-      tracing_observer->AddChromeDumpToTraceIfEnabled(args, kTestPid, &pmd));
+  EXPECT_TRUE(tracing_observer->AddChromeDumpToTraceIfEnabled(
+      args, kTestPid, &pmd, kTimestamp));
 
   tracing_observer->StopTracing();
 }
@@ -172,14 +177,14 @@ TEST_F(TracingObserverProtoTest,
 
   base::trace_event::ProcessMemoryDump pmd = FillSamplePmd();
 
-  EXPECT_FALSE(
-      tracing_observer->AddChromeDumpToTraceIfEnabled(args, kTestPid, &pmd));
+  EXPECT_FALSE(tracing_observer->AddChromeDumpToTraceIfEnabled(
+      args, kTestPid, &pmd, kTimestamp));
 
   perfetto::DataSourceConfig config;
   tracing_observer->StartTracing(GetProducerClient(), config);
 
-  EXPECT_TRUE(
-      tracing_observer->AddChromeDumpToTraceIfEnabled(args, kTestPid, &pmd));
+  EXPECT_TRUE(tracing_observer->AddChromeDumpToTraceIfEnabled(
+      args, kTestPid, &pmd, kTimestamp));
 
   tracing_observer->StopTracing();
 }
@@ -202,12 +207,12 @@ TEST_F(TracingObserverProtoTest,
   std::vector<memory_instrumentation::mojom::VmRegionPtr> memory_map =
       FillMemoryMap(kTestPid);
   EXPECT_FALSE(tracing_observer->AddOsDumpToTraceIfEnabled(
-      args, kTestPid, os_dump, memory_map));
+      args, kTestPid, os_dump, memory_map, kTimestamp));
 
   EnableTraceLog();
 
-  EXPECT_TRUE(tracing_observer->AddOsDumpToTraceIfEnabled(args, kTestPid,
-                                                          os_dump, memory_map));
+  EXPECT_TRUE(tracing_observer->AddOsDumpToTraceIfEnabled(
+      args, kTestPid, os_dump, memory_map, kTimestamp));
 
   tracing_observer->StopTracing();
 }
@@ -227,13 +232,13 @@ TEST_F(TracingObserverProtoTest,
   std::vector<memory_instrumentation::mojom::VmRegionPtr> memory_map =
       FillMemoryMap(kTestPid);
   EXPECT_FALSE(tracing_observer->AddOsDumpToTraceIfEnabled(
-      args, kTestPid, os_dump, memory_map));
+      args, kTestPid, os_dump, memory_map, kTimestamp));
 
   perfetto::DataSourceConfig config;
   tracing_observer->StartTracing(GetProducerClient(), config);
 
-  EXPECT_TRUE(tracing_observer->AddOsDumpToTraceIfEnabled(args, kTestPid,
-                                                          os_dump, memory_map));
+  EXPECT_TRUE(tracing_observer->AddOsDumpToTraceIfEnabled(
+      args, kTestPid, os_dump, memory_map, kTimestamp));
 
   tracing_observer->StopTracing();
 }
@@ -252,15 +257,18 @@ TEST_F(TracingObserverProtoTest, AddChromeDumpToTraceIfEnabled) {
 
   base::trace_event::ProcessMemoryDump pmd = FillSamplePmd();
 
-  EXPECT_TRUE(
-      tracing_observer->AddChromeDumpToTraceIfEnabled(args, kTestPid, &pmd));
+  EXPECT_TRUE(tracing_observer->AddChromeDumpToTraceIfEnabled(
+      args, kTestPid, &pmd, kTimestamp));
 
   ASSERT_EQ(1ul, GetProducerClient()->GetFinalizedPacketCount());
 
   const perfetto::protos::TracePacket* packet =
       GetProducerClient()->GetFinalizedPacket(0);
-
   ASSERT_NE(nullptr, packet);
+  EXPECT_TRUE(packet->has_timestamp());
+  EXPECT_EQ(kTimestampProto, packet->timestamp());
+  EXPECT_TRUE(packet->has_timestamp_clock_id());
+  EXPECT_EQ(static_cast<uint32_t>(kTraceClockId), packet->timestamp_clock_id());
   EXPECT_TRUE(packet->has_memory_tracker_snapshot());
 
   const MemoryTrackerSnapshot& snapshot = packet->memory_tracker_snapshot();
@@ -317,15 +325,21 @@ TEST_F(TracingObserverProtoTest, AddOsDumpToTraceIfEnabled) {
 
   std::vector<memory_instrumentation::mojom::VmRegionPtr> memory_map =
       FillMemoryMap(kTestPid);
-  EXPECT_TRUE(tracing_observer->AddOsDumpToTraceIfEnabled(args, kTestPid,
-                                                          os_dump, memory_map));
+  EXPECT_TRUE(tracing_observer->AddOsDumpToTraceIfEnabled(
+      args, kTestPid, os_dump, memory_map, kTimestamp));
 
   EXPECT_EQ(2ul, GetProducerClient()->GetFinalizedPacketCount());
 
   const perfetto::protos::TracePacket* process_stats_trace_packet =
       GetProducerClient()->GetFinalizedPacket(0);
   ASSERT_NE(nullptr, process_stats_trace_packet);
+  EXPECT_TRUE(process_stats_trace_packet->has_timestamp());
+  EXPECT_EQ(kTimestampProto, process_stats_trace_packet->timestamp());
+  EXPECT_TRUE(process_stats_trace_packet->has_timestamp_clock_id());
+  EXPECT_EQ(static_cast<uint32_t>(kTraceClockId),
+            process_stats_trace_packet->timestamp_clock_id());
   EXPECT_TRUE(process_stats_trace_packet->has_process_stats());
+
   const ::perfetto::protos::ProcessStats& process_stats =
       process_stats_trace_packet->process_stats();
   EXPECT_EQ(1, process_stats.processes_size());

services/resource_coordinator/public/cpp/memory_instrumentation/tracing_observer_traced_value.cc

@@ -67,7 +67,8 @@ void TracingObserverTracedValue::AddToTrace(
 bool TracingObserverTracedValue::AddChromeDumpToTraceIfEnabled(
     const base::trace_event::MemoryDumpRequestArgs& args,
     const base::ProcessId pid,
-    const ProcessMemoryDump* process_memory_dump) {
+    const ProcessMemoryDump* process_memory_dump,
+    const base::TimeTicks& timastamp) {
   if (!ShouldAddToTrace(args))
     return false;
 
@@ -83,7 +84,8 @@ bool TracingObserverTracedValue::AddOsDumpToTraceIfEnabled(
     const base::trace_event::MemoryDumpRequestArgs& args,
     const base::ProcessId pid,
     const mojom::OSMemDump& os_dump,
-    const std::vector<mojom::VmRegionPtr>& memory_maps) {
+    const std::vector<mojom::VmRegionPtr>& memory_maps,
+    const base::TimeTicks& timestamp) {
   if (!ShouldAddToTrace(args))
     return false;
 

services/resource_coordinator/public/cpp/memory_instrumentation/tracing_observer_traced_value.h

@@ -25,12 +25,14 @@ class COMPONENT_EXPORT(RESOURCE_COORDINATOR_PUBLIC_MEMORY_INSTRUMENTATION)
   bool AddChromeDumpToTraceIfEnabled(
       const base::trace_event::MemoryDumpRequestArgs&,
       const base::ProcessId pid,
-      const base::trace_event::ProcessMemoryDump*) override;
+      const base::trace_event::ProcessMemoryDump*,
+      const base::TimeTicks& timestamp) override;
   bool AddOsDumpToTraceIfEnabled(
       const base::trace_event::MemoryDumpRequestArgs&,
       const base::ProcessId,
       const mojom::OSMemDump&,
-      const std::vector<mojom::VmRegionPtr>&) override;
+      const std::vector<mojom::VmRegionPtr>&,
+      const base::TimeTicks& timastamp) override;
 
   static void MemoryMapsAsValueInto(
       const std::vector<mojom::VmRegionPtr>& memory_maps,