Commit a41391d4 authored by Etienne Pierre-doray's avatar Etienne Pierre-doray Committed by Commit Bot

Reland "[Task Scheduler]: Merge blocking and non-blocking pools."

This is a reland of 7a77c4e0

Death test should run in single thread. Adding
testing::GTEST_FLAG(death_test_style) = "threadsafe";
before EXPECT_DCHECK_DEATH() to fix flakiness.

Original change's description:
> [Task Scheduler]: Merge blocking and non-blocking pools.
>
> This CL creates a feature experiment that removes blocking worker pools
> from the task scheduler. Tasks with MayBlock trait are instead posted to
> the usual foreground/background pool, giving us better control over scheduling.
>
> Note: ScopedBlockingCall is used to add extra workers to the pool when a task is
> blocking. In this CL, we take advantage of this behavior to make sure enough
> workers are running on the CPU.
>
> Bug: 874080
> Change-Id: I7889a941f82203388c259b0d96073eb5ddf3de69
> Reviewed-on: https://chromium-review.googlesource.com/c/1249836
> Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
> Reviewed-by: François Doray <fdoray@chromium.org>
> Reviewed-by: Gabriel Charette <gab@chromium.org>
> Reviewed-by: Robert Kaplow (sloooow) <rkaplow@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#600401}

TBR=rkaplow@chromium.org

