Google Git
Sign in
flutter/third_party/perfetto/refs/heads/dev/reveman/tp-clock-fix^!/.
commit
5be60c6fbc08cdd720daa10b32c04387ba07a76f
[log]
author
David Reveman <dreveman@gmail.com>
Thu Nov 20 19:58:49 2025 -0500
committer
David Reveman <dreveman@gmail.com>
Wed Dec 17 11:59:09 2025 -0500
tree
8308da98cb04d4e251da5ca4f0823b3327cb423b
parent
17327fc485eec4ef82e18baa5b28e3484cc692d2 [diff]
tp: only allow missing clocks when full sort is enabled

This allows us to continue to generate errors for missing
timestamps in the default case shen partial sorting is used
and defer the errors until EOF when full sorting is used.

Also introduce ProcessEndOfFileDeferredPackets() function
to the ChunkedTraceReader API, which allows us to properly
process deferred packets.

diff --git a/include/perfetto/trace_processor/trace_processor_storage.h b/include/perfetto/trace_processor/trace_processor_storage.h
index 0e5748f..c6ceb88 100644
--- a/include/perfetto/trace_processor/trace_processor_storage.h
+++ b/include/perfetto/trace_processor/trace_processor_storage.h

@@ -45,6 +45,9 @@
   // See comment on TraceProcessor::Flush.
   virtual void Flush() = 0;
 
+  // See comment on TraceProcessor::ProcessEndOfFileDeferredPackets.
+  virtual base::Status ProcessEndOfFileDeferredPackets() = 0;
+
   // See comment on TraceProcessor::NotifyEndOfFile.
   virtual base::Status NotifyEndOfFile() = 0;
 };

diff --git a/src/trace_processor/forwarding_trace_parser.cc b/src/trace_processor/forwarding_trace_parser.cc
index efb66fa..723d602 100644
--- a/src/trace_processor/forwarding_trace_parser.cc
+++ b/src/trace_processor/forwarding_trace_parser.cc

@@ -146,6 +146,13 @@
   return reader_->Parse(std::move(blob));
 }
 
+base::Status ForwardingTraceParser::ProcessEndOfFileDeferredPackets() {
+  if (reader_) {
+    RETURN_IF_ERROR(reader_->ProcessEndOfFileDeferredPackets());
+  }
+  return base::OkStatus();
+}
+
 base::Status ForwardingTraceParser::NotifyEndOfFile() {
   if (reader_) {
     RETURN_IF_ERROR(reader_->NotifyEndOfFile());

diff --git a/src/trace_processor/forwarding_trace_parser.h b/src/trace_processor/forwarding_trace_parser.h
index bcc91c3..64a6b4f 100644
--- a/src/trace_processor/forwarding_trace_parser.h
+++ b/src/trace_processor/forwarding_trace_parser.h

@@ -38,6 +38,7 @@
 
   // ChunkedTraceReader implementation
   base::Status Parse(TraceBlobView) override;
+  [[nodiscard]] base::Status ProcessEndOfFileDeferredPackets() override;
   [[nodiscard]] base::Status NotifyEndOfFile() override;
 
   TraceType trace_type() const { return trace_type_; }

diff --git a/src/trace_processor/importers/archive/gzip_trace_parser.cc b/src/trace_processor/importers/archive/gzip_trace_parser.cc
index 5e550ca..e1b7d93 100644
--- a/src/trace_processor/importers/archive/gzip_trace_parser.cc
+++ b/src/trace_processor/importers/archive/gzip_trace_parser.cc

@@ -125,6 +125,10 @@
   }
 }
 
