Exposing feature parameters for OptimizedRealtimeThreadingMac experiment

There are some concerns about increased audio rendering glitches in Beta finch experiment (go/mac-rt-thread-finch).
They may be caused either by the renderer side RT audio thread being descheduled since it's made "preemptible",
or by multiple RT audio threads competing for CPU since we relaxed their deadlines.
This CL is exposing the controls to allow for further experimentation around that as needed.

The plan is to backmerge this to M88 and to run an
 3-leg M88 experiment with "preemptible" set to false and true in experiment groups..
Meanwhile, there is 1% M87 stable experiment to confirm positive effects on Meet we've observed in M87 Beta.

Bug: 1132225
Change-Id: I7a79f5a0317814ff16252e0d88db8d61d5af599c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2546712
Commit-Queue: Olga Sharonova <olka@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Auto-Submit: Olga Sharonova <olka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#829231}

base/threading/platform_thread_mac.mm

@@ -69,13 +69,40 @@ void PlatformThread::SetName(const std::string& name) {
 const Feature kOptimizedRealtimeThreadingMac{"OptimizedRealtimeThreadingMac",
                                              FEATURE_DISABLED_BY_DEFAULT};
 
+// Fine-tuning optimized realt-time thread config:
+// Whether or not the thread should be preeptible.
+const FeatureParam<bool> kOptimizedRealtimeThreadingMacPreemptible{
+    &kOptimizedRealtimeThreadingMac, "preemptible", true};
+// Portion of the time quantum the thread is expected to be busy, (0, 1].
+const FeatureParam<double> kOptimizedRealtimeThreadingMacBusy{
+    &kOptimizedRealtimeThreadingMac, "busy", 0.5};
+// Maximum portion of the time quantum the thread is expected to be busy,
+// (kOptimizedRealtimeThreadingMacBusy, 1].
+const FeatureParam<double> kOptimizedRealtimeThreadingMacBusyLimit{
+    &kOptimizedRealtimeThreadingMac, "busy_limit", 1.0};
+
 namespace {
-// PlatformThread::SetCurrentThreadRealtimePeriodValue() doesn't query the state
-// of kOptimizedRealtimeThreadingMac feature directly because FeatureList
-// initialization is not always synchronized with
-// PlatformThread::SetCurrentThreadRealtimePeriodValue(). The initial value
-// should match the default state of kOptimizedRealtimeThreadingMac.
-std::atomic<bool> g_use_optimized_realtime_threading(false);
+
+struct TimeConstraints {
+  bool preemptible{kOptimizedRealtimeThreadingMacPreemptible.default_value};
+  double busy{kOptimizedRealtimeThreadingMacBusy.default_value};
+  double busy_limit{kOptimizedRealtimeThreadingMacBusyLimit.default_value};
+
+  static TimeConstraints ReadFromFeatureParams() {
+    double busy_limit = kOptimizedRealtimeThreadingMacBusyLimit.Get();
+    return TimeConstraints{
+        kOptimizedRealtimeThreadingMacPreemptible.Get(),
+        std::min(busy_limit, kOptimizedRealtimeThreadingMacBusy.Get()),
+        busy_limit};
+  }
+};
+
+// Use atomics to access FeatureList values when setting up a thread, since
+// there are cases when FeatureList initialization is not synchronized with
+// PlatformThread creation.
+std::atomic<bool> g_use_optimized_realtime_threading(
+    kOptimizedRealtimeThreadingMac.default_state == FEATURE_ENABLED_BY_DEFAULT);
+std::atomic<TimeConstraints> g_time_constraints;
 
 }  // namespace
 
@@ -86,6 +113,7 @@ void PlatformThread::InitializeOptimizedRealtimeThreadingFeature() {
   // tests that call this before initializing the FeatureList, only check the
   // state of the feature if the FeatureList is initialized.
   if (FeatureList::GetInstance()) {
+    g_time_constraints.store(TimeConstraints::ReadFromFeatureParams());
     g_use_optimized_realtime_threading.store(
         FeatureList::IsEnabled(kOptimizedRealtimeThreadingMac));
   }
@@ -123,11 +151,14 @@ thread_time_constraint_policy_data_t GetTimeConstraints(
     uint32_t abs_realtime_period =
         saturated_cast<uint32_t>(realtime_period.InNanoseconds() *
                                  (double(tb_info.denom) / tb_info.numer));
-
+    TimeConstraints config = g_time_constraints.load();
     time_constraints.period = abs_realtime_period;
-    time_constraints.computation = abs_realtime_period / 2;
-    time_constraints.constraint = abs_realtime_period;
-    time_constraints.preemptible = YES;
+    time_constraints.constraint = std::min(
+        abs_realtime_period, uint32_t(abs_realtime_period * config.busy_limit));
+    time_constraints.computation =
+        std::min(time_constraints.constraint,
+                 uint32_t(abs_realtime_period * config.busy));
+    time_constraints.preemptible = config.preemptible ? YES : NO;
     return time_constraints;
   }
 

base/threading/platform_thread_unittest.cc

@@ -25,6 +25,7 @@
 #include <mach/mach.h>
 #include <mach/mach_time.h>
 #include <mach/thread_policy.h>
+#include "base/metrics/field_trial_params.h"
 #include "base/time/time.h"
 #endif
 
@@ -468,9 +469,16 @@ class RealtimeTestThread : public FunctionTestThread {
           (static_cast<double>(tb_info.denom) / tb_info.numer));
 
       EXPECT_EQ(time_constraints.period, abs_realtime_period);
-      EXPECT_EQ(time_constraints.computation, abs_realtime_period / 2);
-      EXPECT_EQ(time_constraints.constraint, abs_realtime_period);
-      EXPECT_TRUE(time_constraints.preemptible);
+      EXPECT_EQ(
+          time_constraints.computation,
+          static_cast<uint32_t>(abs_realtime_period *
+                                kOptimizedRealtimeThreadingMacBusy.Get()));
+      EXPECT_EQ(
+          time_constraints.constraint,
+          static_cast<uint32_t>(abs_realtime_period *
+                                kOptimizedRealtimeThreadingMacBusyLimit.Get()));
+      EXPECT_EQ(time_constraints.preemptible,
+                kOptimizedRealtimeThreadingMacPreemptible.Get());
     } else {
       // Old-style empirical values.
       const double kTimeQuantum = 2.9;
@@ -494,7 +502,9 @@ class RealtimeTestThread : public FunctionTestThread {
   const TimeDelta realtime_period_;
 };
 
-class RealtimePlatformThreadTest : public testing::TestWithParam<TimeDelta> {
+class RealtimePlatformThreadTest
+    : public testing::TestWithParam<
+          std::tuple<bool, FieldTrialParams, TimeDelta>> {
  protected:
   void VerifyRealtimeConfig(TimeDelta period) {
     RealtimeTestThread thread(period);
@@ -512,29 +522,44 @@ class RealtimePlatformThreadTest : public testing::TestWithParam<TimeDelta> {
   }
 };
 
-TEST_P(RealtimePlatformThreadTest, RealtimeAudioConfigMacFeatureOn) {
+TEST_P(RealtimePlatformThreadTest, RealtimeAudioConfigMac) {
   test::ScopedFeatureList feature_list;
-  feature_list.InitAndEnableFeature(kOptimizedRealtimeThreadingMac);
-  PlatformThread::InitializeOptimizedRealtimeThreadingFeature();
-  VerifyRealtimeConfig(GetParam());
-}
+  if (std::get<0>(GetParam())) {
+    feature_list.InitAndEnableFeatureWithParameters(
+        kOptimizedRealtimeThreadingMac, std::get<1>(GetParam()));
+  } else {
+    feature_list.InitAndDisableFeature(kOptimizedRealtimeThreadingMac);
+  }
 
-TEST_P(RealtimePlatformThreadTest, RealtimeAudioConfigMacFeatureOff) {
-  test::ScopedFeatureList feature_list;
-  feature_list.InitAndDisableFeature(kOptimizedRealtimeThreadingMac);
   PlatformThread::InitializeOptimizedRealtimeThreadingFeature();
-  VerifyRealtimeConfig(GetParam());
+  VerifyRealtimeConfig(std::get<2>(GetParam()));
 }
 
-INSTANTIATE_TEST_SUITE_P(RealtimePlatformThreadTest,
-                         RealtimePlatformThreadTest,
-                         testing::Values(TimeDelta(),
-                                         TimeDelta::FromSeconds(256.0 / 48000),
-                                         TimeDelta::FromMilliseconds(5),
-                                         TimeDelta::FromMilliseconds(10),
-                                         TimeDelta::FromSeconds(1024.0 / 44100),
-                                         TimeDelta::FromSeconds(1024.0 /
-                                                                16000)));
+INSTANTIATE_TEST_SUITE_P(
+    RealtimePlatformThreadTest,
+    RealtimePlatformThreadTest,
+    testing::Combine(
+        testing::Bool(),
+        testing::Values(
+            FieldTrialParams{
+                {kOptimizedRealtimeThreadingMacPreemptible.name, "true"}},
+            FieldTrialParams{
+                {kOptimizedRealtimeThreadingMacPreemptible.name, "false"}},
+            FieldTrialParams{
+                {kOptimizedRealtimeThreadingMacBusy.name, "0.5"},
+                {kOptimizedRealtimeThreadingMacBusyLimit.name, "0.75"}},
+            FieldTrialParams{
+                {kOptimizedRealtimeThreadingMacBusy.name, "0.7"},
+                {kOptimizedRealtimeThreadingMacBusyLimit.name, "0.7"}},
+            FieldTrialParams{
+                {kOptimizedRealtimeThreadingMacBusy.name, "1.0"},
+                {kOptimizedRealtimeThreadingMacBusyLimit.name, "1.0"}}),
+        testing::Values(TimeDelta(),
+                        TimeDelta::FromSeconds(256.0 / 48000),
+                        TimeDelta::FromMilliseconds(5),
+                        TimeDelta::FromMilliseconds(10),
+                        TimeDelta::FromSeconds(1024.0 / 44100),
+                        TimeDelta::FromSeconds(1024.0 / 16000))));
 
 }  // namespace
 

base/threading/threading_features.h

@@ -14,6 +14,12 @@ struct Feature;
 
 #if defined(OS_APPLE)
 extern const BASE_EXPORT Feature kOptimizedRealtimeThreadingMac;
+extern const BASE_EXPORT FeatureParam<bool>
+    kOptimizedRealtimeThreadingMacPreemptible;
+extern const BASE_EXPORT FeatureParam<double>
+    kOptimizedRealtimeThreadingMacBusy;
+extern const BASE_EXPORT FeatureParam<double>
+    kOptimizedRealtimeThreadingMacBusyLimit;
 #endif
 
 }  // namespace base