Bug: 874080
Change-Id: I54665c798d332eead0f81da0f15c93368ec883a8
Reviewed-on: https://chromium-review.googlesource.com/c/1290911
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: default avatarFrançois Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601122}
parent be1855d9
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/bind_helpers.h" #include "base/bind_helpers.h"
#include "base/compiler_specific.h" #include "base/compiler_specific.h"
#include "base/feature_list.h"
#include "base/message_loop/message_loop.h" #include "base/message_loop/message_loop.h"
#include "base/metrics/field_trial_params.h" #include "base/metrics/field_trial_params.h"
#include "base/stl_util.h" #include "base/stl_util.h"
...@@ -28,6 +29,27 @@ ...@@ -28,6 +29,27 @@
namespace base { namespace base {
namespace internal { namespace internal {
namespace {
// Returns worker pool EnvironmentType for given arguments |is_background| and
// |is_blocking|.
EnvironmentType GetEnvironmentIndex(bool is_background, bool is_blocking) {
if (is_background) {
if (is_blocking)
return BACKGROUND_BLOCKING;
return BACKGROUND;
}
if (is_blocking)
return FOREGROUND_BLOCKING;
return FOREGROUND;
}
} // namespace
const base::Feature kMergeBlockingNonBlockingPools = {
"MergeBlockingNonBlockingPools", base::FEATURE_DISABLED_BY_DEFAULT};
TaskSchedulerImpl::TaskSchedulerImpl(StringPiece histogram_label) TaskSchedulerImpl::TaskSchedulerImpl(StringPiece histogram_label)
: TaskSchedulerImpl(histogram_label, : TaskSchedulerImpl(histogram_label,
std::make_unique<TaskTrackerImpl>(histogram_label)) {} std::make_unique<TaskTrackerImpl>(histogram_label)) {}
...@@ -65,22 +87,20 @@ TaskSchedulerImpl::TaskSchedulerImpl( ...@@ -65,22 +87,20 @@ TaskSchedulerImpl::TaskSchedulerImpl(
task_tracker_->GetTrackedRef(), &delayed_task_manager_)); task_tracker_->GetTrackedRef(), &delayed_task_manager_));
} }
// Map environment indexes to pools. // Map environment indexes to pools. |kMergeBlockingNonBlockingPools| is
environment_to_worker_pool_[FOREGROUND] = worker_pools_[FOREGROUND].get(); // assumed to be disabled.
environment_to_worker_pool_[FOREGROUND] =
worker_pools_[GetEnvironmentIndex(false, false)].get();
environment_to_worker_pool_[FOREGROUND_BLOCKING] = environment_to_worker_pool_[FOREGROUND_BLOCKING] =
worker_pools_[FOREGROUND_BLOCKING].get(); worker_pools_[GetEnvironmentIndex(false, true)].get();
environment_to_worker_pool_[BACKGROUND] =
if (CanUseBackgroundPriorityForSchedulerWorker()) { worker_pools_[GetEnvironmentIndex(
environment_to_worker_pool_[BACKGROUND] = worker_pools_[BACKGROUND].get(); CanUseBackgroundPriorityForSchedulerWorker(), false)]
environment_to_worker_pool_[BACKGROUND_BLOCKING] = .get();
worker_pools_[BACKGROUND_BLOCKING].get(); environment_to_worker_pool_[BACKGROUND_BLOCKING] =
} else { worker_pools_[GetEnvironmentIndex(
// On platforms without background thread priority, tasks posted to the CanUseBackgroundPriorityForSchedulerWorker(), true)]
// background environment are run by foreground pools. .get();
environment_to_worker_pool_[BACKGROUND] = worker_pools_[FOREGROUND].get();
environment_to_worker_pool_[BACKGROUND_BLOCKING] =
worker_pools_[FOREGROUND_BLOCKING].get();
}
} }
TaskSchedulerImpl::~TaskSchedulerImpl() { TaskSchedulerImpl::~TaskSchedulerImpl() {
...@@ -99,6 +119,26 @@ void TaskSchedulerImpl::Start( ...@@ -99,6 +119,26 @@ void TaskSchedulerImpl::Start(
all_tasks_user_blocking_.Set(); all_tasks_user_blocking_.Set();
} }
const bool use_blocking_pools =
!base::FeatureList::IsEnabled(kMergeBlockingNonBlockingPools);
// Remap environment indexes to pools with |use_blocking_pools|.
// TODO(etiennep): This is only necessary because of the kMergeBlockingNonBlockingPools
// experiment. Remove this after the experiment.
environment_to_worker_pool_[FOREGROUND] =
worker_pools_[GetEnvironmentIndex(false, false)].get();
environment_to_worker_pool_[FOREGROUND_BLOCKING] =
worker_pools_[GetEnvironmentIndex(false, use_blocking_pools)].get();
environment_to_worker_pool_[BACKGROUND] =
worker_pools_[GetEnvironmentIndex(
CanUseBackgroundPriorityForSchedulerWorker(), false)]
.get();
environment_to_worker_pool_[BACKGROUND_BLOCKING] =
worker_pools_[GetEnvironmentIndex(
CanUseBackgroundPriorityForSchedulerWorker(),
use_blocking_pools)]
.get();
// Start the service thread. On platforms that support it (POSIX except NaCL // Start the service thread. On platforms that support it (POSIX except NaCL
// SFI), the service thread runs a MessageLoopForIO which is used to support // SFI), the service thread runs a MessageLoopForIO which is used to support
// FileDescriptorWatcher in the scope in which tasks run. // FileDescriptorWatcher in the scope in which tasks run.
...@@ -153,10 +193,10 @@ void TaskSchedulerImpl::Start( ...@@ -153,10 +193,10 @@ void TaskSchedulerImpl::Start(
max_best_effort_tasks_in_foreground_pool, service_thread_task_runner, max_best_effort_tasks_in_foreground_pool, service_thread_task_runner,
scheduler_worker_observer, worker_environment); scheduler_worker_observer, worker_environment);
const int max_best_effort_tasks_in_foreground_blocking_pool = std::max( const int max_best_effort_tasks_in_foreground_blocking_pool = std::max(
1, 1, std::min(
std::min( init_params.background_blocking_worker_pool_params.max_tasks(),
init_params.background_blocking_worker_pool_params.max_tasks(), init_params.foreground_blocking_worker_pool_params.max_tasks() /
init_params.foreground_blocking_worker_pool_params.max_tasks() / 2)); 2));
worker_pools_[FOREGROUND_BLOCKING]->Start( worker_pools_[FOREGROUND_BLOCKING]->Start(
init_params.foreground_blocking_worker_pool_params, init_params.foreground_blocking_worker_pool_params,
max_best_effort_tasks_in_foreground_blocking_pool, max_best_effort_tasks_in_foreground_blocking_pool,
......
...@@ -38,9 +38,12 @@ namespace base { ...@@ -38,9 +38,12 @@ namespace base {
class HistogramBase; class HistogramBase;
class Thread; class Thread;
struct Feature;
namespace internal { namespace internal {
extern const BASE_EXPORT base::Feature kMergeBlockingNonBlockingPools;
// Default TaskScheduler implementation. This class is thread-safe. // Default TaskScheduler implementation. This class is thread-safe.
class BASE_EXPORT TaskSchedulerImpl : public TaskScheduler { class BASE_EXPORT TaskSchedulerImpl : public TaskScheduler {
public: public:
......
...@@ -2565,6 +2565,25 @@ ...@@ -2565,6 +2565,25 @@
] ]
} }
], ],
"MergeBlockingNonBlockingPools": [
{
"platforms": [
"android",
"chromeos",
"linux",
"mac",
"windows"
],
"experiments": [
{
"name": "Enabled",
"enable_features": [
"MergeBlockingNonBlockingPools"
]
}
]
}
],
"ModernMediaControls": [ "ModernMediaControls": [
{ {
"platforms": [ "platforms": [
......
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