+base::Status GzipTraceParser::ProcessEndOfFileDeferredPackets() {
+  return inner_ ? inner_->ProcessEndOfFileDeferredPackets() : base::OkStatus();
+}
+
 base::Status GzipTraceParser::NotifyEndOfFile() {
   if (output_state_ != kStreamBoundary || decompressor_.AvailIn() > 0) {
     return base::ErrStatus("GZIP stream incomplete, trace is likely corrupt");

diff --git a/src/trace_processor/importers/archive/gzip_trace_parser.h b/src/trace_processor/importers/archive/gzip_trace_parser.h
index 17fdc4c..6a8121a 100644
--- a/src/trace_processor/importers/archive/gzip_trace_parser.h
+++ b/src/trace_processor/importers/archive/gzip_trace_parser.h

@@ -37,6 +37,7 @@
 
   // ChunkedTraceReader implementation
   base::Status Parse(TraceBlobView) override;
+  base::Status ProcessEndOfFileDeferredPackets() override;
   base::Status NotifyEndOfFile() override;
 
   base::Status ParseUnowned(const uint8_t*, size_t);

diff --git a/src/trace_processor/importers/archive/tar_trace_reader.cc b/src/trace_processor/importers/archive/tar_trace_reader.cc
index 7fc8106..e6ec3f2 100644
--- a/src/trace_processor/importers/archive/tar_trace_reader.cc
+++ b/src/trace_processor/importers/archive/tar_trace_reader.cc

@@ -205,6 +205,7 @@
     for (auto& data : file.second.data) {
       RETURN_IF_ERROR(parser.Parse(std::move(data)));
     }
+    RETURN_IF_ERROR(parser.ProcessEndOfFileDeferredPackets());
     RETURN_IF_ERROR(parser.NotifyEndOfFile());
     // Make sure the ForwardingTraceParser determined the same trace type as we
     // did.

diff --git a/src/trace_processor/importers/archive/zip_trace_reader.cc b/src/trace_processor/importers/archive/zip_trace_reader.cc
index b834b3a..33b8a90 100644
--- a/src/trace_processor/importers/archive/zip_trace_reader.cc
+++ b/src/trace_processor/importers/archive/zip_trace_reader.cc

@@ -82,6 +82,7 @@
 
     RETURN_IF_ERROR(parser.Parse(std::move(file.second.data)));
     RETURN_IF_ERROR(parser.NotifyEndOfFile());
+    RETURN_IF_ERROR(parser.ProcessEndOfFileDeferredPackets());
     // Make sure the ForwardingTraceParser determined the same trace type as we
     // did.
     PERFETTO_CHECK(parser.trace_type() == file.first.trace_type);

diff --git a/src/trace_processor/importers/common/chunked_trace_reader.h b/src/trace_processor/importers/common/chunked_trace_reader.h
index dc03b13..1a7d99a 100644
--- a/src/trace_processor/importers/common/chunked_trace_reader.h
+++ b/src/trace_processor/importers/common/chunked_trace_reader.h

@@ -36,6 +36,11 @@
   // The buffer size is guaranteed to be > 0.
   virtual base::Status Parse(TraceBlobView) = 0;
 
+  // Called after the last Parse() call and before NotifyEndOfFile() call.
+  [[nodiscard]] virtual base::Status ProcessEndOfFileDeferredPackets() {
+    return base::OkStatus();
+  }
+
   // Called after the last Parse() call.
   [[nodiscard]] virtual base::Status NotifyEndOfFile() = 0;
 };

diff --git a/src/trace_processor/importers/proto/proto_trace_parser_impl_unittest.cc b/src/trace_processor/importers/proto/proto_trace_parser_impl_unittest.cc
index 66ba4bb..57060d7 100644
--- a/src/trace_processor/importers/proto/proto_trace_parser_impl_unittest.cc
+++ b/src/trace_processor/importers/proto/proto_trace_parser_impl_unittest.cc

@@ -300,7 +300,10 @@
     auto status = reader_->Parse(TraceBlobView(
         TraceBlob::TakeOwnership(std::move(raw_trace), trace_bytes.size())));
     if (status.ok()) {
-      status = reader_->NotifyEndOfFile();
+      status = reader_->ProcessEndOfFileDeferredPackets();
+      if (status.ok()) {
+        status = reader_->NotifyEndOfFile();
+      }
     }
 
     ResetTraceBuffers();

diff --git a/src/trace_processor/importers/proto/proto_trace_reader.cc b/src/trace_processor/importers/proto/proto_trace_reader.cc
index bc37eb0..4082bdb 100644
--- a/src/trace_processor/importers/proto/proto_trace_reader.cc
+++ b/src/trace_processor/importers/proto/proto_trace_reader.cc

@@ -378,17 +378,20 @@
         converted_clock_id =
             ClockTracker::SequenceToGlobalClock(seq_id, timestamp_clock_id);
       }
