0

[Cleanup] Remove DifferentWorkQueueCapacities experiment code

No good metric movements.

Bug: 1347892
Change-Id: Ibac003d725e2b8806eea1dd1c5c3394c0d37ba18
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4181491
Commit-Queue: Peilin Wang <peilinwang@google.com>
Reviewed-by: Egor Pasko <pasko@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1095067}
This commit is contained in:
Peilin Wang
2023-01-20 16:10:46 +00:00
committed by Chromium LUCI CQ
parent 6538aef3c4
commit f05f73a825
4 changed files with 6 additions and 106 deletions

@ -27,7 +27,6 @@
#include "base/strings/string_split.h"
#include "base/strings/string_util.h"
#include "base/strings/stringprintf.h"
#include "base/task/sequence_manager/work_queue.h"
#include "build/build_config.h"
namespace base {
@ -590,8 +589,6 @@ void FeatureList::SetInstance(std::unique_ptr<FeatureList> instance) {
g_cache_override_state =
base::FeatureList::IsEnabled(kCacheFeatureOverrideState);
base::sequence_manager::internal::WorkQueue::ConfigureCapacityFieldTrial();
#if BUILDFLAG(DCHECK_IS_CONFIGURABLE)
// Update the behaviour of LOGGING_DCHECK to match the Feature configuration.
// DCHECK is also forced to be FATAL if we are running a death-test.

@ -4,8 +4,6 @@
#include "base/task/sequence_manager/work_queue.h"
#include <atomic>
#include "base/containers/stack_container.h"
#include "base/debug/alias.h"
#include "base/metrics/field_trial_params.h"
@ -20,40 +18,6 @@ namespace base {
namespace sequence_manager {
namespace internal {
namespace {
BASE_FEATURE(kDifferentWorkQueueCapacities,
"DifferentWorkQueueCapacities",
FEATURE_DISABLED_BY_DEFAULT);
std::atomic<bool> g_different_work_queue_capacities_feature_enabled(false);
} // namespace
// static
void WorkQueue::ConfigureCapacityFieldTrial() {
g_different_work_queue_capacities_feature_enabled.store(
FeatureList::IsEnabled(kDifferentWorkQueueCapacities),
std::memory_order_relaxed);
}
// static
bool WorkQueue::IsDifferentWorkQueueCapacitiesEnabled() {
return g_different_work_queue_capacities_feature_enabled.load(
std::memory_order_relaxed);
}
// static
size_t WorkQueue::GetStackCapacityChoice() {
static const size_t kStackCapacityChoice =
IsDifferentWorkQueueCapacitiesEnabled()
? static_cast<size_t>(GetFieldTrialParamByFeatureAsInt(
kDifferentWorkQueueCapacities, "StackCapacity",
StackCapacity::kDefault))
: StackCapacity::kDefault;
return kStackCapacityChoice;
}
WorkQueue::WorkQueue(TaskQueueImpl* task_queue,
const char* name,
QueueType queue_type)
@ -256,12 +220,15 @@ Task WorkQueue::TakeTaskFromWorkQueue() {
return pending_task;
}
template <size_t stack_capacity>
bool WorkQueue::RemoveAllCancelledTasksFromFrontImpl() {
bool WorkQueue::RemoveAllCanceledTasksFromFront() {
if (!work_queue_sets_) {
return false;
}
// Since task destructors could have a side-effect of deleting this task queue
// we move cancelled tasks into a temporary container which can be emptied
// without accessing |this|.
StackVector<Task, stack_capacity> tasks_to_delete;
StackVector<Task, 8> tasks_to_delete;
while (!tasks_.empty()) {
const auto& pending_task = tasks_.front();
@ -291,31 +258,6 @@ bool WorkQueue::RemoveAllCancelledTasksFromFrontImpl() {
return !tasks_to_delete->empty();
}
template bool WorkQueue::RemoveAllCancelledTasksFromFrontImpl<
WorkQueue::StackCapacity::kSmall>();
template bool WorkQueue::RemoveAllCancelledTasksFromFrontImpl<
WorkQueue::StackCapacity::kMedium>();
template bool WorkQueue::RemoveAllCancelledTasksFromFrontImpl<
WorkQueue::StackCapacity::kLarge>();
template bool WorkQueue::RemoveAllCancelledTasksFromFrontImpl<
WorkQueue::StackCapacity::kDefault>();
bool WorkQueue::RemoveAllCanceledTasksFromFront() {
if (!work_queue_sets_)
return false;
switch (GetStackCapacityChoice()) {
case StackCapacity::kSmall:
return RemoveAllCancelledTasksFromFrontImpl<StackCapacity::kSmall>();
case StackCapacity::kMedium:
return RemoveAllCancelledTasksFromFrontImpl<StackCapacity::kMedium>();
case StackCapacity::kLarge:
return RemoveAllCancelledTasksFromFrontImpl<StackCapacity::kLarge>();
default:
return RemoveAllCancelledTasksFromFrontImpl<StackCapacity::kDefault>();
}
}
void WorkQueue::AssignToWorkQueueSets(WorkQueueSets* work_queue_sets) {
work_queue_sets_ = work_queue_sets;
}

@ -163,27 +163,6 @@ class BASE_EXPORT WorkQueue {
void CollectTasksOlderThan(TaskOrder reference,
std::vector<const Task*>* result) const;
// This is for an experiment where we try different WorkQueue capacities
// when deleting tasks in RemoveAllCanceledTasksFromFront() (see
// crbug.com/1347892). Tests spawn threads around the time FeatureList is
// initialized, which creates race conditions as WorkQueue is trying to read
// the Feature while threads spawn. To solve this, we store the value of the
// Feature in a std::atomic<bool> and initialize it once in SetInstance(), and
// read this atomic value instead of calling IsEnabled().
static void ConfigureCapacityFieldTrial();
static bool IsDifferentWorkQueueCapacitiesEnabled();
private:
enum StackCapacity : size_t {
kSmall = 4,
kMedium = 16,
kLarge = 24,
kDefault = 8,
};
static size_t GetStackCapacityChoice();
template <size_t stack_capacity>
bool RemoveAllCancelledTasksFromFrontImpl();
bool InsertFenceImpl(Fence fence);

@ -4062,24 +4062,6 @@
]
}
],
"DifferentWorkQueueCapacities": [
{
"platforms": [
"android"
],
"experiments": [
{
"name": "Enabled",
"params": {
"StackCapacity": "24"
},
"enable_features": [
"DifferentWorkQueueCapacities"
]
}
]
}
],
"DisableGles2ForOopR": [
{
"platforms": [