Skip to content

Commit

Permalink
[tracing] StartupTracingController
Browse files Browse the repository at this point in the history
Refactor startup tracing logic out of TracingControllerImpl and use
modern TracingSession from Perfetto client API directly.

This allows for much greater simplification, including removing
substantial amount of logic from TracingControllerImpl (including
logic for finishing trace recording after a timeout) and removing
PerfettoFileTracer.

This also allows for a clean implementation of features like streaming
trace to a file and merging browsertest and startup tracing logic.

R=eseckler@chromium.org,skyostil@chromium.org
BUG=1157954,1082916

Change-Id: Id4c608b05cbd6b0696cb8e3445888b6a9d86c797
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2587040
Commit-Queue: Alexander Timin <altimin@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: Eric Seckler <eseckler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#839190}
  • Loading branch information
Alexander Timin authored and Chromium LUCI CQ committed Dec 23, 2020
1 parent 78eb032 commit fc97567
Show file tree
Hide file tree
Showing 16 changed files with 366 additions and 476 deletions.
30 changes: 15 additions & 15 deletions components/tracing/common/trace_startup_config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ TraceStartupConfig::TraceStartupConfig() {
if (EnableFromCommandLine()) {
DCHECK(IsEnabled());
} else if (EnableFromConfigFile()) {
DCHECK(IsEnabled() || IsUsingPerfettoOutput());
DCHECK(IsEnabled());
} else if (EnableFromBackgroundTracing()) {
DCHECK(IsEnabled());
DCHECK(!IsTracingStartupForDuration());
Expand All @@ -99,12 +99,7 @@ TraceStartupConfig::TraceStartupConfig() {
TraceStartupConfig::~TraceStartupConfig() = default;

bool TraceStartupConfig::IsEnabled() const {
// TODO(oysteine): Support early startup tracing using Perfetto
// output; right now the early startup tracing gets controlled
// through the TracingController, and the Perfetto output is
// using the Consumer Mojo interface; the two can't be used
// together.
return is_enabled_ && !IsUsingPerfettoOutput();
return is_enabled_;
}

void TraceStartupConfig::SetDisabled() {
Expand All @@ -117,15 +112,20 @@ bool TraceStartupConfig::IsTracingStartupForDuration() const {
}

base::trace_event::TraceConfig TraceStartupConfig::GetTraceConfig() const {
DCHECK(IsEnabled() || IsUsingPerfettoOutput());
DCHECK(IsEnabled());
return trace_config_;
}

int TraceStartupConfig::GetStartupDuration() const {
DCHECK(IsEnabled() || IsUsingPerfettoOutput());
DCHECK(IsEnabled());
return startup_duration_in_seconds_;
}

TraceStartupConfig::OutputFormat TraceStartupConfig::GetOutputFormat() const {
DCHECK(IsEnabled());
return output_format_;
}

bool TraceStartupConfig::ShouldTraceToResultFile() const {
return IsEnabled() && should_trace_to_result_file_;
}
Expand All @@ -140,11 +140,6 @@ void TraceStartupConfig::OnTraceToResultFileFinished() {
finished_writing_to_file_ = true;
}

bool TraceStartupConfig::IsUsingPerfettoOutput() const {
return base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kPerfettoOutputFile);
}

void TraceStartupConfig::SetBackgroundStartupTracingEnabled(bool enabled) {
#if defined(OS_ANDROID)
base::android::SetBackgroundStartupTracingFlag(enabled);
Expand All @@ -168,7 +163,6 @@ bool TraceStartupConfig::AttemptAdoptBySessionOwner(SessionOwner owner) {
bool TraceStartupConfig::EnableFromCommandLine() {
auto* command_line = base::CommandLine::ForCurrentProcess();

// Startup duration can be used by along with perfetto-output-file flag.
if (command_line->HasSwitch(switches::kTraceStartupDuration)) {
std::string startup_duration_str =
command_line->GetSwitchValueASCII(switches::kTraceStartupDuration);
Expand All @@ -180,6 +174,12 @@ bool TraceStartupConfig::EnableFromCommandLine() {
}
}

if (command_line->GetSwitchValueASCII(switches::kTraceStartupFormat) ==
"proto") {
// Default is "json".
output_format_ = OutputFormat::kProto;
}

if (!command_line->HasSwitch(switches::kTraceStartup))
return false;

Expand Down
10 changes: 8 additions & 2 deletions components/tracing/common/trace_startup_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ class TRACING_EXPORT TraceStartupConfig {
kSystemTracing
};

enum class OutputFormat {
kLegacyJSON,
kProto,
};

static TraceStartupConfig* GetInstance();

// Default minimum startup trace config with enough events to debug issues.
Expand Down Expand Up @@ -135,6 +140,8 @@ class TRACING_EXPORT TraceStartupConfig {

SessionOwner GetSessionOwner() const;

OutputFormat GetOutputFormat() const;

// Called by a potential session owner to determine if it should take
// ownership of the startup tracing session and begin tracing. Returns |true|
// if the passed |owner| should adopt the session.
Expand All @@ -152,8 +159,6 @@ class TRACING_EXPORT TraceStartupConfig {
TraceStartupConfig();
~TraceStartupConfig();

bool IsUsingPerfettoOutput() const;

bool EnableFromCommandLine();
bool EnableFromConfigFile();
bool EnableFromBackgroundTracing();
Expand All @@ -170,6 +175,7 @@ class TRACING_EXPORT TraceStartupConfig {
bool finished_writing_to_file_ = false;
SessionOwner session_owner_ = SessionOwner::kTracingController;
bool session_adopted_ = false;
OutputFormat output_format_ = OutputFormat::kLegacyJSON;

DISALLOW_COPY_AND_ASSIGN(TraceStartupConfig);
};
Expand Down
15 changes: 8 additions & 7 deletions components/tracing/common/tracing_switches.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@ const char kTraceStartupDuration[] = "trace-startup-duration";
// all events since startup.
const char kTraceStartupFile[] = "trace-startup-file";

// Sets the output format for the trace, valid values are "json" and "proto".
// If not set, the current default is "json".
// "proto", unlike json, supports writing the trace into the output file
// incrementally and is more likely to retain more data if the browser process
// unexpectedly terminates.
// Ignored if "trace-startup-owner" is not "controller".
const char kTraceStartupFormat[] = "trace-startup-format";

// If supplied, sets the tracing record mode and options; otherwise, the default
// "record-until-full" mode will be used.
const char kTraceStartupRecordMode[] = "trace-startup-record-mode";
Expand Down Expand Up @@ -66,13 +74,6 @@ const char kTraceStartupEnablePrivacyFiltering[] =
// Repeat internable data for each TraceEvent in the perfetto proto format.
const char kPerfettoDisableInterning[] = "perfetto-disable-interning";

// If supplied, will enable Perfetto startup tracing and stream the
// output to the given file. On Android, if no file is provided, automatically
// generate a file to write the output to.
// TODO(oysteine): Remove once Perfetto starts early enough after
// process startup to be able to replace the legacy startup tracing.
const char kPerfettoOutputFile[] = "perfetto-output-file";

// Sends a pretty-printed version of tracing info to the console.
const char kTraceToConsole[] = "trace-to-console";

Expand Down
2 changes: 1 addition & 1 deletion components/tracing/common/tracing_switches.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ TRACING_EXPORT extern const char kTraceStartup[];
TRACING_EXPORT extern const char kTraceStartupDuration[];
TRACING_EXPORT extern const char kTraceStartupFile[];
TRACING_EXPORT extern const char kTraceStartupRecordMode[];
TRACING_EXPORT extern const char kTraceStartupFormat[];
TRACING_EXPORT extern const char kTraceStartupOwner[];
TRACING_EXPORT extern const char kTraceStartupEnablePrivacyFiltering[];
TRACING_EXPORT extern const char kPerfettoDisableInterning[];
TRACING_EXPORT extern const char kPerfettoOutputFile[];
TRACING_EXPORT extern const char kTraceToConsole[];
TRACING_EXPORT extern const char kTraceUploadURL[];

Expand Down
4 changes: 2 additions & 2 deletions content/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1742,8 +1742,8 @@ source_set("browser") {
"tracing/file_tracing_provider_impl.h",
"tracing/memory_instrumentation_util.cc",
"tracing/memory_instrumentation_util.h",
"tracing/perfetto_file_tracer.cc",
"tracing/perfetto_file_tracer.h",
"tracing/startup_tracing_controller.cc",
"tracing/startup_tracing_controller.h",
"tracing/tracing_controller_impl.cc",
"tracing/tracing_controller_impl.h",
"tracing/tracing_controller_impl_data_endpoint.cc",
Expand Down
4 changes: 2 additions & 2 deletions content/browser/browser_main_loop.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@
#include "components/discardable_memory/service/discardable_shared_memory_manager.h"
#include "components/services/storage/dom_storage/storage_area_impl.h"
#include "components/tracing/common/trace_startup_config.h"
#include "components/tracing/common/trace_to_console.h"
#include "components/tracing/common/tracing_switches.h"
#include "components/viz/host/compositing_mode_reporter_impl.h"
#include "components/viz/host/gpu_host_impl.h"
Expand Down Expand Up @@ -90,6 +89,7 @@
#include "content/browser/startup_data_impl.h"
#include "content/browser/startup_task_runner.h"
#include "content/browser/tracing/background_tracing_manager_impl.h"
#include "content/browser/tracing/startup_tracing_controller.h"
#include "content/browser/tracing/tracing_controller_impl.h"
#include "content/browser/utility_process_host.h"
#include "content/browser/webrtc/webrtc_internals.h"
Expand Down Expand Up @@ -1427,7 +1427,7 @@ void BrowserMainLoop::InitializeMojo() {
// need to start tracing for all other tracing agents, which require threads.
// We can only do this after starting the main message loop to avoid calling
// MessagePumpForUI::ScheduleWork() before MessagePumpForUI::Start().
TracingControllerImpl::GetInstance()->StartStartupTracingIfNeeded();
StartupTracingController::GetInstance().StartIfNeeded();

#if BUILDFLAG(MOJO_RANDOM_DELAYS_ENABLED)
mojo::BeginRandomMojoDelays();
Expand Down
5 changes: 2 additions & 3 deletions content/browser/browser_main_runner_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
#include "components/tracing/common/tracing_switches.h"
#include "content/browser/browser_main_loop.h"
#include "content/browser/notification_service_impl.h"
#include "content/browser/tracing/tracing_controller_impl.h"
#include "content/browser/tracing/startup_tracing_controller.h"
#include "content/common/content_switches_internal.h"
#include "content/public/common/content_switches.h"
#include "content/public/common/main_function_params.h"
Expand Down Expand Up @@ -167,8 +167,7 @@ void BrowserMainRunnerImpl::Shutdown() {
main_loop_->PreShutdown();

// Finalize the startup tracing session if it is still active.
if (TracingControllerImpl::GetInstance())
TracingControllerImpl::GetInstance()->FinalizeStartupTracingIfNeeded();
StartupTracingController::GetInstance().WaitUntilStopped();

{
// The trace event has to stay between profiler creation and destruction.
Expand Down
2 changes: 1 addition & 1 deletion content/browser/tracing/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ specific_include_rules = {
"cast_tracing_agent\.": [
"+chromecast/tracing"
],
"perfetto_file_tracer\.cc": [
"startup_tracing_controller\.cc": [
"+third_party/perfetto"
]
}
Loading

0 comments on commit fc97567

Please sign in to comment.