[client_channel] don't hop into WorkSerializer to unref ConfigSelector per-call (#34399)

Also fold the `client_channel_subchannel_wrapper_work_serializer_orphan`
experiment into the `work_serializer_dispatch` experiment.

bazel/experiments.bzl

@@ -63,7 +63,6 @@ EXPERIMENTS = {
                 "work_stealing",
             ],
             "cpp_lb_end2end_test": [
-                "client_channel_subchannel_wrapper_work_serializer_orphan",
                 "round_robin_delegate_to_pick_first",
                 "wrr_delegate_to_pick_first",
             ],
@@ -71,12 +70,10 @@ EXPERIMENTS = {
                 "lazier_stream_updates",
             ],
             "lb_unit_test": [
-                "client_channel_subchannel_wrapper_work_serializer_orphan",
                 "round_robin_delegate_to_pick_first",
                 "wrr_delegate_to_pick_first",
             ],
             "xds_end2end_test": [
-                "client_channel_subchannel_wrapper_work_serializer_orphan",
                 "round_robin_delegate_to_pick_first",
                 "wrr_delegate_to_pick_first",
             ],
@@ -128,7 +125,6 @@ EXPERIMENTS = {
                 "work_stealing",
             ],
             "cpp_lb_end2end_test": [
-                "client_channel_subchannel_wrapper_work_serializer_orphan",
                 "round_robin_delegate_to_pick_first",
                 "wrr_delegate_to_pick_first",
             ],
@@ -136,12 +132,10 @@ EXPERIMENTS = {
                 "lazier_stream_updates",
             ],
             "lb_unit_test": [
-                "client_channel_subchannel_wrapper_work_serializer_orphan",
                 "round_robin_delegate_to_pick_first",
                 "wrr_delegate_to_pick_first",
             ],
             "xds_end2end_test": [
-                "client_channel_subchannel_wrapper_work_serializer_orphan",
                 "round_robin_delegate_to_pick_first",
                 "wrr_delegate_to_pick_first",
             ],
@@ -203,7 +197,6 @@ EXPERIMENTS = {
                 "work_stealing",
             ],
             "cpp_lb_end2end_test": [
-                "client_channel_subchannel_wrapper_work_serializer_orphan",
                 "round_robin_delegate_to_pick_first",
                 "wrr_delegate_to_pick_first",
             ],
@@ -211,12 +204,10 @@ EXPERIMENTS = {
                 "lazier_stream_updates",
             ],
             "lb_unit_test": [
-                "client_channel_subchannel_wrapper_work_serializer_orphan",
                 "round_robin_delegate_to_pick_first",
                 "wrr_delegate_to_pick_first",
             ],
             "xds_end2end_test": [
-                "client_channel_subchannel_wrapper_work_serializer_orphan",
                 "round_robin_delegate_to_pick_first",
                 "wrr_delegate_to_pick_first",
             ],

src/core/BUILD

@@ -5531,6 +5531,7 @@ grpc_cc_library(
         "channel_fwd",
         "context",
         "dual_ref_counted",
+        "experiments",
         "grpc_lb_policy_ring_hash",
         "grpc_resolver_xds_header",
         "grpc_service_config",

src/core/ext/filters/client_channel/client_channel.cc

@@ -650,7 +650,7 @@ class ClientChannel::SubchannelWrapper : public SubchannelInterface {
               "chand=%p: destroying subchannel wrapper %p for subchannel %p",
               chand_, this, subchannel_.get());
     }
