Commit 150adef2 authored by David Trainor's avatar David Trainor Committed by Commit Bot

Revert "Sampling profiler: disable GPU main thread profiling on OS X"

This reverts commit 43a4f5d0.

Reason for revert: Looks like the changes to thread_profiler broke some Android tests? (ToastHWATest): https://ci.chromium.org/buildbot/chromium.android/Lollipop%20Phone%20Tester/19475

[FATAL:thread_profiler.cc(154)] Check failed: metrics::CallStackProfileParams::BROWSER_PROCESS != GetProcess() (1 vs. 1)

Reverting.  Sorry!


Original change's description:
> Sampling profiler: disable GPU main thread profiling on OS X
> 
> Disabling pending a resolution to crashes observed in the associated
> bug.
> 
> This change also removes the unused
> GetSamplingParamsForCurrentProcess function and adapts
> IsProfilerEnabledForCurrentProcess to operate on the current process
> and specified thread.
> 
> The call to IsProfilerEnabledForCurrentProcess is also removed
> from SetServiceManagerConnectorForChildProcess since that function is
> only invoked in processes supporting the profiler.
> 
> Bug: 774682
> Change-Id: Ibbc6f1bd9348ba09a3ee4db2e1595411617f1ccd
> Reviewed-on: https://chromium-review.googlesource.com/962937
> Commit-Queue: Mike Wittman <wittman@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: Leonard Grey <lgrey@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#543520}

TBR=sky@chromium.org,wittman@chromium.org,lgrey@chromium.org

Change-Id: I2ecac49a6e14de7f620dc0096ce229f25057c810
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 774682
Reviewed-on: https://chromium-review.googlesource.com/965821Reviewed-by: default avatarDavid Trainor <dtrainor@chromium.org>
Commit-Queue: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543613}
parent b4486d34
......@@ -72,20 +72,29 @@ StackSamplingConfiguration::StackSamplingConfiguration()
: configuration_(GenerateConfiguration()) {
}
bool StackSamplingConfiguration::IsProfilerEnabledForThread(
metrics::CallStackProfileParams::Thread thread) const {
base::StackSamplingProfiler::SamplingParams
StackSamplingConfiguration::GetSamplingParamsForCurrentProcess() const {
base::StackSamplingProfiler::SamplingParams params;
params.bursts = 1;
params.initial_delay = base::TimeDelta::FromMilliseconds(0);
params.sampling_interval = base::TimeDelta::FromMilliseconds(0);
params.samples_per_burst = 0;
if (IsProfilerEnabledForCurrentProcess()) {
const base::TimeDelta duration = base::TimeDelta::FromSeconds(30);
params.sampling_interval = base::TimeDelta::FromMilliseconds(100);
params.samples_per_burst = duration / params.sampling_interval;
}
return params;
}
bool StackSamplingConfiguration::IsProfilerEnabledForCurrentProcess() const {
if (IsBrowserProcess()) {
return configuration_ == PROFILE_ENABLED ||
configuration_ == PROFILE_CONTROL;
}
#if defined(OS_MACOSX)
// Disabled pending a resolution to crashes on Intel GPU tests. See
// https://crbug.com/774682.
if (thread == metrics::CallStackProfileParams::GPU_MAIN_THREAD)
return false;
#endif
DCHECK_EQ(PROFILE_FROM_COMMAND_LINE, configuration_);
// This is a child process. The |kStartStackProfiler| switch passed by the
// browser process determines whether the profiler is enabled for the process.
......
......@@ -10,7 +10,6 @@
#include "base/callback.h"
#include "base/macros.h"
#include "base/profiler/stack_sampling_profiler.h"
#include "components/metrics/call_stack_profile_params.h"
namespace base {
class CommandLine;
......@@ -24,10 +23,12 @@ class StackSamplingConfiguration {
public:
StackSamplingConfiguration();
// Returns true if the profiler should be started for the specified thread in
// the current process.
bool IsProfilerEnabledForThread(
metrics::CallStackProfileParams::Thread thread) const;
// Get the stack sampling params to use for this process.
base::StackSamplingProfiler::SamplingParams
GetSamplingParamsForCurrentProcess() const;
// Returns true if the profiler should be started for the current process.
bool IsProfilerEnabledForCurrentProcess() const;
// Get the synthetic field trial configuration. Returns true if a synthetic
// field trial should be registered. This should only be called from the
......
......@@ -124,10 +124,8 @@ std::unique_ptr<ThreadProfiler> ThreadProfiler::CreateAndStartOnMainThread(
void ThreadProfiler::SetMainThreadTaskRunner(
scoped_refptr<base::SingleThreadTaskRunner> task_runner) {
if (!StackSamplingConfiguration::Get()->IsProfilerEnabledForThread(
periodic_profile_params_.thread)) {
if (!StackSamplingConfiguration::Get()->IsProfilerEnabledForCurrentProcess())
return;
}
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
......@@ -141,7 +139,7 @@ void ThreadProfiler::SetMainThreadTaskRunner(
// static
void ThreadProfiler::StartOnChildThread(
metrics::CallStackProfileParams::Thread thread) {
if (!StackSamplingConfiguration::Get()->IsProfilerEnabledForThread(thread))
if (!StackSamplingConfiguration::Get()->IsProfilerEnabledForCurrentProcess())
return;
auto profiler = std::unique_ptr<ThreadProfiler>(
......@@ -151,6 +149,9 @@ void ThreadProfiler::StartOnChildThread(
void ThreadProfiler::SetServiceManagerConnectorForChildProcess(
service_manager::Connector* connector) {
if (!StackSamplingConfiguration::Get()->IsProfilerEnabledForCurrentProcess())
return;
DCHECK_NE(metrics::CallStackProfileParams::BROWSER_PROCESS, GetProcess());
metrics::mojom::CallStackProfileCollectorPtr browser_interface;
......@@ -193,7 +194,7 @@ ThreadProfiler::ThreadProfiler(
metrics::CallStackProfileParams::PERIODIC_COLLECTION,
metrics::CallStackProfileParams::MAY_SHUFFLE),
weak_factory_(this) {
if (!StackSamplingConfiguration::Get()->IsProfilerEnabledForThread(thread))
if (!StackSamplingConfiguration::Get()->IsProfilerEnabledForCurrentProcess())
return;
startup_profiler_ = std::make_unique<base::StackSamplingProfiler>(
......
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