diff --git a/include/grpcpp/impl/codegen/call.h b/include/grpcpp/impl/codegen/call.h index a5e930aaa5c..e94adada940 100644 --- a/include/grpcpp/impl/codegen/call.h +++ b/include/grpcpp/impl/codegen/call.h @@ -50,8 +50,6 @@ namespace internal { class Call; class CallHook; -const char kBinaryErrorDetailsKey[] = "grpc-status-details-bin"; - // TODO(yangg) if the map is changed before we send, the pointers will be a // mess. Make sure it does not happen. inline grpc_metadata* FillMetadataArray( @@ -531,7 +529,6 @@ class CallOpRecvInitialMetadata { void FinishOp(bool* status) { if (metadata_map_ == nullptr) return; - metadata_map_->FillMap(); metadata_map_ = nullptr; } @@ -566,13 +563,7 @@ class CallOpClientRecvStatus { void FinishOp(bool* status) { if (recv_status_ == nullptr) return; - metadata_map_->FillMap(); - grpc::string binary_error_details; - auto iter = metadata_map_->map()->find(kBinaryErrorDetailsKey); - if (iter != metadata_map_->map()->end()) { - binary_error_details = - grpc::string(iter->second.begin(), iter->second.length()); - } + grpc::string binary_error_details = metadata_map_->GetBinaryErrorDetails(); *recv_status_ = Status(static_cast(status_code_), GRPC_SLICE_IS_EMPTY(error_message_) diff --git a/include/grpcpp/impl/codegen/client_context.h b/include/grpcpp/impl/codegen/client_context.h index 9dda4c7fac8..cf1d9018885 100644 --- a/include/grpcpp/impl/codegen/client_context.h +++ b/include/grpcpp/impl/codegen/client_context.h @@ -202,6 +202,7 @@ class ClientContext { const std::multimap& GetServerInitialMetadata() const { GPR_CODEGEN_ASSERT(initial_metadata_received_); + recv_initial_metadata_.FillMap(); return *recv_initial_metadata_.map(); } @@ -214,6 +215,7 @@ class ClientContext { const std::multimap& GetServerTrailingMetadata() const { // TODO(yangg) check finished + trailing_metadata_.FillMap(); return *trailing_metadata_.map(); } @@ -425,8 +427,8 @@ class ClientContext { mutable std::shared_ptr auth_context_; struct census_context* census_context_; std::multimap send_initial_metadata_; - internal::MetadataMap recv_initial_metadata_; - internal::MetadataMap trailing_metadata_; + mutable internal::MetadataMap recv_initial_metadata_; + mutable internal::MetadataMap trailing_metadata_; grpc_call* propagate_from_call_; PropagationOptions propagation_options_; diff --git a/include/grpcpp/impl/codegen/metadata_map.h b/include/grpcpp/impl/codegen/metadata_map.h index 0866539d886..6e0c2a96372 100644 --- a/include/grpcpp/impl/codegen/metadata_map.h +++ b/include/grpcpp/impl/codegen/metadata_map.h @@ -19,11 +19,15 @@ #ifndef GRPCPP_IMPL_CODEGEN_METADATA_MAP_H #define GRPCPP_IMPL_CODEGEN_METADATA_MAP_H +#include #include namespace grpc { namespace internal { + +const char kBinaryErrorDetailsKey[] = "grpc-status-details-bin"; + class MetadataMap { public: MetadataMap() { memset(&arr_, 0, sizeof(arr_)); } @@ -32,7 +36,31 @@ class MetadataMap { g_core_codegen_interface->grpc_metadata_array_destroy(&arr_); } + grpc::string GetBinaryErrorDetails() { + // if filled, extract from the multimap for O(log(n)) + if (filled) { + auto iter = map_.find(kBinaryErrorDetailsKey); + if (iter != map_.end()) { + return grpc::string(iter->second.begin(), iter->second.length()); + } + } + // if not yet filled, take the O(n) lookup to avoid allocating the + // multimap until it is requested. + else { + for (size_t i = 0; i < arr_.count; i++) { + if (grpc_slice_str_cmp(arr_.metadata[i].key, kBinaryErrorDetailsKey)) { + return grpc::string(reinterpret_cast( + GRPC_SLICE_START_PTR(arr_.metadata[i].value)), + GRPC_SLICE_LENGTH(arr_.metadata[i].value)); + } + } + } + return grpc::string(); + } + void FillMap() { + if (filled) return; + filled = true; for (size_t i = 0; i < arr_.count; i++) { // TODO(yangg) handle duplicates? map_.insert(std::pair( @@ -48,6 +76,7 @@ class MetadataMap { grpc_metadata_array* arr() { return &arr_; } private: + bool filled = false; grpc_metadata_array arr_; std::multimap map_; }; diff --git a/include/grpcpp/impl/codegen/server_context.h b/include/grpcpp/impl/codegen/server_context.h index 6314364db67..506c51a5d96 100644 --- a/include/grpcpp/impl/codegen/server_context.h +++ b/include/grpcpp/impl/codegen/server_context.h @@ -169,6 +169,7 @@ class ServerContext { /// \return A multimap of initial metadata key-value pairs from the server. const std::multimap& client_metadata() const { + client_metadata_.FillMap(); return *client_metadata_.map(); } @@ -294,7 +295,7 @@ class ServerContext { CompletionQueue* cq_; bool sent_initial_metadata_; mutable std::shared_ptr auth_context_; - internal::MetadataMap client_metadata_; + mutable internal::MetadataMap client_metadata_; std::multimap initial_metadata_; std::multimap trailing_metadata_; diff --git a/src/cpp/server/server_cc.cc b/src/cpp/server/server_cc.cc index 3cadf65c801..48f0f110a6c 100644 --- a/src/cpp/server/server_cc.cc +++ b/src/cpp/server/server_cc.cc @@ -697,9 +697,6 @@ ServerInterface::BaseAsyncRequest::~BaseAsyncRequest() { bool ServerInterface::BaseAsyncRequest::FinalizeResult(void** tag, bool* status) { - if (*status) { - context_->client_metadata_.FillMap(); - } context_->set_call(call_); context_->cq_ = call_cq_; internal::Call call(call_, server_, call_cq_, diff --git a/src/cpp/server/server_context.cc b/src/cpp/server/server_context.cc index 6f5bde0d9f4..bf0c027cda0 100644 --- a/src/cpp/server/server_context.cc +++ b/src/cpp/server/server_context.cc @@ -134,7 +134,6 @@ ServerContext::ServerContext(gpr_timespec deadline, grpc_metadata_array* arr) compression_level_set_(false), has_pending_ops_(false) { std::swap(*client_metadata_.arr(), *arr); - client_metadata_.FillMap(); } ServerContext::~ServerContext() {