-    if (!IsClientChannelSubchannelWrapperWorkSerializerOrphanEnabled()) {
+    if (!IsWorkSerializerDispatchEnabled()) {
       chand_->subchannel_wrappers_.erase(this);
       if (chand_->channelz_node_ != nullptr) {
         auto* subchannel_node = subchannel_->channelz_node();
@@ -670,9 +670,7 @@ class ClientChannel::SubchannelWrapper : public SubchannelInterface {
   }
 
   void Orphan() override {
-    if (!IsClientChannelSubchannelWrapperWorkSerializerOrphanEnabled()) {
-      return;
-    }
+    if (!IsWorkSerializerDispatchEnabled()) return;
     // Make sure we clean up the channel's subchannel maps inside the
     // WorkSerializer.
     // Ref held by callback.
@@ -766,7 +764,7 @@ class ClientChannel::SubchannelWrapper : public SubchannelInterface {
         : watcher_(std::move(watcher)), parent_(std::move(parent)) {}
 
     ~WatcherWrapper() override {
-      if (!IsClientChannelSubchannelWrapperWorkSerializerOrphanEnabled()) {
+      if (!IsWorkSerializerDispatchEnabled()) {
         auto* parent = parent_.release();  // ref owned by lambda
         parent->chand_->work_serializer_->Run(
             [parent]() ABSL_EXCLUSIVE_LOCKS_REQUIRED(
@@ -2114,7 +2112,7 @@ absl::optional<absl::Status> ClientChannel::CallData::CheckResolution(
   // We have a result.  Apply service config to call.
   grpc_error_handle error = ApplyServiceConfigToCallLocked(config_selector);
   // ConfigSelector must be unreffed inside the WorkSerializer.
-  if (config_selector.ok()) {
+  if (!IsWorkSerializerDispatchEnabled() && config_selector.ok()) {
     chand()->work_serializer_->Run(
         [config_selector = std::move(*config_selector)]() mutable {
           config_selector.reset();
@@ -2824,9 +2822,7 @@ absl::optional<absl::Status> ClientChannel::LoadBalancedCall::PickSubchannel(
   // We need to unref pickers in the WorkSerializer.
   std::vector<RefCountedPtr<LoadBalancingPolicy::SubchannelPicker>> pickers;
   auto cleanup = absl::MakeCleanup([&]() {
-    if (IsClientChannelSubchannelWrapperWorkSerializerOrphanEnabled()) {
-      return;
-    }
+    if (IsWorkSerializerDispatchEnabled()) return;
     chand_->work_serializer_->Run(
         [pickers = std::move(pickers)]() mutable {
           for (auto& picker : pickers) {
@@ -2837,7 +2833,7 @@ absl::optional<absl::Status> ClientChannel::LoadBalancedCall::PickSubchannel(
   });
   absl::AnyInvocable<void(RefCountedPtr<LoadBalancingPolicy::SubchannelPicker>)>
       set_picker;
-  if (!IsClientChannelSubchannelWrapperWorkSerializerOrphanEnabled()) {
+  if (!IsWorkSerializerDispatchEnabled()) {
     set_picker =
         [&](RefCountedPtr<LoadBalancingPolicy::SubchannelPicker> picker) {
           pickers.emplace_back(std::move(picker));
@@ -2877,7 +2873,7 @@ absl::optional<absl::Status> ClientChannel::LoadBalancedCall::PickSubchannel(
                   "chand=%p lb_call=%p: pick not complete, but picker changed",
                   chand_, this);
         }
-        if (IsClientChannelSubchannelWrapperWorkSerializerOrphanEnabled()) {
+        if (IsWorkSerializerDispatchEnabled()) {
           // Don't unref until after we release the mutex.
           old_picker = std::move(pickers.back());
         }

src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc

@@ -320,7 +320,7 @@ class GrpcLb : public LoadBalancingPolicy {
           client_stats_(std::move(client_stats)) {}
 
     void Orphan() override {
-      if (!IsClientChannelSubchannelWrapperWorkSerializerOrphanEnabled()) {
+      if (!IsWorkSerializerDispatchEnabled()) {
         if (!lb_policy_->shutting_down_) {
           lb_policy_->CacheDeletedSubchannelLocked(wrapped_subchannel());
         }

src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.cc

@@ -139,7 +139,7 @@ class OutlierDetectionLb : public LoadBalancingPolicy {
     }
 
     void Orphan() override {
-      if (!IsClientChannelSubchannelWrapperWorkSerializerOrphanEnabled()) {
+      if (!IsWorkSerializerDispatchEnabled()) {
         if (subchannel_state_ != nullptr) {
           subchannel_state_->RemoveSubchannel(this);
         }

src/core/ext/filters/client_channel/lb_policy/weighted_round_robin/weighted_round_robin.cc

@@ -626,7 +626,7 @@ void OldWeightedRoundRobin::Picker::BuildSchedulerAndStartTimerLocked() {
             self->BuildSchedulerAndStartTimerLocked();
           }
         }
-        if (!IsClientChannelSubchannelWrapperWorkSerializerOrphanEnabled()) {
+        if (!IsWorkSerializerDispatchEnabled()) {
           // Release the picker ref inside the WorkSerializer.
           work_serializer->Run([self = std::move(self)]() {}, DEBUG_LOCATION);
           return;
@@ -1463,7 +1463,7 @@ void WeightedRoundRobin::Picker::BuildSchedulerAndStartTimerLocked() {
             self->BuildSchedulerAndStartTimerLocked();
           }
         }
-        if (!IsClientChannelSubchannelWrapperWorkSerializerOrphanEnabled()) {
+        if (!IsWorkSerializerDispatchEnabled()) {
           // Release the picker ref inside the WorkSerializer.
           work_serializer->Run([self = std::move(self)]() {}, DEBUG_LOCATION);
           return;

src/core/ext/filters/client_channel/lb_policy/xds/xds_override_host.cc

@@ -719,7 +719,7 @@ void XdsOverrideHostLb::SubchannelWrapper::UpdateConnectivityState(
 }
 
 void XdsOverrideHostLb::SubchannelWrapper::Orphan() {
-  if (!IsClientChannelSubchannelWrapperWorkSerializerOrphanEnabled()) {
+  if (!IsWorkSerializerDispatchEnabled()) {
     key_.reset();
     wrapped_subchannel()->CancelConnectivityStateWatch(watcher_);
     return;

src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc

@@ -74,6 +74,7 @@
 #include "src/core/lib/channel/status_util.h"
 #include "src/core/lib/config/core_configuration.h"
 #include "src/core/lib/debug/trace.h"
+#include "src/core/lib/experiments/experiments.h"
 #include "src/core/lib/gprpp/debug_location.h"
 #include "src/core/lib/gprpp/dual_ref_counted.h"
 #include "src/core/lib/gprpp/match.h"
@@ -696,7 +697,15 @@ XdsResolver::XdsConfigSelector::~XdsConfigSelector() {
             resolver_.get(), this);
   }
   route_config_data_.reset();
-  resolver_->MaybeRemoveUnusedClusters();
+  if (!IsWorkSerializerDispatchEnabled()) {
+    resolver_->MaybeRemoveUnusedClusters();
+    return;
+  }
+  resolver_->work_serializer_->Run(
+      [resolver = std::move(resolver_)]() {
+        resolver->MaybeRemoveUnusedClusters();
+      },
+      DEBUG_LOCATION);
 }
 
 absl::optional<uint64_t> HeaderHashHelper(

src/core/lib/experiments/experiments.cc

@@ -114,13 +114,6 @@ const char* const description_wrr_delegate_to_pick_first =
     "Change WRR code to delegate to pick_first as per dualstack backend "
     "design.";
 const char* const additional_constraints_wrr_delegate_to_pick_first = "{}";
-const char* const
-    description_client_channel_subchannel_wrapper_work_serializer_orphan =
-        "Client channel subchannel wrapper hops into WorkSerializer at "
-        "Orphan() time, rather than requiring callers to do it.";
-const char* const
-    additional_constraints_client_channel_subchannel_wrapper_work_serializer_orphan =
-        "{}";
 const char* const description_combiner_offload_to_event_engine =
     "Offload Combiner work onto the EventEngine instead of the Executor.";
 const char* const additional_constraints_combiner_offload_to_event_engine =
@@ -183,10 +176,6 @@ const ExperimentMetadata g_experiment_metadata[] = {
      additional_constraints_round_robin_delegate_to_pick_first, true, true},
     {"wrr_delegate_to_pick_first", description_wrr_delegate_to_pick_first,
      additional_constraints_wrr_delegate_to_pick_first, true, true},
-    {"client_channel_subchannel_wrapper_work_serializer_orphan",
-     description_client_channel_subchannel_wrapper_work_serializer_orphan,
-     additional_constraints_client_channel_subchannel_wrapper_work_serializer_orphan,
-     true, true},
     {"combiner_offload_to_event_engine",
      description_combiner_offload_to_event_engine,
      additional_constraints_combiner_offload_to_event_engine, true, true},
@@ -288,13 +277,6 @@ const char* const description_wrr_delegate_to_pick_first =
     "Change WRR code to delegate to pick_first as per dualstack backend "
     "design.";
 const char* const additional_constraints_wrr_delegate_to_pick_first = "{}";
-const char* const
-    description_client_channel_subchannel_wrapper_work_serializer_orphan =
-        "Client channel subchannel wrapper hops into WorkSerializer at "
-        "Orphan() time, rather than requiring callers to do it.";
-const char* const
-    additional_constraints_client_channel_subchannel_wrapper_work_serializer_orphan =
-        "{}";
 const char* const description_combiner_offload_to_event_engine =
     "Offload Combiner work onto the EventEngine instead of the Executor.";
 const char* const additional_constraints_combiner_offload_to_event_engine =
@@ -357,10 +339,6 @@ const ExperimentMetadata g_experiment_metadata[] = {
      additional_constraints_round_robin_delegate_to_pick_first, true, true},
     {"wrr_delegate_to_pick_first", description_wrr_delegate_to_pick_first,
      additional_constraints_wrr_delegate_to_pick_first, true, true},
-    {"client_channel_subchannel_wrapper_work_serializer_orphan",
-     description_client_channel_subchannel_wrapper_work_serializer_orphan,
-     additional_constraints_client_channel_subchannel_wrapper_work_serializer_orphan,
-     true, true},
     {"combiner_offload_to_event_engine",
      description_combiner_offload_to_event_engine,
      additional_constraints_combiner_offload_to_event_engine, true, true},
@@ -462,13 +440,6 @@ const char* const description_wrr_delegate_to_pick_first =
     "Change WRR code to delegate to pick_first as per dualstack backend "
     "design.";
 const char* const additional_constraints_wrr_delegate_to_pick_first = "{}";
-const char* const
-    description_client_channel_subchannel_wrapper_work_serializer_orphan =
-        "Client channel subchannel wrapper hops into WorkSerializer at "
-        "Orphan() time, rather than requiring callers to do it.";
-const char* const
-    additional_constraints_client_channel_subchannel_wrapper_work_serializer_orphan =
-        "{}";
 const char* const description_combiner_offload_to_event_engine =
     "Offload Combiner work onto the EventEngine instead of the Executor.";
 const char* const additional_constraints_combiner_offload_to_event_engine =
@@ -531,10 +502,6 @@ const ExperimentMetadata g_experiment_metadata[] = {
      additional_constraints_round_robin_delegate_to_pick_first, true, true},
     {"wrr_delegate_to_pick_first", description_wrr_delegate_to_pick_first,
      additional_constraints_wrr_delegate_to_pick_first, true, true},
-    {"client_channel_subchannel_wrapper_work_serializer_orphan",
-     description_client_channel_subchannel_wrapper_work_serializer_orphan,
-     additional_constraints_client_channel_subchannel_wrapper_work_serializer_orphan,
-     true, true},
     {"combiner_offload_to_event_engine",
      description_combiner_offload_to_event_engine,
      additional_constraints_combiner_offload_to_event_engine, true, true},

src/core/lib/experiments/experiments.h

@@ -88,10 +88,6 @@ inline bool IsJitterMaxIdleEnabled() { return true; }
 inline bool IsRoundRobinDelegateToPickFirstEnabled() { return true; }
 #define GRPC_EXPERIMENT_IS_INCLUDED_WRR_DELEGATE_TO_PICK_FIRST
 inline bool IsWrrDelegateToPickFirstEnabled() { return true; }
-#define GRPC_EXPERIMENT_IS_INCLUDED_CLIENT_CHANNEL_SUBCHANNEL_WRAPPER_WORK_SERIALIZER_ORPHAN
-inline bool IsClientChannelSubchannelWrapperWorkSerializerOrphanEnabled() {
-  return true;
-}
 #define GRPC_EXPERIMENT_IS_INCLUDED_COMBINER_OFFLOAD_TO_EVENT_ENGINE
 inline bool IsCombinerOffloadToEventEngineEnabled() { return true; }
 
@@ -127,10 +123,6 @@ inline bool IsJitterMaxIdleEnabled() { return true; }
 inline bool IsRoundRobinDelegateToPickFirstEnabled() { return true; }
 #define GRPC_EXPERIMENT_IS_INCLUDED_WRR_DELEGATE_TO_PICK_FIRST
 inline bool IsWrrDelegateToPickFirstEnabled() { return true; }
-#define GRPC_EXPERIMENT_IS_INCLUDED_CLIENT_CHANNEL_SUBCHANNEL_WRAPPER_WORK_SERIALIZER_ORPHAN
-inline bool IsClientChannelSubchannelWrapperWorkSerializerOrphanEnabled() {
-  return true;
-}
 #define GRPC_EXPERIMENT_IS_INCLUDED_COMBINER_OFFLOAD_TO_EVENT_ENGINE
 inline bool IsCombinerOffloadToEventEngineEnabled() { return true; }
 
@@ -166,10 +158,6 @@ inline bool IsJitterMaxIdleEnabled() { return true; }
 inline bool IsRoundRobinDelegateToPickFirstEnabled() { return true; }
 #define GRPC_EXPERIMENT_IS_INCLUDED_WRR_DELEGATE_TO_PICK_FIRST
 inline bool IsWrrDelegateToPickFirstEnabled() { return true; }
-#define GRPC_EXPERIMENT_IS_INCLUDED_CLIENT_CHANNEL_SUBCHANNEL_WRAPPER_WORK_SERIALIZER_ORPHAN
-inline bool IsClientChannelSubchannelWrapperWorkSerializerOrphanEnabled() {
-  return true;
-}
 #define GRPC_EXPERIMENT_IS_INCLUDED_COMBINER_OFFLOAD_TO_EVENT_ENGINE
 inline bool IsCombinerOffloadToEventEngineEnabled() { return true; }
 #endif
@@ -201,7 +189,6 @@ enum ExperimentIds {
   kExperimentIdJitterMaxIdle,
   kExperimentIdRoundRobinDelegateToPickFirst,
   kExperimentIdWrrDelegateToPickFirst,
-  kExperimentIdClientChannelSubchannelWrapperWorkSerializerOrphan,
   kExperimentIdCombinerOffloadToEventEngine,
   kNumExperiments
 };
@@ -305,11 +292,6 @@ inline bool IsRoundRobinDelegateToPickFirstEnabled() {
 inline bool IsWrrDelegateToPickFirstEnabled() {
   return IsExperimentEnabled(kExperimentIdWrrDelegateToPickFirst);
 }
-#define GRPC_EXPERIMENT_IS_INCLUDED_CLIENT_CHANNEL_SUBCHANNEL_WRAPPER_WORK_SERIALIZER_ORPHAN
-inline bool IsClientChannelSubchannelWrapperWorkSerializerOrphanEnabled() {
-  return IsExperimentEnabled(
-      kExperimentIdClientChannelSubchannelWrapperWorkSerializerOrphan);
-}
 #define GRPC_EXPERIMENT_IS_INCLUDED_COMBINER_OFFLOAD_TO_EVENT_ENGINE
 inline bool IsCombinerOffloadToEventEngineEnabled() {
   return IsExperimentEnabled(kExperimentIdCombinerOffloadToEventEngine);

src/core/lib/experiments/experiments.yaml

@@ -200,13 +200,6 @@
   expiry: 2023/11/15
   owner: roth@google.com
   test_tags: ["lb_unit_test", "cpp_lb_end2end_test", "xds_end2end_test"]
-- name: client_channel_subchannel_wrapper_work_serializer_orphan
-  description:
-    Client channel subchannel wrapper hops into WorkSerializer at
-    Orphan() time, rather than requiring callers to do it.
-  expiry: 2023/11/15
-  owner: roth@google.com
-  test_tags: ["lb_unit_test", "cpp_lb_end2end_test", "xds_end2end_test"]
 - name: combiner_offload_to_event_engine
   description:
     Offload Combiner work onto the EventEngine instead of the Executor.

src/core/lib/experiments/rollouts.yaml

@@ -98,7 +98,5 @@
   default: true
 - name: wrr_delegate_to_pick_first
   default: true
-- name: client_channel_subchannel_wrapper_work_serializer_orphan
-  default: true
 - name: combiner_offload_to_event_engine
   default: true