Skip to content

Commit

Permalink
[PM] Add an experiment to run on a dedicated thread
Browse files Browse the repository at this point in the history
This gives the PM sequence a fixed name that can be searched in traces
to find blocking calls.

Also removes the RunningOnUIThread() helper since base::Feature now
caches lookup results internally.

R=gab

Bug: 1189677
Change-Id: I884eec5c46dd562d2cc902ae16ca5365c334a984
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4346696
Auto-Submit: Joe Mason <joenotcharles@google.com>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Joe Mason <joenotcharles@google.com>
Cr-Commit-Position: refs/heads/main@{#1118801}
  • Loading branch information
JoeNotCharlesGoogle authored and Chromium LUCI CQ committed Mar 17, 2023
1 parent da5d7dc commit 681ab80
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 16 deletions.
4 changes: 4 additions & 0 deletions components/performance_manager/features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ BASE_FEATURE(kRunOnMainThread,
"RunOnMainThread",
base::FEATURE_DISABLED_BY_DEFAULT);

BASE_FEATURE(kRunOnDedicatedThreadPoolThread,
"RunOnDedicatedThreadPoolThread",
base::FEATURE_DISABLED_BY_DEFAULT);

#if !BUILDFLAG(IS_ANDROID)
BASE_FEATURE(kBackgroundTabLoadingFromPerformanceManager,
"BackgroundTabLoadingFromPerformanceManager",
Expand Down
45 changes: 30 additions & 15 deletions components/performance_manager/performance_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "base/notreached.h"
#include "base/task/lazy_thread_pool_task_runner.h"
#include "base/task/sequenced_task_runner.h"
#include "base/task/single_thread_task_runner_thread_mode.h"
#include "base/task/task_traits.h"
#include "components/performance_manager/graph/frame_node_impl.h"
#include "components/performance_manager/graph/page_node_impl.h"
Expand All @@ -41,14 +42,6 @@ PerformanceManagerImpl* g_performance_manager = nullptr;
// |g_performance_manager|. Should only be accessed on the main thread.
bool g_pm_is_available = false;

bool RunningOnUIThread() {
// This doesn't change from test to test, so we cache the value for
// efficiency.
static const bool kRunningOnUIThread =
base::FeatureList::IsEnabled(features::kRunOnMainThread);
return kRunningOnUIThread;
}

// Task traits appropriate for the PM task runner. This is a macro because it
// is used to build both content::BrowserTaskTraits and base::TaskTraits, which
// are type incompatible.
Expand All @@ -60,6 +53,11 @@ bool RunningOnUIThread() {
base::TaskShutdownBehavior::BLOCK_SHUTDOWN, base::MayBlock()

// Builds a UI task runner with the appropriate traits for the PM.
// TODO(crbug.com/1189677): The PM task runner has to block shutdown as some of
// the tasks posted to it should be guaranteed to run before shutdown (e.g.
// removing some entries from the site data store). The UI thread ignores
// MayBlock and TaskShutdownBehavior, so these tasks and any blocking tasks must
// be found and migrated to a worker thread.
scoped_refptr<base::SequencedTaskRunner> GetUITaskRunner() {
return content::GetUIThreadTaskRunner({PM_TASK_TRAITS});
}
Expand Down Expand Up @@ -234,19 +232,17 @@ void PerformanceManagerImpl::SetOnDestroyedCallbackForTesting(

PerformanceManagerImpl::PerformanceManagerImpl() {
DETACH_FROM_SEQUENCE(sequence_checker_);
if (RunningOnUIThread())
if (base::FeatureList::IsEnabled(features::kRunOnMainThread)) {
ui_task_runner_ = GetUITaskRunner();
}
}

// static
scoped_refptr<base::SequencedTaskRunner>
PerformanceManagerImpl::GetTaskRunner() {
// The performance manager TaskRunner. Thread-safe.
static base::LazyThreadPoolSequencedTaskRunner
performance_manager_task_runner =
LAZY_THREAD_POOL_SEQUENCED_TASK_RUNNER_INITIALIZER(
base::TaskTraits{PM_TASK_TRAITS});
if (RunningOnUIThread()) {
if (base::FeatureList::IsEnabled(features::kRunOnMainThread)) {
CHECK(!base::FeatureList::IsEnabled(
features::kRunOnDedicatedThreadPoolThread));
// Used the cached runner, if available. This prevents doing repeated
// lookups.
if (g_performance_manager)
Expand All @@ -260,6 +256,20 @@ PerformanceManagerImpl::GetTaskRunner() {
// |g_performance_manager| while it was alive.
return GetUITaskRunner();
}
if (base::FeatureList::IsEnabled(features::kRunOnDedicatedThreadPoolThread)) {
CHECK(!base::FeatureList::IsEnabled(features::kRunOnMainThread));
// Use a dedicated thread so that all tasks on the PM sequence can be
// identified in traces.
static base::LazyThreadPoolSingleThreadTaskRunner task_runner =
LAZY_THREAD_POOL_SINGLE_THREAD_TASK_RUNNER_INITIALIZER(
base::TaskTraits{PM_TASK_TRAITS},
base::SingleThreadTaskRunnerThreadMode::DEDICATED);
return task_runner.Get();
}
static base::LazyThreadPoolSequencedTaskRunner
performance_manager_task_runner =
LAZY_THREAD_POOL_SEQUENCED_TASK_RUNNER_INITIALIZER(
base::TaskTraits{PM_TASK_TRAITS});
return performance_manager_task_runner.Get();
}

Expand Down Expand Up @@ -380,6 +390,11 @@ void PerformanceManagerImpl::OnStartImpl(GraphImplCallback on_start) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(!g_performance_manager);

if (base::FeatureList::IsEnabled(features::kRunOnDedicatedThreadPoolThread)) {
// This should be the first task that runs on the dedicated thread.
base::PlatformThread::SetName("Performance Manager");
}

g_performance_manager = this;
graph_.SetUp();
graph_.set_ukm_recorder(ukm::UkmRecorder::Get());
Expand Down
10 changes: 9 additions & 1 deletion components/performance_manager/public/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,17 @@

namespace performance_manager::features {

// The feature that gates whether or not the PM runs on the main (UI) thread.
// If enabled the PM runs on the main (UI) thread. Incompatible with
// kRunOnDedicatedThreadPoolThread.
BASE_DECLARE_FEATURE(kRunOnMainThread);

// If enabled the PM runs on a single ThreadPool thread that isn't shared with
// any other task runners. It will be named "Performance Manager" in traces.
// This makes it easy to identify tasks running on the PM sequence, but may not
// perform as well as a shared sequence, which is the default. Incompatible with
// kRunOnMainThread.
BASE_DECLARE_FEATURE(kRunOnDedicatedThreadPoolThread);

#if !BUILDFLAG(IS_ANDROID)

#if BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_LINUX)
Expand Down

0 comments on commit 681ab80

Please sign in to comment.