Skip to content

Commit

Permalink
tp: always try to convert ftrace timestamps
Browse files Browse the repository at this point in the history
We were assuming the default trace clock was always BOOTTIME for ftrace
which is not a valid assumption if the primary_trace_clock field is set.
Make sure to always go into clock tracker conversion so that the correct
conversion always happens.

Change-Id: I840ca6b0ff161bf740a01d773f183905b3cb4a49
  • Loading branch information
LalitMaganti committed Sep 5, 2024
1 parent 82b399b commit 3fb75e9
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 28 deletions.
9 changes: 6 additions & 3 deletions src/trace_processor/importers/common/clock_tracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -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_));
Expand Down
48 changes: 23 additions & 25 deletions src/trace_processor/importers/ftrace/ftrace_tokenizer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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()) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 3fb75e9

Please sign in to comment.