Commit 11c92d48 authored by Eric Seckler's avatar Eric Seckler Committed by Commit Bot

tracing: Avoid trace timestamp conversion to CLOCK_BOOTTIME on Posix.

Chrome traces use CLOCK_MONOTONIC as their trace clock. If the trace
only contains Chrome data, it doesn't make sense to convert its
timestamps to CLOCK_BOOTTIME - in fact, this conversion could lead to
cross-thread timestamp skew and misaligned system tracing events on
CrOS/chromecast.

This patch accomplishes this by configuring the correct clock
(CLOCK_MONOTONIC in this case) as trace clock for chrome-only traces,
thereby instructing trace processor not to convert timestamps.

Requires aosp/1315608.

Bug: 1085002,1060400
Change-Id: I5ff8660a6e27f28d27f68d72bb7cf312494431bc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2210427Reviewed-by: default avatarSami Kyöstilä <skyostil@chromium.org>
Commit-Queue: Eric Seckler <eseckler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#771003}
parent d0ba8444
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include "base/trace_event/trace_event.h" #include "base/trace_event/trace_event.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "build/chromecast_buildflags.h" #include "build/chromecast_buildflags.h"
#include "services/tracing/public/cpp/perfetto/trace_time.h"
#include "services/tracing/public/mojom/perfetto_service.mojom.h" #include "services/tracing/public/mojom/perfetto_service.mojom.h"
namespace tracing { namespace tracing {
...@@ -65,9 +66,18 @@ perfetto::TraceConfig GetDefaultPerfettoConfig( ...@@ -65,9 +66,18 @@ perfetto::TraceConfig GetDefaultPerfettoConfig(
break; break;
} }
// Perfetto uses clock_gettime for its internal snapshotting, which gets
// blocked by the sandboxed and isn't needed for Chrome regardless.
auto* builtin_data_sources = perfetto_config.mutable_builtin_data_sources(); auto* builtin_data_sources = perfetto_config.mutable_builtin_data_sources();
// Chrome uses CLOCK_MONOTONIC as its trace clock on Posix. To avoid that
// trace processor converts Chrome's event timestamps into CLOCK_BOOTTIME
// during import, we set the trace clock here (the service will emit it into
// the trace's ClockSnapshots). See also crbug.com/1060400, where the
// conversion to BOOTTIME caused CrOS and chromecast system data source data
// to be misaligned.
builtin_data_sources->set_primary_trace_clock(
static_cast<perfetto::protos::gen::BuiltinClock>(kTraceClockId));
// Chrome emits system / trace config metadata itself.
builtin_data_sources->set_disable_trace_config(privacy_filtering_enabled); builtin_data_sources->set_disable_trace_config(privacy_filtering_enabled);
builtin_data_sources->set_disable_system_info(privacy_filtering_enabled); builtin_data_sources->set_disable_system_info(privacy_filtering_enabled);
builtin_data_sources->set_disable_service_events(privacy_filtering_enabled); builtin_data_sources->set_disable_service_events(privacy_filtering_enabled);
......
...@@ -305,8 +305,7 @@ class TraceEventDataSourceTest : public testing::Test { ...@@ -305,8 +305,7 @@ class TraceEventDataSourceTest : public testing::Test {
ASSERT_EQ(packet->clock_snapshot().clocks().size(), 3); ASSERT_EQ(packet->clock_snapshot().clocks().size(), 3);
EXPECT_EQ(packet->clock_snapshot().clocks()[0].clock_id(), EXPECT_EQ(packet->clock_snapshot().clocks()[0].clock_id(),
static_cast<uint32_t>( static_cast<uint32_t>(kTraceClockId));
perfetto::protos::pbzero::ClockSnapshot::Clock::BOOTTIME));
EXPECT_FALSE(packet->clock_snapshot().clocks()[0].has_unit_multiplier_ns()); EXPECT_FALSE(packet->clock_snapshot().clocks()[0].has_unit_multiplier_ns());
EXPECT_FALSE(packet->clock_snapshot().clocks()[0].has_is_incremental()); EXPECT_FALSE(packet->clock_snapshot().clocks()[0].has_is_incremental());
......
...@@ -6,21 +6,21 @@ ...@@ -6,21 +6,21 @@
#define SERVICES_TRACING_PUBLIC_CPP_PERFETTO_TRACE_TIME_H_ #define SERVICES_TRACING_PUBLIC_CPP_PERFETTO_TRACE_TIME_H_
#include "build/build_config.h" #include "build/build_config.h"
#include "third_party/perfetto/protos/perfetto/trace/clock_snapshot.pbzero.h" #include "third_party/perfetto/protos/perfetto/common/builtin_clock.pbzero.h"
namespace tracing { namespace tracing {
#if defined(OS_LINUX) || defined(OS_ANDROID) || defined(OS_FUCHSIA) #if defined(OS_LINUX) || defined(OS_ANDROID) || defined(OS_FUCHSIA)
// Linux, Android, and Fuchsia all use CLOCK_MONOTONIC. See crbug.com/166153 // Linux, Android, and Fuchsia all use CLOCK_MONOTONIC. See crbug.com/166153
// about efforts to unify base::TimeTicks across all platforms. // about efforts to unify base::TimeTicks across all platforms.
constexpr perfetto::protos::pbzero::ClockSnapshot::Clock::BuiltinClocks constexpr perfetto::protos::pbzero::BuiltinClock kTraceClockId =
kTraceClockId = perfetto::protos::pbzero::ClockSnapshot::Clock::MONOTONIC; perfetto::protos::pbzero::BUILTIN_CLOCK_MONOTONIC;
#else #else
// Mac and Windows TimeTicks advance when sleeping, so are closest to BOOTTIME // Mac and Windows TimeTicks advance when sleeping, so are closest to BOOTTIME
// in behavior. // in behavior.
// TODO(eseckler): Support specifying Mac/Win platform clocks in BuiltinClocks. // TODO(eseckler): Support specifying Mac/Win platform clocks in BuiltinClock.
constexpr perfetto::protos::pbzero::ClockSnapshot::Clock::BuiltinClocks constexpr perfetto::protos::pbzero::BuiltinClock kTraceClockId =
kTraceClockId = perfetto::protos::pbzero::ClockSnapshot::Clock::BOOTTIME; perfetto::protos::pbzero::BUILTIN_CLOCK_BOOTTIME;
#endif #endif
// Returns CLOCK_BOOTTIME on systems that support it, otherwise falls back to // Returns CLOCK_BOOTTIME on systems that support it, otherwise falls back to
......
...@@ -915,20 +915,10 @@ void TrackEventThreadLocalEventSink::DoResetIncrementalState( ...@@ -915,20 +915,10 @@ void TrackEventThreadLocalEventSink::DoResetIncrementalState(
} }
ClockSnapshot* clocks = packet->set_clock_snapshot(); ClockSnapshot* clocks = packet->set_clock_snapshot();
// Always reference the boottime timestamps to help trace processor // Reference clock is in nanoseconds.
// translate the clocks to boottime more efficiently.
ClockSnapshot::Clock* clock_reference = clocks->add_clocks(); ClockSnapshot::Clock* clock_reference = clocks->add_clocks();
clock_reference->set_clock_id(ClockSnapshot::Clock::BOOTTIME); clock_reference->set_clock_id(kTraceClockId);
if (kTraceClockId == ClockSnapshot::Clock::BOOTTIME) {
clock_reference->set_timestamp(timestamp.since_origin().InNanoseconds()); clock_reference->set_timestamp(timestamp.since_origin().InNanoseconds());
} else {
int64_t current_boot_nanos = TraceBootTicksNow();
int64_t current_monotonic_nanos =
TRACE_TIME_TICKS_NOW().since_origin().InNanoseconds();
int64_t diff = current_boot_nanos - current_monotonic_nanos;
clock_reference->set_timestamp(timestamp.since_origin().InNanoseconds() +
diff);
}
// Absolute clock in micros. // Absolute clock in micros.
ClockSnapshot::Clock* clock_absolute = clocks->add_clocks(); ClockSnapshot::Clock* clock_absolute = clocks->add_clocks();
clock_absolute->set_clock_id(kClockIdAbsolute); clock_absolute->set_clock_id(kClockIdAbsolute);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment