Merge pull request #16407 from ncteisen/lazy-metadata

Lazily Fill Metadata Map

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<StatusCode>(status_code_),
                GRPC_SLICE_IS_EMPTY(error_message_)

include/grpcpp/impl/codegen/client_context.h

@@ -425,8 +425,8 @@ class ClientContext {
   mutable std::shared_ptr<const AuthContext> auth_context_;
   struct census_context* census_context_;
   std::multimap<grpc::string, grpc::string> 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_;

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 <grpc/impl/codegen/log.h>
 #include <grpcpp/impl/codegen/slice.h>
 
 namespace grpc {
 
 namespace internal {
+
+const char kBinaryErrorDetailsKey[] = "grpc-status-details-bin";
+
 class MetadataMap {
  public:
   MetadataMap() { memset(&arr_, 0, sizeof(arr_)); }
@@ -32,24 +36,54 @@ class MetadataMap {
     g_core_codegen_interface->grpc_metadata_array_destroy(&arr_);
   }
 
-  void FillMap() {
-    for (size_t i = 0; i < arr_.count; i++) {
-      // TODO(yangg) handle duplicates?
-      map_.insert(std::pair<grpc::string_ref, grpc::string_ref>(
-          StringRefFromSlice(&arr_.metadata[i].key),
-          StringRefFromSlice(&arr_.metadata[i].value)));
+  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.
+    // TODO(ncteisen): plumb this through core as a first class object, just
+    // like code and message.
+    else {
+      for (size_t i = 0; i < arr_.count; i++) {
+        if (strncmp(reinterpret_cast<const char*>(
+                        GRPC_SLICE_START_PTR(arr_.metadata[i].key)),
+                    kBinaryErrorDetailsKey,
+                    GRPC_SLICE_LENGTH(arr_.metadata[i].key)) == 0) {
+          return grpc::string(reinterpret_cast<const char*>(
+                                  GRPC_SLICE_START_PTR(arr_.metadata[i].value)),
+                              GRPC_SLICE_LENGTH(arr_.metadata[i].value));
+        }
+      }
     }
+    return grpc::string();
   }
 
-  std::multimap<grpc::string_ref, grpc::string_ref>* map() { return &map_; }
-  const std::multimap<grpc::string_ref, grpc::string_ref>* map() const {
+  std::multimap<grpc::string_ref, grpc::string_ref>* map() {
+    FillMap();
     return &map_;
   }
   grpc_metadata_array* arr() { return &arr_; }
 
  private:
+  bool filled_ = false;
   grpc_metadata_array arr_;
   std::multimap<grpc::string_ref, grpc::string_ref> map_;
+
+  void FillMap() {
+    if (filled_) return;
+    filled_ = true;
+    for (size_t i = 0; i < arr_.count; i++) {
+      // TODO(yangg) handle duplicates?
+      map_.insert(std::pair<grpc::string_ref, grpc::string_ref>(
+          StringRefFromSlice(&arr_.metadata[i].key),
+          StringRefFromSlice(&arr_.metadata[i].value)));
+    }
+  }
 };
 }  // namespace internal
 

include/grpcpp/impl/codegen/server_context.h

@@ -294,7 +294,7 @@ class ServerContext {
   CompletionQueue* cq_;
   bool sent_initial_metadata_;
   mutable std::shared_ptr<const AuthContext> auth_context_;
-  internal::MetadataMap client_metadata_;
+  mutable internal::MetadataMap client_metadata_;
   std::multimap<grpc::string, grpc::string> initial_metadata_;
   std::multimap<grpc::string, grpc::string> trailing_metadata_;
 

src/cpp/server/server_cc.cc

@@ -686,9 +686,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_,

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() {