From baa52808f1e6b4ef3fb62127e989519ad3417e27 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Fri, 19 Apr 2019 15:46:16 -0700 Subject: [PATCH] Use a single copy of CreateErrorFromVector from ServiceConfig --- .../filters/client_channel/client_channel.cc | 3 ++ .../client_channel/lb_policy/grpclb/grpclb.cc | 21 ++------------ .../client_channel/lb_policy/xds/xds.cc | 20 +------------ .../client_channel/resolver_result_parsing.cc | 28 ++++--------------- .../filters/client_channel/service_config.cc | 18 ------------ .../filters/client_channel/service_config.h | 18 ++++++++++++ .../message_size/message_size_parser.cc | 21 ++------------ 7 files changed, 31 insertions(+), 98 deletions(-) diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index a1029ef67a7..15fe9a8c70a 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -3080,6 +3080,9 @@ void CallData::ApplyServiceConfigToCallLocked(grpc_call_element* elem) { chand, this); } if (chand->service_config() != nullptr) { + // Store a ref to the service_config in CallData. Also, save pointers to the + // ServiceConfig and ServiceConfigObjectsVector (for this call) in the + // call_context so that all future filters can access it. service_config_ = chand->service_config(); call_context_[GRPC_SERVICE_CONFIG].value = &service_config_; const auto* method_params_vector_ptr = diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc index a4c28d8d431..d8e942e6570 100644 --- a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc +++ b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc @@ -1798,24 +1798,6 @@ void GrpcLb::CreateOrUpdateChildPolicyLocked() { policy_to_update->UpdateLocked(std::move(update_args)); } -// Consumes all the errors in the vector and forms a referencing error from -// them. If the vector is empty, return GRPC_ERROR_NONE. -template -grpc_error* CreateErrorFromVector(const char* desc, - InlinedVector* error_list) { - grpc_error* error = GRPC_ERROR_NONE; - if (error_list->size() != 0) { - error = GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING( - desc, error_list->data(), error_list->size()); - // Remove refs to all errors in error_list. - for (size_t i = 0; i < error_list->size(); i++) { - GRPC_ERROR_UNREF((*error_list)[i]); - } - error_list->clear(); - } - return error; -} - // // factory // @@ -1858,7 +1840,8 @@ class GrpcLbFactory : public LoadBalancingPolicyFactory { return UniquePtr( New(std::move(child_policy))); } else { - *error = CreateErrorFromVector("GrpcLb Parser", &error_list); + *error = + ServiceConfig::CreateErrorFromVector("GrpcLb Parser", &error_list); return nullptr; } } diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/xds.cc b/src/core/ext/filters/client_channel/lb_policy/xds/xds.cc index 7d222558f4d..7b9c2b320fd 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/xds.cc +++ b/src/core/ext/filters/client_channel/lb_policy/xds/xds.cc @@ -1652,24 +1652,6 @@ void XdsLb::LocalityMap::LocalityEntry::Helper::RequestReresolution() { } } -// Consumes all the errors in the vector and forms a referencing error from -// them. If the vector is empty, return GRPC_ERROR_NONE. -template -grpc_error* CreateErrorFromVector(const char* desc, - InlinedVector* error_list) { - grpc_error* error = GRPC_ERROR_NONE; - if (error_list->size() != 0) { - error = GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING( - desc, error_list->data(), error_list->size()); - // Remove refs to all errors in error_list. - for (size_t i = 0; i < error_list->size(); i++) { - GRPC_ERROR_UNREF((*error_list)[i]); - } - error_list->clear(); - } - return error; -} - // // factory // @@ -1743,7 +1725,7 @@ class XdsFactory : public LoadBalancingPolicyFactory { return UniquePtr(New( balancer_name, std::move(child_policy), std::move(fallback_policy))); } else { - *error = CreateErrorFromVector("Xds Parser", &error_list); + *error = ServiceConfig::CreateErrorFromVector("Xds Parser", &error_list); return nullptr; } } 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 1756917b391..dea8c915856 100644 --- a/src/core/ext/filters/client_channel/resolver_result_parsing.cc +++ b/src/core/ext/filters/client_channel/resolver_result_parsing.cc @@ -79,26 +79,6 @@ ProcessedResolverResult::ProcessedResolverResult( if (lb_policy_name_ == nullptr) ProcessLbPolicyName(*resolver_result); } -namespace { -// Consumes all the errors in the vector and forms a referencing error from -// them. If the vector is empty, return GRPC_ERROR_NONE. -template -grpc_error* CreateErrorFromVector(const char* desc, - InlinedVector* error_list) { - grpc_error* error = GRPC_ERROR_NONE; - if (error_list->size() != 0) { - error = GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING( - desc, error_list->data(), error_list->size()); - // Remove refs to all errors in error_list. - for (size_t i = 0; i < error_list->size(); i++) { - GRPC_ERROR_UNREF((*error_list)[i]); - } - error_list->clear(); - } - return error; -} -} // namespace - void ProcessedResolverResult::ProcessServiceConfig( const Resolver::Result& resolver_result, bool parse_retry) { if (service_config_ == nullptr) return; @@ -349,7 +329,7 @@ UniquePtr ParseRetryPolicy( return nullptr; } } - *error = CreateErrorFromVector("retryPolicy", &error_list); + *error = ServiceConfig::CreateErrorFromVector("retryPolicy", &error_list); return *error == GRPC_ERROR_NONE ? std::move(retry_policy) : nullptr; } @@ -504,7 +484,8 @@ ClientChannelServiceConfigParser::ParseGlobalParams(const grpc_json* json, } } } - *error = CreateErrorFromVector("Client channel global parser", &error_list); + *error = ServiceConfig::CreateErrorFromVector("Client channel global parser", + &error_list); if (*error == GRPC_ERROR_NONE) { return UniquePtr( New(std::move(parsed_lb_config), @@ -560,7 +541,8 @@ ClientChannelServiceConfigParser::ParsePerMethodParams(const grpc_json* json, } } } - *error = CreateErrorFromVector("Client channel parser", &error_list); + *error = ServiceConfig::CreateErrorFromVector("Client channel parser", + &error_list); if (*error == GRPC_ERROR_NONE) { return UniquePtr( New(timeout, wait_for_ready, diff --git a/src/core/ext/filters/client_channel/service_config.cc b/src/core/ext/filters/client_channel/service_config.cc index 9a246785820..c6c6af35a68 100644 --- a/src/core/ext/filters/client_channel/service_config.cc +++ b/src/core/ext/filters/client_channel/service_config.cc @@ -38,24 +38,6 @@ typedef InlinedVector, ServiceConfig::kNumPreallocatedParsers> ServiceConfigParserList; ServiceConfigParserList* g_registered_parsers; - -// Consumes all the errors in the vector and forms a referencing error from -// them. If the vector is empty, return GRPC_ERROR_NONE. -template -grpc_error* CreateErrorFromVector(const char* desc, - InlinedVector* error_list) { - grpc_error* error = GRPC_ERROR_NONE; - if (error_list->size() != 0) { - error = GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING( - desc, error_list->data(), error_list->size()); - // Remove refs to all errors in error_list. - for (size_t i = 0; i < error_list->size(); i++) { - GRPC_ERROR_UNREF((*error_list)[i]); - } - error_list->clear(); - } - return error; -} } // namespace RefCountedPtr ServiceConfig::Create(const char* json, diff --git a/src/core/ext/filters/client_channel/service_config.h b/src/core/ext/filters/client_channel/service_config.h index 28e482bf2b7..f45cab9ee7d 100644 --- a/src/core/ext/filters/client_channel/service_config.h +++ b/src/core/ext/filters/client_channel/service_config.h @@ -150,6 +150,24 @@ class ServiceConfig : public RefCounted { static void Shutdown(); + // Consumes all the errors in the vector and forms a referencing error from + // them. If the vector is empty, return GRPC_ERROR_NONE. + template + static grpc_error* CreateErrorFromVector( + const char* desc, InlinedVector* error_list) { + grpc_error* error = GRPC_ERROR_NONE; + if (error_list->size() != 0) { + error = GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING( + desc, error_list->data(), error_list->size()); + // Remove refs to all errors in error_list. + for (size_t i = 0; i < error_list->size(); i++) { + GRPC_ERROR_UNREF((*error_list)[i]); + } + error_list->clear(); + } + return error; + } + private: // So New() can call our private ctor. template diff --git a/src/core/ext/filters/message_size/message_size_parser.cc b/src/core/ext/filters/message_size/message_size_parser.cc index 534082ee6ff..a3444a984dd 100644 --- a/src/core/ext/filters/message_size/message_size_parser.cc +++ b/src/core/ext/filters/message_size/message_size_parser.cc @@ -22,24 +22,6 @@ namespace { size_t g_message_size_parser_index; - -// Consumes all the errors in the vector and forms a referencing error from -// them. If the vector is empty, return GRPC_ERROR_NONE. -template -grpc_error* CreateErrorFromVector( - const char* desc, grpc_core::InlinedVector* error_list) { - grpc_error* error = GRPC_ERROR_NONE; - if (error_list->size() != 0) { - error = GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING( - desc, error_list->data(), error_list->size()); - // Remove refs to all errors in error_list. - for (size_t i = 0; i < error_list->size(); i++) { - GRPC_ERROR_UNREF((*error_list)[i]); - } - error_list->clear(); - } - return error; -} } // namespace namespace grpc_core { @@ -85,7 +67,8 @@ UniquePtr MessageSizeParser::ParsePerMethodParams( } } if (!error_list.empty()) { - *error = CreateErrorFromVector("Message size parser", &error_list); + *error = ServiceConfig::CreateErrorFromVector("Message size parser", + &error_list); return nullptr; } return UniquePtr(New(