Commit 1b876dd0 authored by Francois Doray's avatar Francois Doray Committed by Commit Bot

Base: Add feature to disable thread priorities.

This will allow us to assess the impact of using thread priorities.
The impact can be both positive (unimportant work gets out of the way)
and negative (priority inversions when sharing locks across threads
with different priorities).

Note: This CL has extra complexity because we don't have guarantees
that the FeatureList is initialized before threads are started in
unit tests https://crbug.com/846380.

Bug: 872820, 890978, 902441, 846380
Change-Id: I8887c7b9e0eb77f7c11aa6e2be8af1ea7150b491
Reviewed-on: https://chromium-review.googlesource.com/c/1318686
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: default avatarAlexei Svitkine <asvitkine@chromium.org>
Reviewed-by: default avatarGabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607276}
parent afb8f2fd
......@@ -849,6 +849,7 @@ jumbo_component("base") {
"third_party/nspr/prtime.h",
"third_party/superfasthash/superfasthash.c",
"thread_annotations.h",
"threading/platform_thread.cc",
"threading/platform_thread.h",
"threading/platform_thread_android.cc",
"threading/platform_thread_linux.cc",
......
......@@ -24,6 +24,7 @@
#include "base/task/task_scheduler/sequence_sort_key.h"
#include "base/task/task_scheduler/service_thread.h"
#include "base/task/task_scheduler/task.h"
#include "base/threading/platform_thread.h"
#include "base/time/time.h"
namespace base {
......@@ -113,6 +114,8 @@ TaskSchedulerImpl::~TaskSchedulerImpl() {
void TaskSchedulerImpl::Start(
const TaskScheduler::InitParams& init_params,
SchedulerWorkerObserver* scheduler_worker_observer) {
internal::InitializeThreadPrioritiesFeature();
// This is set in Start() and not in the constructor because variation params
// are usually not ready when TaskSchedulerImpl is instantiated in a process.
if (FeatureList::IsEnabled(kAllTasksUserBlocking))
......
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "base/threading/platform_thread.h"
#include <memory>
#include "base/feature_list.h"
namespace base {
namespace {
// Whether thread priorities should be used. When disabled,
// PlatformThread::SetCurrentThreadPriority() no-ops.
const Feature kThreadPrioritiesFeature{"ThreadPriorities",
FEATURE_ENABLED_BY_DEFAULT};
// Whether thread priorities should be used.
//
// PlatformThread::SetCurrentThreadPriority() doesn't query the state of the
// feature directly because FeatureList initialization is not always
// synchronized with PlatformThread::SetCurrentThreadPriority().
std::atomic<bool> g_use_thread_priorities(true);
} // namespace
// static
void PlatformThread::SetCurrentThreadPriority(ThreadPriority priority) {
if (g_use_thread_priorities.load())
SetCurrentThreadPriorityImpl(priority);
}
namespace internal {
void InitializeThreadPrioritiesFeature() {
// A DCHECK is triggered on FeatureList initialization if the state of a
// feature has been checked before. To avoid triggering this DCHECK in unit
// tests that call this before initializing the FeatureList, only check the
// state of the feature if the FeatureList is initialized.
if (FeatureList::GetInstance() &&
!FeatureList::IsEnabled(kThreadPrioritiesFeature)) {
g_use_thread_priorities.store(false);
}
}
} // namespace internal
} // namespace base
......@@ -236,9 +236,20 @@ class BASE_EXPORT PlatformThread {
static size_t GetDefaultThreadStackSize();
private:
static void SetCurrentThreadPriorityImpl(ThreadPriority priority);
DISALLOW_IMPLICIT_CONSTRUCTORS(PlatformThread);
};
namespace internal {
// Initializes the "ThreadPriorities" feature. The feature state is only taken
// into account after this initialization. This initialization must be
// synchronized with calls to PlatformThread::SetCurrentThreadPriority().
void InitializeThreadPrioritiesFeature();
} // namespace internal
} // namespace base
#endif // BASE_THREADING_PLATFORM_THREAD_H_
......@@ -36,7 +36,7 @@ bool PlatformThread::CanIncreaseThreadPriority(ThreadPriority priority) {
}
// static
void PlatformThread::SetCurrentThreadPriority(ThreadPriority priority) {
void PlatformThread::SetCurrentThreadPriorityImpl(ThreadPriority priority) {
if (priority != ThreadPriority::NORMAL) {
NOTIMPLEMENTED() << "setting ThreadPriority " << static_cast<int>(priority);
}
......
......@@ -153,7 +153,7 @@ bool PlatformThread::CanIncreaseThreadPriority(ThreadPriority priority) {
}
// static
void PlatformThread::SetCurrentThreadPriority(ThreadPriority priority) {
void PlatformThread::SetCurrentThreadPriorityImpl(ThreadPriority priority) {
// Changing the priority of the main thread causes performance regressions.
// https://crbug.com/601270
DCHECK(![[NSThread currentThread] isMainThread]);
......
......@@ -293,7 +293,7 @@ bool PlatformThread::CanIncreaseThreadPriority(ThreadPriority priority) {
}
// static
void PlatformThread::SetCurrentThreadPriority(ThreadPriority priority) {
void PlatformThread::SetCurrentThreadPriorityImpl(ThreadPriority priority) {
#if defined(OS_NACL)
NOTIMPLEMENTED();
#else
......
......@@ -297,7 +297,7 @@ bool PlatformThread::CanIncreaseThreadPriority(ThreadPriority priority) {
}
// static
void PlatformThread::SetCurrentThreadPriority(ThreadPriority priority) {
void PlatformThread::SetCurrentThreadPriorityImpl(ThreadPriority priority) {
// A DCHECK is triggered on FeatureList initialization if the state of a
// feature has been checked before. We only want to trigger that DCHECK if the
// priority has been set to BACKGROUND before, so we are careful not to access
......
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