-      auto trace_ts = context_->clock_tracker->ToTraceTime(
-          converted_clock_id, timestamp, packet.offset());
-      if (!trace_ts) {
-        // We need to switch to full sorting mode to ensure that packets with
-        // missing timestamp are handled correctly. Don't save the packet unless
-        // switching to full sorting mode succeeded.
-        if (!received_eof_ && context_->sorter->SetSortingMode(
-                                  TraceSorter::SortingMode::kFullSort)) {
+
+      // If full sorting mode is enabled then allow packets with missing
+      // timestamps to be deferred to EOF.
+      if (context_->sorter->sorting_mode() ==
+          TraceSorter::SortingMode::kFullSort) {
+        if (!received_eof_ &&
+            !context_->clock_tracker->HasPathToTraceTime(converted_clock_id)) {
           eof_deferred_packets_.push_back(std::move(packet));
           return base::OkStatus();
         }
+      }
+      auto trace_ts = context_->clock_tracker->ToTraceTime(
+          converted_clock_id, timestamp, packet.offset());
+      if (!trace_ts) {
         // We don't return an error here as it will cause the trace to stop
         // parsing. Instead, we rely on the stat increment (which happened
         // automatically in ToTraceTime) to inform the user about the error.
@@ -925,11 +928,15 @@
   }
 }
 
-base::Status ProtoTraceReader::NotifyEndOfFile() {
+base::Status ProtoTraceReader::ProcessEndOfFileDeferredPackets() {
   received_eof_ = true;
   for (auto& packet : eof_deferred_packets_) {
     RETURN_IF_ERROR(TimestampTokenizeAndPushToSorter(std::move(packet)));
   }
+  return base::OkStatus();
+}
+
+base::Status ProtoTraceReader::NotifyEndOfFile() {
   for (auto& module : module_context_.modules) {
     module->NotifyEndOfFile();
   }

diff --git a/src/trace_processor/importers/proto/proto_trace_reader.h b/src/trace_processor/importers/proto/proto_trace_reader.h
index 18c883d..dcb32d7 100644
--- a/src/trace_processor/importers/proto/proto_trace_reader.h
+++ b/src/trace_processor/importers/proto/proto_trace_reader.h

@@ -64,6 +64,7 @@
 
   // ChunkedTraceReader implementation.
   base::Status Parse(TraceBlobView) override;
+  base::Status ProcessEndOfFileDeferredPackets() override;
   base::Status NotifyEndOfFile() override;
 
   using SyncClockSnapshots = base::FlatHashMap<

diff --git a/src/trace_processor/minimal_shell.cc b/src/trace_processor/minimal_shell.cc
index 6f85fe0..2e2215e 100644
--- a/src/trace_processor/minimal_shell.cc
+++ b/src/trace_processor/minimal_shell.cc

@@ -34,6 +34,7 @@
 base::Status MinimalMain(int, char**) {
   std::unique_ptr<TraceProcessor> tp = TraceProcessor::CreateInstance({});
   RETURN_IF_ERROR(tp->Parse(std::unique_ptr<uint8_t[]>(new uint8_t[0]), 0));
+  RETURN_IF_ERROR(tp->ProcessEndOfFileDeferredPackets());
   RETURN_IF_ERROR(tp->NotifyEndOfFile());
 
   auto it = tp->ExecuteQuery("SELECT id FROM slice");

diff --git a/src/trace_processor/read_trace.cc b/src/trace_processor/read_trace.cc
index 1e6aa38..bc32837 100644
--- a/src/trace_processor/read_trace.cc
+++ b/src/trace_processor/read_trace.cc

@@ -79,6 +79,7 @@
     const std::function<void(uint64_t parsed_size)>& progress_callback,
     bool call_notify_end_of_file) {
   RETURN_IF_ERROR(ReadTraceUnfinalized(tp, filename, progress_callback));
+  RETURN_IF_ERROR(tp->ProcessEndOfFileDeferredPackets());
   if (call_notify_end_of_file) {
     return tp->NotifyEndOfFile();
   }
@@ -99,6 +100,7 @@
         new SerializingProtoTraceReader(output));
     GzipTraceParser parser(std::move(reader));
     RETURN_IF_ERROR(parser.ParseUnowned(data, size));
+    RETURN_IF_ERROR(parser.ProcessEndOfFileDeferredPackets());
     return parser.NotifyEndOfFile();
   }
 

diff --git a/src/trace_processor/rpc/httpd.cc b/src/trace_processor/rpc/httpd.cc
index f31e04a..8d4dc1d 100644
--- a/src/trace_processor/rpc/httpd.cc
+++ b/src/trace_processor/rpc/httpd.cc

@@ -194,6 +194,7 @@
   }
 
   if (req.uri == "/notify_eof") {
+    global_trace_processor_rpc_.ProcessEndOfFileDeferredPackets();
     global_trace_processor_rpc_.NotifyEndOfFile();
     return conn.SendResponse("200 OK", default_headers);
   }

diff --git a/src/trace_processor/rpc/rpc.cc b/src/trace_processor/rpc/rpc.cc
index 48d8178..98a3587 100644
--- a/src/trace_processor/rpc/rpc.cc
+++ b/src/trace_processor/rpc/rpc.cc

@@ -439,6 +439,15 @@
   return trace_processor_->Parse(std::move(data_copy), len);
 }
 
