Merge "tp: always try to convert ftrace timestamps" into main

google · Sep 9, 2024 · ee11bfd · ee11bfd
2 parents bdb3068 + 3fb75e9
commit ee11bfd
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 28 deletions.
diff --git a/src/trace_processor/importers/common/clock_tracker.h b/src/trace_processor/importers/common/clock_tracker.h
@@ -176,7 +176,7 @@ class ClockTracker {
   }
 
   // Apply the clock offset to convert remote trace times to host trace time.
-  int64_t ToHostTraceTime(int64_t timestamp) {
+  PERFETTO_ALWAYS_INLINE int64_t ToHostTraceTime(int64_t timestamp) {
     if (PERFETTO_LIKELY(!context_->machine_id())) {
       // No need to convert host timestamps.
       return timestamp;
@@ -188,7 +188,9 @@ class ClockTracker {
     return timestamp - clock_offset;
   }
 
-  base::StatusOr<int64_t> ToTraceTime(ClockId clock_id, int64_t timestamp) {
+  PERFETTO_ALWAYS_INLINE base::StatusOr<int64_t> ToTraceTime(
+      ClockId clock_id,
+      int64_t timestamp) {
     if (PERFETTO_UNLIKELY(!trace_time_clock_id_used_for_conversion_)) {
       context_->metadata_tracker->SetMetadata(
           metadata::trace_time_clock_id,
@@ -197,8 +199,9 @@ class ClockTracker {
     }
     trace_time_clock_id_used_for_conversion_ = true;
 
-    if (clock_id == trace_time_clock_id_)
+    if (clock_id == trace_time_clock_id_) {
       return ToHostTraceTime(timestamp);
+    }
 
     ASSIGN_OR_RETURN(int64_t ts,
                      Convert(clock_id, timestamp, trace_time_clock_id_));

diff --git a/src/trace_processor/importers/ftrace/ftrace_tokenizer.cc b/src/trace_processor/importers/ftrace/ftrace_tokenizer.cc
@@ -15,14 +15,26 @@
  */
 
 #include "src/trace_processor/importers/ftrace/ftrace_tokenizer.h"
+#include <cstddef>
+#include <cstdint>
+#include <optional>
+#include <utility>
+#include <vector>
 
 #include "perfetto/base/logging.h"
 #include "perfetto/base/status.h"
+#include "perfetto/protozero/field.h"
 #include "perfetto/protozero/proto_decoder.h"
 #include "perfetto/protozero/proto_utils.h"
+#include "perfetto/public/compiler.h"
 #include "perfetto/trace_processor/basic_types.h"
+#include "perfetto/trace_processor/ref_counted.h"
+#include "perfetto/trace_processor/trace_blob_view.h"
+#include "src/trace_processor/importers/common/clock_tracker.h"
 #include "src/trace_processor/importers/common/machine_tracker.h"
 #include "src/trace_processor/importers/common/metadata_tracker.h"
+#include "src/trace_processor/importers/common/parser_types.h"
+#include "src/trace_processor/importers/proto/packet_sequence_state_generation.h"
 #include "src/trace_processor/sorter/trace_sorter.h"
 #include "src/trace_processor/storage/metadata.h"
 #include "src/trace_processor/storage/stats.h"
@@ -51,17 +63,6 @@ namespace {
 
 static constexpr uint32_t kFtraceGlobalClockIdForOldKernels = 64;
 
-PERFETTO_ALWAYS_INLINE base::StatusOr<int64_t> ResolveTraceTime(
-    TraceProcessorContext* context,
-    ClockTracker::ClockId clock_id,
-    int64_t ts) {
-  // On most traces (i.e. P+), the clock should be BOOTTIME.
-  if (PERFETTO_LIKELY(clock_id == BuiltinClock::BUILTIN_CLOCK_BOOTTIME &&
-                      !context->machine_id()))
-    return ts;
-  return context->clock_tracker->ToTraceTime(clock_id, ts);
-}
-
 // Fast path for parsing the event id of an ftrace event.
 // Speculate on the fact that, if the timestamp was found, the common pid
 // will appear immediately after and the event id immediately after that.
@@ -189,9 +190,8 @@ base::Status FtraceTokenizer::TokenizeFtraceBundle(
                             ? decoder.previous_bundle_end_timestamp()
                             : decoder.last_read_event_timestamp();
       int64_t timestamp = 0;
-      ASSIGN_OR_RETURN(
-          timestamp,
-          ResolveTraceTime(context_, clock_id, static_cast<int64_t>(raw_ts)));
+      ASSIGN_OR_RETURN(timestamp, context_->clock_tracker->ToTraceTime(
+                                      clock_id, static_cast<int64_t>(raw_ts)));
 
       std::optional<SqlValue> curr_latest_timestamp =
           context_->metadata_tracker->GetMetadata(
@@ -281,10 +281,8 @@ void FtraceTokenizer::TokenizeFtraceEvent(
     return;
   }
 
-  int64_t int64_timestamp = static_cast<int64_t>(raw_timestamp);
-  base::StatusOr<int64_t> timestamp =
-      ResolveTraceTime(context_, clock_id, int64_timestamp);
-
+  auto timestamp = context_->clock_tracker->ToTraceTime(
+      clock_id, static_cast<int64_t>(raw_timestamp));
   // ClockTracker will increment some error stats if it failed to convert the
   // timestamp so just return.
   if (!timestamp.ok()) {
@@ -347,8 +345,8 @@ void FtraceTokenizer::TokenizeFtraceCompactSchedSwitch(
     event.next_pid = *npid_it;
     event.next_prio = *nprio_it;
 
-    base::StatusOr<int64_t> timestamp =
-        ResolveTraceTime(context_, clock_id, event_timestamp);
+    auto timestamp =
+        context_->clock_tracker->ToTraceTime(clock_id, event_timestamp);
     if (!timestamp.ok()) {
       DlogWithLimit(timestamp.status());
       return;
@@ -404,8 +402,8 @@ void FtraceTokenizer::TokenizeFtraceCompactSchedWaking(
       common_flags_it++;
     }
 
-    base::StatusOr<int64_t> timestamp =
-        ResolveTraceTime(context_, clock_id, event_timestamp);
+    auto timestamp =
+        context_->clock_tracker->ToTraceTime(clock_id, event_timestamp);
     if (!timestamp.ok()) {
       DlogWithLimit(timestamp.status());
       return;
@@ -465,9 +463,9 @@ void FtraceTokenizer::TokenizeFtraceGpuWorkPeriod(
 
   // Enforce clock type for the event data to be CLOCK_MONOTONIC_RAW
   // as specified, to calculate the timestamp correctly.
-  int64_t int64_timestamp = static_cast<int64_t>(raw_timestamp);
-  base::StatusOr<int64_t> timestamp = ResolveTraceTime(
-      context_, BuiltinClock::BUILTIN_CLOCK_MONOTONIC_RAW, int64_timestamp);
+  auto timestamp = context_->clock_tracker->ToTraceTime(
+      BuiltinClock::BUILTIN_CLOCK_MONOTONIC_RAW,
+      static_cast<int64_t>(raw_timestamp));
 
   // ClockTracker will increment some error stats if it failed to convert the
   // timestamp so just return.