Skip to content

GitLab

T
tangled
Commit 0deae037 authored by Mike Wittman's avatar Mike Wittman Committed by Commit Bot
Browse files
Options

Modernize ThreadProfilerConfiguration 

Replace DISALLOW_COPY_AND_ASSIGN, delete destructor, use NoDestructor
in place of LazyInstance, and make enums camel case.

Change-Id: Icf278d2755acc8e3fd8fb97b071d0c199f4353b3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2438887
Commit-Queue: Mike Wittman <wittman@chromium.org>
Reviewed-by: default avatarEtienne Pierre-Doray <etiennep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#822904}
parent 3a1f3f06
Hide whitespace changes
Inline Side-by-side
Showing with 47 additions and 46 deletions
chrome/common/profiler/thread_profiler_configuration.cc
View file @ 0deae037
......@@ -6,7 +6,7 @@
#include "base/check.h"
#include "base/command_line.h"
#include "base/lazy_instance.h"
#include "base/no_destructor.h"
#include "base/notreached.h"
#include "base/rand_util.h"
#include "build/branding_buildflags.h"
......@@ -18,9 +18,6 @@
namespace {
base::LazyInstance<ThreadProfilerConfiguration>::Leaky g_configuration =
LAZY_INSTANCE_INITIALIZER;
// Returns true if the current execution is taking place in the browser process.
// Allows the profiler to be run in a special browser test mode for testing that
// profiles are collected as expected, by providing a switch value. The test
......@@ -46,14 +43,12 @@ base::Optional<version_info::Channel> GetReleaseChannel() {
} // namespace
ThreadProfilerConfiguration::ThreadProfilerConfiguration()
: platform_configuration_(ThreadProfilerPlatformConfiguration::Create(
IsBrowserTestModeEnabled())),
configuration_(GenerateConfiguration(
GetProfileParamsProcess(*base::CommandLine::ForCurrentProcess()),
*platform_configuration_)) {}
ThreadProfilerConfiguration::~ThreadProfilerConfiguration() = default;
// static
ThreadProfilerConfiguration* ThreadProfilerConfiguration::Get() {
static base::NoDestructor<ThreadProfilerConfiguration>
thread_profiler_configuration;
return thread_profiler_configuration.get();
}
base::StackSamplingProfiler::SamplingParams
ThreadProfilerConfiguration::GetSamplingParams() const {
......@@ -73,7 +68,7 @@ ThreadProfilerConfiguration::GetSamplingParams() const {
bool ThreadProfilerConfiguration::IsProfilerEnabledForCurrentProcess() const {
if (const ChildProcessConfiguration* child_process_configuration =
absl::get_if<ChildProcessConfiguration>(&configuration_)) {
return *child_process_configuration == CHILD_PROCESS_PROFILE_ENABLED;
return *child_process_configuration == kChildProcessProfileEnabled;
}
const base::Optional<VariationGroup>& variation_group =
......@@ -103,19 +98,19 @@ bool ThreadProfilerConfiguration::GetSyntheticFieldTrial(
*trial_name = "SyntheticStackProfilingConfiguration";
*group_name = std::string();
switch (*variation_group) {
case PROFILE_DISABLED:
case kProfileDisabled:
*group_name = "Disabled";
break;
case PROFILE_DISABLED_MODULE_NOT_INSTALLED:
case kProfileDisabledModuleNotInstalled:
*group_name = "DisabledModuleNotInstalled";
break;
case PROFILE_CONTROL:
case kProfileControl:
*group_name = "Control";
break;
case PROFILE_ENABLED:
case kProfileEnabled:
*group_name = "Enabled";
break;
}
......@@ -149,18 +144,20 @@ void ThreadProfilerConfiguration::AppendCommandLineSwitchForChildProcess(
}
}
// static
ThreadProfilerConfiguration* ThreadProfilerConfiguration::Get() {
return g_configuration.Pointer();
}
ThreadProfilerConfiguration::ThreadProfilerConfiguration()
: platform_configuration_(ThreadProfilerPlatformConfiguration::Create(
IsBrowserTestModeEnabled())),
configuration_(GenerateConfiguration(
GetProfileParamsProcess(*base::CommandLine::ForCurrentProcess()),
*platform_configuration_)) {}
// static
bool ThreadProfilerConfiguration::EnableForVariationGroup(
base::Optional<VariationGroup> variation_group) {
// Enable if assigned to a variation group, and the group is one of the groups
// that are to be enabled.
return variation_group.has_value() && (*variation_group == PROFILE_ENABLED ||
*variation_group == PROFILE_CONTROL);
return variation_group.has_value() && (*variation_group == kProfileEnabled ||
*variation_group == kProfileControl);
}
// static
......@@ -182,7 +179,7 @@ ThreadProfilerConfiguration::ChooseVariationGroup(
cumulative_weight += variation.weight;
}
NOTREACHED();
return PROFILE_DISABLED;
return kProfileDisabled;
}
// static
......@@ -202,7 +199,7 @@ ThreadProfilerConfiguration::GenerateBrowserProcessConfiguration(
platform_configuration.RequestRuntimeModuleInstall();
FALLTHROUGH;
case RuntimeModuleState::kModuleNotAvailable:
return PROFILE_DISABLED_MODULE_NOT_INSTALLED;
return kProfileDisabledModuleNotInstalled;
case RuntimeModuleState::kModuleNotRequired:
case RuntimeModuleState::kModulePresent:
......@@ -215,9 +212,9 @@ ThreadProfilerConfiguration::GenerateBrowserProcessConfiguration(
CHECK_EQ(0, relative_populations.experiment % 2);
return ChooseVariationGroup({
{PROFILE_ENABLED, relative_populations.enabled},
{PROFILE_CONTROL, relative_populations.experiment / 2},
{PROFILE_DISABLED, relative_populations.experiment / 2},
{kProfileEnabled, relative_populations.enabled},
{kProfileControl, relative_populations.experiment / 2},
{kProfileDisabled, relative_populations.experiment / 2},
});
}
......@@ -229,8 +226,8 @@ ThreadProfilerConfiguration::GenerateChildProcessConfiguration(
// browser process determines whether the profiler is enabled for the
// process.
return command_line.HasSwitch(switches::kStartStackProfiler)
? CHILD_PROCESS_PROFILE_ENABLED
: CHILD_PROCESS_PROFILE_DISABLED;
? kChildProcessProfileEnabled
: kChildProcessProfileDisabled;
}
// static
......
chrome/common/profiler/thread_profiler_configuration.h
View file @ 0deae037
......@@ -8,8 +8,6 @@
#include <initializer_list>
#include <string>
#include "base/callback.h"
#include "base/macros.h"
#include "base/optional.h"
#include "base/profiler/stack_sampling_profiler.h"
#include "components/metrics/call_stack_profile_params.h"
......@@ -17,6 +15,8 @@
namespace base {
class CommandLine;
template <typename>
class NoDestructor;
} // namespace base
class ThreadProfilerPlatformConfiguration;
......@@ -27,8 +27,13 @@ class ThreadProfilerPlatformConfiguration;
// processes are communicated via command line arguments.
class ThreadProfilerConfiguration {
public:
ThreadProfilerConfiguration();
~ThreadProfilerConfiguration();
// Returns the ThreadProfilerConfiguration for the process.
static ThreadProfilerConfiguration* Get();
~ThreadProfilerConfiguration() = delete;
ThreadProfilerConfiguration(const ThreadProfilerConfiguration&) = delete;
ThreadProfilerConfiguration& operator=(const ThreadProfilerConfiguration&) =
delete;
// Get the stack sampling params to use.
base::StackSamplingProfiler::SamplingParams GetSamplingParams() const;
......@@ -52,26 +57,25 @@ class ThreadProfilerConfiguration {
void AppendCommandLineSwitchForChildProcess(
base::CommandLine* command_line) const;
// Returns the ThreadProfilerConfiguration for the process.
static ThreadProfilerConfiguration* Get();
private:
friend base::NoDestructor<ThreadProfilerConfiguration>;
// The variation groups that represent the Chrome-wide profiling
// configurations.
enum VariationGroup {
// Disabled within the experiment.
PROFILE_DISABLED,
kProfileDisabled,
// Disabled because the required module is not installed, and outside the
// experiment.
PROFILE_DISABLED_MODULE_NOT_INSTALLED,
kProfileDisabledModuleNotInstalled,
// Enabled within the experiment (and paired with equal-sized
// PROFILE_DISABLED group).
PROFILE_CONTROL,
// kProfileDisabled group).
kProfileControl,
// Enabled outside of the experiment.
PROFILE_ENABLED,
kProfileEnabled,
};
// The configuration state for the browser process. If !has_value() profiling
......@@ -81,8 +85,8 @@ class ThreadProfilerConfiguration {
// The configuration state in child processes.
enum ChildProcessConfiguration {
CHILD_PROCESS_PROFILE_DISABLED,
CHILD_PROCESS_PROFILE_ENABLED,
kChildProcessProfileDisabled,
kChildProcessProfileEnabled,
};
// The configuration state for the current process, browser or child.
......@@ -96,6 +100,8 @@ class ThreadProfilerConfiguration {
int weight;
};
ThreadProfilerConfiguration();
// True if the profiler is to be enabled for |variation_group|.
static bool EnableForVariationGroup(
base::Optional<VariationGroup> variation_group);
......@@ -127,8 +133,6 @@ class ThreadProfilerConfiguration {
// Represents the configuration to use in the current process.
const Configuration configuration_;
DISALLOW_COPY_AND_ASSIGN(ThreadProfilerConfiguration);
};
#endif // CHROME_COMMON_PROFILER_THREAD_PROFILER_CONFIGURATION_H_
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