+base::Status Rpc::ProcessEndOfFileDeferredPackets() {
+  PERFETTO_TP_TRACE(metatrace::Category::API_TIMELINE,
+                    "RPC_PROCESS_END_OF_FILE_DEFERRED_PACKETS");
+
+  RETURN_IF_ERROR(trace_processor_->ProcessEndOfFileDeferredPackets());
+  MaybePrintProgress();
+  return base::OkStatus();
+}
+
 base::Status Rpc::NotifyEndOfFile() {
   PERFETTO_TP_TRACE(metatrace::Category::API_TIMELINE,
                     "RPC_NOTIFY_END_OF_FILE");

diff --git a/src/trace_processor/rpc/rpc.h b/src/trace_processor/rpc/rpc.h
index 27d0d7d..0e60816 100644
--- a/src/trace_processor/rpc/rpc.h
+++ b/src/trace_processor/rpc/rpc.h

@@ -108,6 +108,7 @@
   // the corresponding names in trace_processor.h . See that header for docs.
 
   base::Status Parse(const uint8_t*, size_t);
+  base::Status ProcessEndOfFileDeferredPackets();
   base::Status NotifyEndOfFile();
   std::string GetCurrentTraceName();
   std::vector<uint8_t> ComputeMetric(const uint8_t*, size_t);

diff --git a/src/trace_processor/trace_processor_impl.cc b/src/trace_processor/trace_processor_impl.cc
index 3391ad7..f266c1d 100644
--- a/src/trace_processor/trace_processor_impl.cc
+++ b/src/trace_processor/trace_processor_impl.cc

@@ -625,6 +625,10 @@
   }
 }
 
+base::Status TraceProcessorImpl::ProcessEndOfFileDeferredPackets() {
+  return TraceProcessorStorageImpl::ProcessEndOfFileDeferredPackets();
+}
+
 base::Status TraceProcessorImpl::NotifyEndOfFile() {
   if (notify_eof_called_) {
     const char kMessage[] =

diff --git a/src/trace_processor/trace_processor_impl.h b/src/trace_processor/trace_processor_impl.h
index 0e25f40..409ac23 100644
--- a/src/trace_processor/trace_processor_impl.h
+++ b/src/trace_processor/trace_processor_impl.h

@@ -66,6 +66,7 @@
 
   base::Status Parse(TraceBlobView) override;
   void Flush() override;
+  base::Status ProcessEndOfFileDeferredPackets() override;
   base::Status NotifyEndOfFile() override;
 
   // =================================================================

diff --git a/src/trace_processor/trace_processor_shell.cc b/src/trace_processor/trace_processor_shell.cc
index 1c3e10b..bb95da9 100644
--- a/src/trace_processor/trace_processor_shell.cc
+++ b/src/trace_processor/trace_processor_shell.cc

@@ -1282,6 +1282,8 @@
                            trace_file_path.c_str(), load_status.c_message());
   }
 
+  RETURN_IF_ERROR(trace_processor->ProcessEndOfFileDeferredPackets());
+
   bool is_proto_trace = false;
   {
     auto it = trace_processor->ExecuteQuery(

diff --git a/src/trace_processor/trace_processor_storage_impl.cc b/src/trace_processor/trace_processor_storage_impl.cc
index ec2ab43..1dbff5b 100644
--- a/src/trace_processor/trace_processor_storage_impl.cc
+++ b/src/trace_processor/trace_processor_storage_impl.cc

@@ -111,6 +111,16 @@
   }
 }
 
+base::Status TraceProcessorStorageImpl::ProcessEndOfFileDeferredPackets() {
+  if (!parser_) {
+    return base::OkStatus();
+  }
+  if (unrecoverable_parse_error_) {
+    return base::ErrStatus("Unrecoverable parsing error already occurred");
+  }
+  return parser_->ProcessEndOfFileDeferredPackets();
+}
+
 base::Status TraceProcessorStorageImpl::NotifyEndOfFile() {
   if (!parser_) {
     return base::OkStatus();

diff --git a/src/trace_processor/trace_processor_storage_impl.h b/src/trace_processor/trace_processor_storage_impl.h
index b1c209e..12f617e 100644
--- a/src/trace_processor/trace_processor_storage_impl.h
+++ b/src/trace_processor/trace_processor_storage_impl.h

@@ -38,6 +38,7 @@
 
   base::Status Parse(TraceBlobView) override;
   void Flush() override;
+  base::Status ProcessEndOfFileDeferredPackets() override;
   base::Status NotifyEndOfFile() override;
 
   void DestroyContext();

diff --git a/src/trace_processor/util/clock_synchronizer.h b/src/trace_processor/util/clock_synchronizer.h
index 1b207f9..3a39f51 100644
--- a/src/trace_processor/util/clock_synchronizer.h
+++ b/src/trace_processor/util/clock_synchronizer.h

@@ -401,6 +401,13 @@
     return base::OkStatus();
   }
 
+  bool HasPathToTraceTime(ClockId clock_id) {
+    if (clock_id == trace_time_clock_id_) {
+      return true;
+    }
+    return FindPath(clock_id, trace_time_clock_id_).valid();
+  }
+
   // Returns the timezone offset in seconds from UTC, if one has been set.
   std::optional<int64_t> timezone_offset() const { return timezone_offset_; }