From d39c981d4fd9868e0bb7251fcce89911872e8e61 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Fri, 24 Apr 2020 10:32:15 -0700 Subject: [PATCH] Revert "Remove StringLess and src/core/lib/gprpp/map.h" --- BUILD | 1 + BUILD.gn | 1 + build_autogenerated.yaml | 1 + gRPC-C++.podspec | 2 + gRPC-Core.podspec | 2 + grpc.gemspec | 1 + package.xml | 1 + .../filters/client_channel/backend_metric.cc | 4 +- .../filters/client_channel/client_channel.cc | 47 +++++++++-------- .../health/health_check_client.cc | 7 ++- .../health/health_check_client.h | 6 +-- .../ext/filters/client_channel/lb_policy.h | 5 +- .../client_channel/resolver_result_parsing.cc | 15 +++--- .../client_channel/resolver_result_parsing.h | 6 +-- .../ext/filters/client_channel/subchannel.cc | 43 ++++++++-------- .../ext/filters/client_channel/subchannel.h | 18 +++---- .../client_channel/xds/xds_bootstrap.h | 1 + .../filters/client_channel/xds/xds_client.cc | 3 +- .../filters/client_channel/xds/xds_client.h | 2 +- .../client_channel/xds/xds_client_stats.h | 4 +- src/core/lib/channel/channelz.h | 2 +- src/core/lib/channel/channelz_registry.h | 3 +- src/core/lib/gprpp/map.h | 51 +++++++++++++++++++ src/core/lib/transport/connectivity_state.h | 3 +- .../client_channel/service_config_test.cc | 4 +- test/cpp/end2end/xds_end2end_test.cc | 2 +- tools/doxygen/Doxyfile.c++.internal | 1 + tools/doxygen/Doxyfile.core.internal | 1 + 28 files changed, 152 insertions(+), 85 deletions(-) create mode 100644 src/core/lib/gprpp/map.h diff --git a/BUILD b/BUILD index 8be2c685cd8..91cd3488408 100644 --- a/BUILD +++ b/BUILD @@ -551,6 +551,7 @@ grpc_cc_library( "src/core/lib/gprpp/global_config_generic.h", "src/core/lib/gprpp/host_port.h", "src/core/lib/gprpp/manual_constructor.h", + "src/core/lib/gprpp/map.h", "src/core/lib/gprpp/memory.h", "src/core/lib/gprpp/mpscq.h", "src/core/lib/gprpp/string_view.h", diff --git a/BUILD.gn b/BUILD.gn index 40dfa4bf46c..9ec801e373c 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -156,6 +156,7 @@ config("grpc_config") { "src/core/lib/gprpp/host_port.cc", "src/core/lib/gprpp/host_port.h", "src/core/lib/gprpp/manual_constructor.h", + "src/core/lib/gprpp/map.h", "src/core/lib/gprpp/memory.h", "src/core/lib/gprpp/mpscq.cc", "src/core/lib/gprpp/mpscq.h", diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 560904729ad..d01ff14cc58 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -299,6 +299,7 @@ libs: - src/core/lib/gprpp/global_config_generic.h - src/core/lib/gprpp/host_port.h - src/core/lib/gprpp/manual_constructor.h + - src/core/lib/gprpp/map.h - src/core/lib/gprpp/memory.h - src/core/lib/gprpp/mpscq.h - src/core/lib/gprpp/string_view.h diff --git a/gRPC-C++.podspec b/gRPC-C++.podspec index b94955f47a4..77e015d1d97 100644 --- a/gRPC-C++.podspec +++ b/gRPC-C++.podspec @@ -419,6 +419,7 @@ Pod::Spec.new do |s| 'src/core/lib/gprpp/global_config_generic.h', 'src/core/lib/gprpp/host_port.h', 'src/core/lib/gprpp/manual_constructor.h', + 'src/core/lib/gprpp/map.h', 'src/core/lib/gprpp/memory.h', 'src/core/lib/gprpp/mpscq.h', 'src/core/lib/gprpp/orphanable.h', @@ -868,6 +869,7 @@ Pod::Spec.new do |s| 'src/core/lib/gprpp/global_config_generic.h', 'src/core/lib/gprpp/host_port.h', 'src/core/lib/gprpp/manual_constructor.h', + 'src/core/lib/gprpp/map.h', 'src/core/lib/gprpp/memory.h', 'src/core/lib/gprpp/mpscq.h', 'src/core/lib/gprpp/orphanable.h', diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index d04fa0aaabc..a9c11d7d933 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -611,6 +611,7 @@ Pod::Spec.new do |s| 'src/core/lib/gprpp/host_port.cc', 'src/core/lib/gprpp/host_port.h', 'src/core/lib/gprpp/manual_constructor.h', + 'src/core/lib/gprpp/map.h', 'src/core/lib/gprpp/memory.h', 'src/core/lib/gprpp/mpscq.cc', 'src/core/lib/gprpp/mpscq.h', @@ -1224,6 +1225,7 @@ Pod::Spec.new do |s| 'src/core/lib/gprpp/global_config_generic.h', 'src/core/lib/gprpp/host_port.h', 'src/core/lib/gprpp/manual_constructor.h', + 'src/core/lib/gprpp/map.h', 'src/core/lib/gprpp/memory.h', 'src/core/lib/gprpp/mpscq.h', 'src/core/lib/gprpp/orphanable.h', diff --git a/grpc.gemspec b/grpc.gemspec index e2f6580651e..7042206375a 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -533,6 +533,7 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/gprpp/host_port.cc ) s.files += %w( src/core/lib/gprpp/host_port.h ) s.files += %w( src/core/lib/gprpp/manual_constructor.h ) + s.files += %w( src/core/lib/gprpp/map.h ) s.files += %w( src/core/lib/gprpp/memory.h ) s.files += %w( src/core/lib/gprpp/mpscq.cc ) s.files += %w( src/core/lib/gprpp/mpscq.h ) diff --git a/package.xml b/package.xml index c5294e90da6..862ea4f566c 100644 --- a/package.xml +++ b/package.xml @@ -513,6 +513,7 @@ + diff --git a/src/core/ext/filters/client_channel/backend_metric.cc b/src/core/ext/filters/client_channel/backend_metric.cc index dbc296a39d4..b36614f5b80 100644 --- a/src/core/ext/filters/client_channel/backend_metric.cc +++ b/src/core/ext/filters/client_channel/backend_metric.cc @@ -26,12 +26,12 @@ namespace grpc_core { namespace { template -std::map ParseMap( +std::map ParseMap( udpa_data_orca_v1_OrcaLoadReport* msg, EntryType** (*entry_func)(udpa_data_orca_v1_OrcaLoadReport*, size_t*), upb_strview (*key_func)(const EntryType*), double (*value_func)(const EntryType*), Arena* arena) { - std::map result; + std::map result; size_t size; const auto* const* entries = entry_func(msg, &size); for (size_t i = 0; i < size; ++i) { diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index 97fc16822ef..a66a3a615c7 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -26,7 +26,6 @@ #include #include -#include #include #include @@ -57,6 +56,7 @@ #include "src/core/lib/channel/status_util.h" #include "src/core/lib/gpr/string.h" #include "src/core/lib/gprpp/manual_constructor.h" +#include "src/core/lib/gprpp/map.h" #include "src/core/lib/gprpp/sync.h" #include "src/core/lib/iomgr/iomgr.h" #include "src/core/lib/iomgr/polling_entity.h" @@ -295,7 +295,7 @@ class ChannelData { RefCountedPtr subchannel_pool_; OrphanablePtr resolving_lb_policy_; ConnectivityStateTracker state_tracker_; - std::string health_check_service_name_; + grpc_core::UniquePtr health_check_service_name_; RefCountedPtr saved_service_config_; bool received_first_resolver_result_ = false; // The number of SubchannelWrapper instances referencing a given Subchannel. @@ -850,7 +850,7 @@ class CallData { class ChannelData::SubchannelWrapper : public SubchannelInterface { public: SubchannelWrapper(ChannelData* chand, Subchannel* subchannel, - std::string health_check_service_name) + grpc_core::UniquePtr health_check_service_name) : SubchannelInterface(&grpc_client_channel_routing_trace), chand_(chand), subchannel_(subchannel), @@ -897,7 +897,7 @@ class ChannelData::SubchannelWrapper : public SubchannelInterface { grpc_connectivity_state CheckConnectivityState() override { RefCountedPtr connected_subchannel; grpc_connectivity_state connectivity_state = - subchannel_->CheckConnectivityState(health_check_service_name_, + subchannel_->CheckConnectivityState(health_check_service_name_.get(), &connected_subchannel); MaybeUpdateConnectedSubchannel(std::move(connected_subchannel)); return connectivity_state; @@ -912,7 +912,9 @@ class ChannelData::SubchannelWrapper : public SubchannelInterface { Ref(DEBUG_LOCATION, "WatcherWrapper"), initial_state); subchannel_->WatchConnectivityState( - initial_state, health_check_service_name_, + initial_state, + grpc_core::UniquePtr( + gpr_strdup(health_check_service_name_.get())), RefCountedPtr( watcher_wrapper)); } @@ -921,7 +923,7 @@ class ChannelData::SubchannelWrapper : public SubchannelInterface { ConnectivityStateWatcherInterface* watcher) override { auto it = watcher_map_.find(watcher); GPR_ASSERT(it != watcher_map_.end()); - subchannel_->CancelConnectivityStateWatch(health_check_service_name_, + subchannel_->CancelConnectivityStateWatch(health_check_service_name_.get(), it->second); watcher_map_.erase(it); } @@ -934,13 +936,14 @@ class ChannelData::SubchannelWrapper : public SubchannelInterface { return subchannel_->channel_args(); } - void UpdateHealthCheckServiceName(std::string health_check_service_name) { + void UpdateHealthCheckServiceName( + grpc_core::UniquePtr health_check_service_name) { if (GRPC_TRACE_FLAG_ENABLED(grpc_client_channel_routing_trace)) { gpr_log(GPR_INFO, "chand=%p: subchannel wrapper %p: updating health check service " "name from \"%s\" to \"%s\"", - chand_, this, health_check_service_name_.c_str(), - health_check_service_name.c_str()); + chand_, this, health_check_service_name_.get(), + health_check_service_name.get()); } for (auto& p : watcher_map_) { WatcherWrapper*& watcher_wrapper = p.second; @@ -955,11 +958,13 @@ class ChannelData::SubchannelWrapper : public SubchannelInterface { // problem, we may be able to handle it by waiting for the new // watcher to report READY before we use it to replace the old one. WatcherWrapper* replacement = watcher_wrapper->MakeReplacement(); - subchannel_->CancelConnectivityStateWatch(health_check_service_name_, - watcher_wrapper); + subchannel_->CancelConnectivityStateWatch( + health_check_service_name_.get(), watcher_wrapper); watcher_wrapper = replacement; subchannel_->WatchConnectivityState( - replacement->last_seen_state(), health_check_service_name, + replacement->last_seen_state(), + grpc_core::UniquePtr( + gpr_strdup(health_check_service_name.get())), RefCountedPtr( replacement)); } @@ -1097,7 +1102,7 @@ class ChannelData::SubchannelWrapper : public SubchannelInterface { ChannelData* chand_; Subchannel* subchannel_; - std::string health_check_service_name_; + grpc_core::UniquePtr health_check_service_name_; // Maps from the address of the watcher passed to us by the LB policy // to the address of the WrapperWatcher that we passed to the underlying // subchannel. This is needed so that when the LB policy calls @@ -1260,9 +1265,10 @@ class ChannelData::ClientChannelControlHelper const grpc_channel_args& args) override { bool inhibit_health_checking = grpc_channel_arg_get_bool( grpc_channel_args_find(&args, GRPC_ARG_INHIBIT_HEALTH_CHECKING), false); - std::string health_check_service_name; + grpc_core::UniquePtr health_check_service_name; if (!inhibit_health_checking) { - health_check_service_name = chand_->health_check_service_name_; + health_check_service_name.reset( + gpr_strdup(chand_->health_check_service_name_.get())); } static const char* args_to_remove[] = { GRPC_ARG_INHIBIT_HEALTH_CHECKING, @@ -1457,7 +1463,7 @@ void ChannelData::UpdateStateAndPickerLocked( std::unique_ptr picker) { // Clean the control plane when entering IDLE. if (picker_ == nullptr) { - health_check_service_name_.clear(); + health_check_service_name_.reset(); saved_service_config_.reset(); received_first_resolver_result_ = false; } @@ -1700,15 +1706,16 @@ bool ChannelData::ProcessResolverResultLocked( } // Save health check service name. if (service_config != nullptr) { - chand->health_check_service_name_ = - parsed_service_config->health_check_service_name(); + chand->health_check_service_name_.reset( + gpr_strdup(parsed_service_config->health_check_service_name())); } else { - chand->health_check_service_name_.clear(); + chand->health_check_service_name_.reset(); } // Update health check service name used by existing subchannel wrappers. for (auto* subchannel_wrapper : chand->subchannel_wrappers_) { subchannel_wrapper->UpdateHealthCheckServiceName( - chand->health_check_service_name_); + grpc_core::UniquePtr( + gpr_strdup(chand->health_check_service_name_.get()))); } // Save service config. chand->saved_service_config_ = std::move(service_config); diff --git a/src/core/ext/filters/client_channel/health/health_check_client.cc b/src/core/ext/filters/client_channel/health/health_check_client.cc index ba1bc26d293..5c4aa986e80 100644 --- a/src/core/ext/filters/client_channel/health/health_check_client.cc +++ b/src/core/ext/filters/client_channel/health/health_check_client.cc @@ -44,7 +44,7 @@ TraceFlag grpc_health_check_client_trace(false, "health_check_client"); // HealthCheckClient::HealthCheckClient( - absl::string_view service_name, + const char* service_name, RefCountedPtr connected_subchannel, grpc_pollset_set* interested_parties, RefCountedPtr channelz_node, @@ -171,14 +171,13 @@ void HealthCheckClient::OnRetryTimer(void* arg, grpc_error* error) { namespace { -void EncodeRequest(absl::string_view service_name, +void EncodeRequest(const char* service_name, ManualConstructor* send_message) { upb::Arena arena; grpc_health_v1_HealthCheckRequest* request_struct = grpc_health_v1_HealthCheckRequest_new(arena.ptr()); grpc_health_v1_HealthCheckRequest_set_service( - request_struct, - upb_strview_make(service_name.data(), service_name.size())); + request_struct, upb_strview_makez(service_name)); size_t buf_length; char* buf = grpc_health_v1_HealthCheckRequest_serialize( request_struct, arena.ptr(), &buf_length); diff --git a/src/core/ext/filters/client_channel/health/health_check_client.h b/src/core/ext/filters/client_channel/health/health_check_client.h index 9b90e607631..f8b9ade5ab6 100644 --- a/src/core/ext/filters/client_channel/health/health_check_client.h +++ b/src/core/ext/filters/client_channel/health/health_check_client.h @@ -21,8 +21,6 @@ #include -#include "absl/strings/string_view.h" - #include #include @@ -46,7 +44,7 @@ namespace grpc_core { class HealthCheckClient : public InternallyRefCounted { public: - HealthCheckClient(absl::string_view service_name, + HealthCheckClient(const char* service_name, RefCountedPtr connected_subchannel, grpc_pollset_set* interested_parties, RefCountedPtr channelz_node, @@ -152,7 +150,7 @@ class HealthCheckClient : public InternallyRefCounted { void SetHealthStatusLocked(grpc_connectivity_state state, const char* reason); // Requires holding mu_. - absl::string_view service_name_; + const char* service_name_; // Do not own. RefCountedPtr connected_subchannel_; grpc_pollset_set* interested_parties_; // Do not own. RefCountedPtr channelz_node_; diff --git a/src/core/ext/filters/client_channel/lb_policy.h b/src/core/ext/filters/client_channel/lb_policy.h index 9cdd1d478f5..4f269e9d383 100644 --- a/src/core/ext/filters/client_channel/lb_policy.h +++ b/src/core/ext/filters/client_channel/lb_policy.h @@ -27,6 +27,7 @@ #include "src/core/ext/filters/client_channel/server_address.h" #include "src/core/ext/filters/client_channel/service_config.h" #include "src/core/ext/filters/client_channel/subchannel_interface.h" +#include "src/core/lib/gprpp/map.h" #include "src/core/lib/gprpp/orphanable.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/gprpp/string_view.h" @@ -92,11 +93,11 @@ class LoadBalancingPolicy : public InternallyRefCounted { /// Application-specific requests cost metrics. Metric names are /// determined by the application. Each value is an absolute cost /// (e.g. 3487 bytes of storage) associated with the request. - std::map request_cost; + std::map request_cost; /// Application-specific resource utilization metrics. Metric names /// are determined by the application. Each value is expressed as a /// fraction of total resources available. - std::map utilization; + std::map utilization; }; /// Interface for accessing per-call state. diff --git a/src/core/ext/filters/client_channel/resolver_result_parsing.cc b/src/core/ext/filters/client_channel/resolver_result_parsing.cc index 0c23269c66a..77ee5fad3cd 100644 --- a/src/core/ext/filters/client_channel/resolver_result_parsing.cc +++ b/src/core/ext/filters/client_channel/resolver_result_parsing.cc @@ -284,13 +284,13 @@ grpc_error* ParseRetryThrottling( return GRPC_ERROR_CREATE_FROM_VECTOR("retryPolicy", &error_list); } -std::string ParseHealthCheckConfig(const Json& field, grpc_error** error) { +const char* ParseHealthCheckConfig(const Json& field, grpc_error** error) { GPR_DEBUG_ASSERT(error != nullptr && *error == GRPC_ERROR_NONE); - std::string service_name; + const char* service_name = nullptr; if (field.type() != Json::Type::OBJECT) { *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( "field:healthCheckConfig error:should be of type object"); - return service_name; + return nullptr; } std::vector error_list; auto it = field.object_value().find("serviceName"); @@ -299,9 +299,12 @@ std::string ParseHealthCheckConfig(const Json& field, grpc_error** error) { error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( "field:serviceName error:should be of type string")); } else { - service_name = it->second.string_value(); + service_name = it->second.string_value().c_str(); } } + if (!error_list.empty()) { + return nullptr; + } *error = GRPC_ERROR_CREATE_FROM_VECTOR("field:healthCheckConfig", &error_list); return service_name; @@ -318,7 +321,7 @@ ClientChannelServiceConfigParser::ParseGlobalParams(const Json& json, std::string lb_policy_name; absl::optional retry_throttling; - std::string health_check_service_name; + const char* health_check_service_name = nullptr; // Parse LB config. auto it = json.object_value().find("loadBalancingConfig"); if (it != json.object_value().end()) { @@ -385,7 +388,7 @@ ClientChannelServiceConfigParser::ParseGlobalParams(const Json& json, if (*error == GRPC_ERROR_NONE) { return absl::make_unique( std::move(parsed_lb_config), std::move(lb_policy_name), - retry_throttling, std::move(health_check_service_name)); + retry_throttling, health_check_service_name); } return nullptr; } diff --git a/src/core/ext/filters/client_channel/resolver_result_parsing.h b/src/core/ext/filters/client_channel/resolver_result_parsing.h index c736a2ef91f..b38ae730708 100644 --- a/src/core/ext/filters/client_channel/resolver_result_parsing.h +++ b/src/core/ext/filters/client_channel/resolver_result_parsing.h @@ -49,7 +49,7 @@ class ClientChannelGlobalParsedConfig : public ServiceConfig::ParsedConfig { RefCountedPtr parsed_lb_config, std::string parsed_deprecated_lb_policy, const absl::optional& retry_throttling, - std::string health_check_service_name) + const char* health_check_service_name) : parsed_lb_config_(std::move(parsed_lb_config)), parsed_deprecated_lb_policy_(std::move(parsed_deprecated_lb_policy)), retry_throttling_(retry_throttling), @@ -67,7 +67,7 @@ class ClientChannelGlobalParsedConfig : public ServiceConfig::ParsedConfig { return parsed_deprecated_lb_policy_; } - const std::string& health_check_service_name() const { + const char* health_check_service_name() const { return health_check_service_name_; } @@ -75,7 +75,7 @@ class ClientChannelGlobalParsedConfig : public ServiceConfig::ParsedConfig { RefCountedPtr parsed_lb_config_; std::string parsed_deprecated_lb_policy_; absl::optional retry_throttling_; - std::string health_check_service_name_; + const char* health_check_service_name_; }; class ClientChannelMethodParsedConfig : public ServiceConfig::ParsedConfig { diff --git a/src/core/ext/filters/client_channel/subchannel.cc b/src/core/ext/filters/client_channel/subchannel.cc index 62d2eff4fa1..f350b2ec36a 100644 --- a/src/core/ext/filters/client_channel/subchannel.cc +++ b/src/core/ext/filters/client_channel/subchannel.cc @@ -424,10 +424,11 @@ void Subchannel::ConnectivityStateWatcherList::NotifyLocked( class Subchannel::HealthWatcherMap::HealthWatcher : public AsyncConnectivityStateWatcherInterface { public: - HealthWatcher(Subchannel* c, absl::string_view health_check_service_name, + HealthWatcher(Subchannel* c, + grpc_core::UniquePtr health_check_service_name, grpc_connectivity_state subchannel_state) : subchannel_(c), - health_check_service_name_(std::string(health_check_service_name)), + health_check_service_name_(std::move(health_check_service_name)), state_(subchannel_state == GRPC_CHANNEL_READY ? GRPC_CHANNEL_CONNECTING : subchannel_state) { GRPC_SUBCHANNEL_WEAK_REF(subchannel_, "health_watcher"); @@ -439,8 +440,8 @@ class Subchannel::HealthWatcherMap::HealthWatcher GRPC_SUBCHANNEL_WEAK_UNREF(subchannel_, "health_watcher"); } - absl::string_view health_check_service_name() const { - return health_check_service_name_; + const char* health_check_service_name() const { + return health_check_service_name_.get(); } grpc_connectivity_state state() const { return state_; } @@ -499,12 +500,12 @@ class Subchannel::HealthWatcherMap::HealthWatcher void StartHealthCheckingLocked() { GPR_ASSERT(health_check_client_ == nullptr); health_check_client_ = MakeOrphanable( - health_check_service_name_, subchannel_->connected_subchannel_, + health_check_service_name_.get(), subchannel_->connected_subchannel_, subchannel_->pollset_set_, subchannel_->channelz_node_, Ref()); } Subchannel* subchannel_; - std::string health_check_service_name_; + grpc_core::UniquePtr health_check_service_name_; OrphanablePtr health_check_client_; grpc_connectivity_state state_; ConnectivityStateWatcherList watcher_list_; @@ -516,17 +517,18 @@ class Subchannel::HealthWatcherMap::HealthWatcher void Subchannel::HealthWatcherMap::AddWatcherLocked( Subchannel* subchannel, grpc_connectivity_state initial_state, - absl::string_view health_check_service_name, + grpc_core::UniquePtr health_check_service_name, RefCountedPtr watcher) { // If the health check service name is not already present in the map, // add it. - auto it = map_.find(health_check_service_name); + auto it = map_.find(health_check_service_name.get()); HealthWatcher* health_watcher; if (it == map_.end()) { + const char* key = health_check_service_name.get(); auto w = MakeOrphanable( - subchannel, health_check_service_name, subchannel->state_); + subchannel, std::move(health_check_service_name), subchannel->state_); health_watcher = w.get(); - map_[w->health_check_service_name()] = std::move(w); + map_[key] = std::move(w); } else { health_watcher = it->second.get(); } @@ -535,7 +537,7 @@ void Subchannel::HealthWatcherMap::AddWatcherLocked( } void Subchannel::HealthWatcherMap::RemoveWatcherLocked( - absl::string_view health_check_service_name, + const char* health_check_service_name, ConnectivityStateWatcherInterface* watcher) { auto it = map_.find(health_check_service_name); GPR_ASSERT(it != map_.end()); @@ -553,7 +555,7 @@ void Subchannel::HealthWatcherMap::NotifyLocked(grpc_connectivity_state state) { grpc_connectivity_state Subchannel::HealthWatcherMap::CheckConnectivityStateLocked( - Subchannel* subchannel, absl::string_view health_check_service_name) { + Subchannel* subchannel, const char* health_check_service_name) { auto it = map_.find(health_check_service_name); if (it == map_.end()) { // If the health check service name is not found in the map, we're @@ -797,11 +799,11 @@ channelz::SubchannelNode* Subchannel::channelz_node() { } grpc_connectivity_state Subchannel::CheckConnectivityState( - absl::string_view health_check_service_name, + const char* health_check_service_name, RefCountedPtr* connected_subchannel) { MutexLock lock(&mu_); grpc_connectivity_state state; - if (health_check_service_name.empty()) { + if (health_check_service_name == nullptr) { state = state_; } else { state = health_watcher_map_.CheckConnectivityStateLocked( @@ -815,33 +817,34 @@ grpc_connectivity_state Subchannel::CheckConnectivityState( void Subchannel::WatchConnectivityState( grpc_connectivity_state initial_state, - absl::string_view health_check_service_name, + grpc_core::UniquePtr health_check_service_name, RefCountedPtr watcher) { MutexLock lock(&mu_); grpc_pollset_set* interested_parties = watcher->interested_parties(); if (interested_parties != nullptr) { grpc_pollset_set_add_pollset_set(pollset_set_, interested_parties); } - if (health_check_service_name.empty()) { + if (health_check_service_name == nullptr) { if (state_ != initial_state) { new AsyncWatcherNotifierLocked(watcher, this, state_); } watcher_list_.AddWatcherLocked(std::move(watcher)); } else { - health_watcher_map_.AddWatcherLocked( - this, initial_state, health_check_service_name, std::move(watcher)); + health_watcher_map_.AddWatcherLocked(this, initial_state, + std::move(health_check_service_name), + std::move(watcher)); } } void Subchannel::CancelConnectivityStateWatch( - absl::string_view health_check_service_name, + const char* health_check_service_name, ConnectivityStateWatcherInterface* watcher) { MutexLock lock(&mu_); grpc_pollset_set* interested_parties = watcher->interested_parties(); if (interested_parties != nullptr) { grpc_pollset_set_del_pollset_set(pollset_set_, interested_parties); } - if (health_check_service_name.empty()) { + if (health_check_service_name == nullptr) { watcher_list_.RemoveWatcherLocked(watcher); } else { health_watcher_map_.RemoveWatcherLocked(health_check_service_name, watcher); diff --git a/src/core/ext/filters/client_channel/subchannel.h b/src/core/ext/filters/client_channel/subchannel.h index bc45c28dd36..9478fa7340c 100644 --- a/src/core/ext/filters/client_channel/subchannel.h +++ b/src/core/ext/filters/client_channel/subchannel.h @@ -23,8 +23,6 @@ #include -#include "absl/strings/string_view.h" - #include "src/core/ext/filters/client_channel/client_channel_channelz.h" #include "src/core/ext/filters/client_channel/connector.h" #include "src/core/ext/filters/client_channel/subchannel_pool_interface.h" @@ -32,6 +30,7 @@ #include "src/core/lib/channel/channel_stack.h" #include "src/core/lib/gpr/time_precise.h" #include "src/core/lib/gprpp/arena.h" +#include "src/core/lib/gprpp/map.h" #include "src/core/lib/gprpp/ref_counted.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/gprpp/sync.h" @@ -252,7 +251,7 @@ class Subchannel { // service name. // If the return value is GRPC_CHANNEL_READY, also sets *connected_subchannel. grpc_connectivity_state CheckConnectivityState( - absl::string_view health_check_service_name, + const char* health_check_service_name, RefCountedPtr* connected_subchannel); // Starts watching the subchannel's connectivity state. @@ -265,12 +264,12 @@ class Subchannel { // destroyed or when CancelConnectivityStateWatch() is called. void WatchConnectivityState( grpc_connectivity_state initial_state, - absl::string_view health_check_service_name, + grpc_core::UniquePtr health_check_service_name, RefCountedPtr watcher); // Cancels a connectivity state watch. // If the watcher has already been destroyed, this is a no-op. - void CancelConnectivityStateWatch(absl::string_view health_check_service_name, + void CancelConnectivityStateWatch(const char* health_check_service_name, ConnectivityStateWatcherInterface* watcher); // Attempt to connect to the backend. Has no effect if already connected. @@ -334,24 +333,23 @@ class Subchannel { public: void AddWatcherLocked( Subchannel* subchannel, grpc_connectivity_state initial_state, - absl::string_view health_check_service_name, + grpc_core::UniquePtr health_check_service_name, RefCountedPtr watcher); - void RemoveWatcherLocked(absl::string_view health_check_service_name, + void RemoveWatcherLocked(const char* health_check_service_name, ConnectivityStateWatcherInterface* watcher); // Notifies the watcher when the subchannel's state changes. void NotifyLocked(grpc_connectivity_state state); grpc_connectivity_state CheckConnectivityStateLocked( - Subchannel* subchannel, absl::string_view health_check_service_name); + Subchannel* subchannel, const char* health_check_service_name); void ShutdownLocked(); private: class HealthWatcher; - // Key points to the health_check_service_name_ field in the value object. - std::map> map_; + std::map, StringLess> map_; }; class ConnectedSubchannelStateWatcher; diff --git a/src/core/ext/filters/client_channel/xds/xds_bootstrap.h b/src/core/ext/filters/client_channel/xds/xds_bootstrap.h index f3e49ba0e5a..13eff49f575 100644 --- a/src/core/ext/filters/client_channel/xds/xds_bootstrap.h +++ b/src/core/ext/filters/client_channel/xds/xds_bootstrap.h @@ -27,6 +27,7 @@ #include +#include "src/core/lib/gprpp/map.h" #include "src/core/lib/gprpp/memory.h" #include "src/core/lib/iomgr/error.h" #include "src/core/lib/json/json.h" diff --git a/src/core/ext/filters/client_channel/xds/xds_client.cc b/src/core/ext/filters/client_channel/xds/xds_client.cc index abfcc452526..9dc7b97773c 100644 --- a/src/core/ext/filters/client_channel/xds/xds_client.cc +++ b/src/core/ext/filters/client_channel/xds/xds_client.cc @@ -22,8 +22,6 @@ #include #include -#include - #include "absl/container/inlined_vector.h" #include "absl/strings/str_format.h" #include "absl/strings/str_join.h" @@ -47,6 +45,7 @@ #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/channel/channel_stack.h" #include "src/core/lib/gpr/string.h" +#include "src/core/lib/gprpp/map.h" #include "src/core/lib/gprpp/memory.h" #include "src/core/lib/gprpp/orphanable.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" diff --git a/src/core/ext/filters/client_channel/xds/xds_client.h b/src/core/ext/filters/client_channel/xds/xds_client.h index ccdffc5f417..267a0fe9e19 100644 --- a/src/core/ext/filters/client_channel/xds/xds_client.h +++ b/src/core/ext/filters/client_channel/xds/xds_client.h @@ -19,7 +19,6 @@ #include -#include #include #include "absl/types/optional.h" @@ -28,6 +27,7 @@ #include "src/core/ext/filters/client_channel/xds/xds_api.h" #include "src/core/ext/filters/client_channel/xds/xds_bootstrap.h" #include "src/core/ext/filters/client_channel/xds/xds_client_stats.h" +#include "src/core/lib/gprpp/map.h" #include "src/core/lib/gprpp/memory.h" #include "src/core/lib/gprpp/orphanable.h" #include "src/core/lib/gprpp/ref_counted.h" diff --git a/src/core/ext/filters/client_channel/xds/xds_client_stats.h b/src/core/ext/filters/client_channel/xds/xds_client_stats.h index 1539c9a4986..7a358d47b91 100644 --- a/src/core/ext/filters/client_channel/xds/xds_client_stats.h +++ b/src/core/ext/filters/client_channel/xds/xds_client_stats.h @@ -21,14 +21,12 @@ #include -#include - #include #include "src/core/lib/gprpp/atomic.h" +#include "src/core/lib/gprpp/map.h" #include "src/core/lib/gprpp/memory.h" #include "src/core/lib/gprpp/ref_counted.h" -#include "src/core/lib/gprpp/string_view.h" #include "src/core/lib/gprpp/sync.h" #include "src/core/lib/iomgr/exec_ctx.h" diff --git a/src/core/lib/channel/channelz.h b/src/core/lib/channel/channelz.h index 3bf2598414e..63502030966 100644 --- a/src/core/lib/channel/channelz.h +++ b/src/core/lib/channel/channelz.h @@ -23,7 +23,6 @@ #include -#include #include #include "absl/container/inlined_vector.h" @@ -32,6 +31,7 @@ #include "src/core/lib/gpr/time_precise.h" #include "src/core/lib/gprpp/atomic.h" #include "src/core/lib/gprpp/manual_constructor.h" +#include "src/core/lib/gprpp/map.h" #include "src/core/lib/gprpp/ref_counted.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/gprpp/sync.h" diff --git a/src/core/lib/channel/channelz_registry.h b/src/core/lib/channel/channelz_registry.h index ac6d442eff5..d777651c12f 100644 --- a/src/core/lib/channel/channelz_registry.h +++ b/src/core/lib/channel/channelz_registry.h @@ -23,10 +23,9 @@ #include -#include - #include "src/core/lib/channel/channel_trace.h" #include "src/core/lib/channel/channelz.h" +#include "src/core/lib/gprpp/map.h" #include "src/core/lib/gprpp/sync.h" namespace grpc_core { diff --git a/src/core/lib/gprpp/map.h b/src/core/lib/gprpp/map.h new file mode 100644 index 00000000000..f14f3f6fc84 --- /dev/null +++ b/src/core/lib/gprpp/map.h @@ -0,0 +1,51 @@ +/* + * + * Copyright 2017 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +#ifndef GRPC_CORE_LIB_GPRPP_MAP_H +#define GRPC_CORE_LIB_GPRPP_MAP_H + +#include + +#include + +#include + +#include "src/core/lib/gprpp/ref_counted_ptr.h" +#include "src/core/lib/gprpp/string_view.h" + +namespace grpc_core { + +struct StringLess { + bool operator()(const char* a, const char* b) const { + return strcmp(a, b) < 0; + } + bool operator()(const grpc_core::UniquePtr& a, + const grpc_core::UniquePtr& b) const { + return strcmp(a.get(), b.get()) < 0; + } + bool operator()(const StringView& a, const StringView& b) const { + const size_t min_size = std::min(a.size(), b.size()); + int c = strncmp(a.data(), b.data(), min_size); + if (c != 0) return c < 0; + return a.size() < b.size(); + } +}; + +} // namespace grpc_core + +#endif /* GRPC_CORE_LIB_GPRPP_MAP_H */ diff --git a/src/core/lib/transport/connectivity_state.h b/src/core/lib/transport/connectivity_state.h index ba9cc927e61..5ab62bed40c 100644 --- a/src/core/lib/transport/connectivity_state.h +++ b/src/core/lib/transport/connectivity_state.h @@ -21,12 +21,11 @@ #include -#include - #include #include "src/core/lib/debug/trace.h" #include "src/core/lib/gprpp/atomic.h" +#include "src/core/lib/gprpp/map.h" #include "src/core/lib/gprpp/orphanable.h" #include "src/core/lib/iomgr/closure.h" #include "src/core/lib/iomgr/exec_ctx.h" diff --git a/test/core/client_channel/service_config_test.cc b/test/core/client_channel/service_config_test.cc index b0f5a3ae70f..5cdb51341ab 100644 --- a/test/core/client_channel/service_config_test.cc +++ b/test/core/client_channel/service_config_test.cc @@ -913,8 +913,8 @@ TEST_F(ClientChannelParserTest, ValidHealthCheck) { static_cast( svc_cfg->GetGlobalParsedConfig(0)); ASSERT_NE(parsed_config, nullptr); - EXPECT_EQ(parsed_config->health_check_service_name(), - "health_check_service_name"); + EXPECT_STREQ(parsed_config->health_check_service_name(), + "health_check_service_name"); } TEST_F(ClientChannelParserTest, InvalidHealthCheckMultipleEntries) { diff --git a/test/cpp/end2end/xds_end2end_test.cc b/test/cpp/end2end/xds_end2end_test.cc index 3ec43c61e72..aa873490ca4 100644 --- a/test/cpp/end2end/xds_end2end_test.cc +++ b/test/cpp/end2end/xds_end2end_test.cc @@ -17,7 +17,6 @@ */ #include -#include #include #include #include @@ -48,6 +47,7 @@ #include "src/core/ext/filters/client_channel/xds/xds_api.h" #include "src/core/lib/gpr/env.h" #include "src/core/lib/gpr/tmpfile.h" +#include "src/core/lib/gprpp/map.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/gprpp/sync.h" #include "src/core/lib/iomgr/sockaddr.h" diff --git a/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index 36a25781429..c5d220fabf1 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -1496,6 +1496,7 @@ src/core/lib/gprpp/global_config_generic.h \ src/core/lib/gprpp/host_port.cc \ src/core/lib/gprpp/host_port.h \ src/core/lib/gprpp/manual_constructor.h \ +src/core/lib/gprpp/map.h \ src/core/lib/gprpp/memory.h \ src/core/lib/gprpp/mpscq.cc \ src/core/lib/gprpp/mpscq.h \ diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index dbbf9104c61..04f37228424 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -1307,6 +1307,7 @@ src/core/lib/gprpp/global_config_generic.h \ src/core/lib/gprpp/host_port.cc \ src/core/lib/gprpp/host_port.h \ src/core/lib/gprpp/manual_constructor.h \ +src/core/lib/gprpp/map.h \ src/core/lib/gprpp/memory.h \ src/core/lib/gprpp/mpscq.cc \ src/core/lib/gprpp/mpscq.h \