Split initialization_util into a Hermetic Library and a Variations Library

components/task_scheduler_util/initialization/browser_util.cc

+// Copyright 2016 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 "components/task_scheduler_util/initialization/browser_util.h"
+
+#include <map>
+#include <string>
+
+#include "base/task_scheduler/initialization_util.h"
+#include "base/task_scheduler/switches.h"
+#include "base/task_scheduler/task_traits.h"
+#include "base/threading/sequenced_worker_pool.h"
+#include "build/build_config.h"
+
+namespace task_scheduler_util {
+namespace initialization {
+
+namespace {
+
+using StandbyThreadPolicy =
+    base::SchedulerWorkerPoolParams::StandbyThreadPolicy;
+using ThreadPriority = base::ThreadPriority;
+
+struct SchedulerWorkerPoolConfiguration {
+  const char* name;
+  ThreadPriority priority_hint;
+  const SingleWorkerPoolConfiguration& config;

[fdoray, 2016/12/07 13:34:11]

SchedulerWorkerPoolConfiguration vs. SingleWorkerPoolConfiguration: these names
are similar but refer to different things. I would rename
SingleWorkerPoolConfiguration -> SchedulerWorkerPoolCustomizableConfiguration to
highlight the fact that this is the dynamically customizable part. I did my best
to avoid using the word "variations" :)

[robliao, 2016/12/07 18:39:19]

Done.

+
+  SchedulerWorkerPoolConfiguration(

[gab, 2016/12/07 15:53:06]

constructors go before members in style-guide for both structs and classes

[robliao, 2016/12/07 18:39:19]

Done.

+      const char* name,
+      ThreadPriority priority_hint,
+      const SingleWorkerPoolConfiguration& config)

[gab, 2016/12/07 15:53:06]

To avoid name clash since structs to use _ suffixed members. I like the paradigm
of suffixing the in-params with "_in".

(I know it resolves correctly in the initializer list but readability >
correctness :))

[robliao, 2016/12/07 18:39:19]

Done.

+      : name(name),
+        priority_hint(priority_hint),
+        config(config) {}
+};
+
+}  // namespace
+
+std::vector<base::SchedulerWorkerPoolParams>
+BrowserWorkerPoolConfigurationToSchedulerWorkerPoolParams(
+    const BrowserWorkerPoolsConfiguration& config) {
+  const SchedulerWorkerPoolConfiguration worker_pool_config[] = {
+      SchedulerWorkerPoolConfiguration("Background",
+                                       ThreadPriority::BACKGROUND,
+                                       config.background),
+      SchedulerWorkerPoolConfiguration("BackgroundFileIO",
+                                       ThreadPriority::BACKGROUND,
+                                       config.background_file_io),
+      SchedulerWorkerPoolConfiguration("Foreground",
+                                       ThreadPriority::NORMAL,
+                                       config.foreground),
+      SchedulerWorkerPoolConfiguration("ForegroundFileIO",
+                                       ThreadPriority::NORMAL,
+                                       config.foreground_file_io),
+
+  };
+  static_assert(arraysize(worker_pool_config) == WORKER_POOL_COUNT,
+                "Mismatched Worker Pool Types and Predefined Parameters");
+  constexpr size_t kNumWorkerPoolsDefined = sizeof(config) /
+                                            sizeof(config.background);
+  static_assert(arraysize(worker_pool_config) == kNumWorkerPoolsDefined,
+                "Mismatch in predefined parameters and worker pools.");
+  std::vector<base::SchedulerWorkerPoolParams> params_vector;
+  for (size_t i = 0; i < arraysize(worker_pool_config); ++i) {

[fdoray, 2016/12/07 13:34:11]

for (const auto& config : worker_pool_config) {

[robliao, 2016/12/07 18:39:19]

Done.

+    const auto* const config = &worker_pool_config[i];

[gab, 2016/12/07 15:53:06]

const auto& is fine (and prefered) in general, what I meant earlier is that
const auto* should be used when the type is clearly a pointer (i.e. const auto&
foo ...; foo->... is weird as the arrow operator comes out of nowhere yet
encodes that auto has to resolve to a pointer so might as well be explicit)

Either way, the for-each loop proposed by francois is the best solution here.

[robliao, 2016/12/07 18:39:19]

Acknowledged.

+    params_vector.emplace_back(
+        config->name, config->priority_hint,
+        config->config.standby_thread_policy, config->config.threads,

[gab, 2016/12/07 15:53:06]

config->config is weird, can we change the name of one of the two members?

[robliao, 2016/12/07 18:39:19]

config.config -> config.single_worker_pool_config

+        config->config.detach_period);
+  }
+  DCHECK_EQ(WORKER_POOL_COUNT, params_vector.size());
+  return params_vector;
+}
+
+// Returns the worker pool index for |traits| defaulting to FOREGROUND or
+// FOREGROUND_FILE_IO on any other priorities.
+size_t BrowserWorkerPoolIndexForTraits(const base::TaskTraits& traits) {
+  const bool is_background =
+      traits.priority() == base::TaskPriority::BACKGROUND;
+  if (traits.with_file_io())
+    return is_background ? BACKGROUND_FILE_IO : FOREGROUND_FILE_IO;
+
+  return is_background ? BACKGROUND : FOREGROUND;
+}
+
+std::vector<base::SchedulerWorkerPoolParams>
+GetDefaultBrowserSchedulerWorkerPoolParams() {
+  constexpr size_t kNumWorkerPoolsDefined =
+      sizeof(BrowserWorkerPoolsConfiguration) /
+      sizeof(SingleWorkerPoolConfiguration);
+  static_assert(kNumWorkerPoolsDefined == 4,
+                "Expected 4 worker pools in BrowserWorkerPoolsConfiguration");
+  BrowserWorkerPoolsConfiguration config;
+  constexpr size_t kSizeAssignedFields =
+      sizeof(config.background.threads) +
+      sizeof(config.background.detach_period) +
+      sizeof(config.background.standby_thread_policy);
+  static_assert(kSizeAssignedFields == sizeof(config.background),

[gab, 2016/12/07 15:53:06]

Does this work if the struct is padded by the compiler?

[robliao, 2016/12/07 18:39:19]

Padding does make this go out the window. I guess there isn't a way to easily
determine without indirecting all of the parameters through a pointer type (and
even then it's not a sure fire guarantee since padding is up to the compiler).

The only way seems to be to indirect all of the parameters through another
struct to add consistent padding, but that seems overkill.

Removed for now.

+                "Not all fields were assigned");
+#if defined(OS_ANDROID) || defined(OS_IOS)
+  config.background.standby_thread_policy = StandbyThreadPolicy::ONE;
+  config.background.threads =
+      base::RecommendedMaxNumberOfThreadsInPool(2, 8, 0.1, 0);
+  config.background.detach_period = base::TimeDelta::FromSeconds(30);
+
+  config.background_file_io.standby_thread_policy = StandbyThreadPolicy::ONE;
+  config.background_file_io.threads =
+      base::RecommendedMaxNumberOfThreadsInPool(2, 8, 0.1, 0);
+  config.background_file_io.detach_period = base::TimeDelta::FromSeconds(30);
+
+  config.foreground.standby_thread_policy = StandbyThreadPolicy::ONE;
+  config.foreground.threads =
+      base::RecommendedMaxNumberOfThreadsInPool(3, 8, 0.3, 0);
+  config.foreground.detach_period = base::TimeDelta::FromSeconds(30);
+
+  config.foreground_file_io.standby_thread_policy = StandbyThreadPolicy::ONE;
+  config.foreground_file_io.threads =
+      base::RecommendedMaxNumberOfThreadsInPool(3, 8, 0.3, 0);
+  config.foreground_file_io.detach_period = base::TimeDelta::FromSeconds(30);
+#else
+  config.background.standby_thread_policy = StandbyThreadPolicy::ONE;
+  config.background.threads =
+      base::RecommendedMaxNumberOfThreadsInPool(3, 8, 0.1, 0);
+  config.background.detach_period = base::TimeDelta::FromSeconds(30);
+
+  config.background_file_io.standby_thread_policy = StandbyThreadPolicy::ONE;
+  config.background_file_io.threads =
+      base::RecommendedMaxNumberOfThreadsInPool(3, 8, 0.1, 0);
+  config.background_file_io.detach_period = base::TimeDelta::FromSeconds(30);
+
+  config.foreground.standby_thread_policy = StandbyThreadPolicy::ONE;
+  config.foreground.threads =
+      base::RecommendedMaxNumberOfThreadsInPool(8, 32, 0.3, 0);
+  config.foreground.detach_period = base::TimeDelta::FromSeconds(30);
+
+  config.foreground_file_io.standby_thread_policy = StandbyThreadPolicy::ONE;
+  config.foreground_file_io.threads =
+      base::RecommendedMaxNumberOfThreadsInPool(8, 32, 0.3, 0);
+  config.foreground_file_io.detach_period = base::TimeDelta::FromSeconds(30);
+#endif
+  return BrowserWorkerPoolConfigurationToSchedulerWorkerPoolParams(config);
+}
+
+}  // namespace initialization
+}  // namespace task_scheduler_util