protobuf: optimize unpackTo() for message upgrades.

While looking at eds_speed_test flamegraphs as part of envoyproxy#10875, it seemed we were doing some redundant work, namely first unpacking to v2 message and then upgrading from v2 to v3. Since v2 and v3 are wire compatible, and upgrade is just a wirecast, we might as well just unpack to v2. This improves eds_speed_test significantly, the penalty for v3 vs. v2 drops from 2.6x to 2x. Part of envoyproxy#11362 envoyproxy#10875. Risk level: Low Testing: Coverage of behavior from existing tests. Signed-off-by: Harvey Tuch <[email protected]>
htuch · Jul 10, 2020 · ecbb54a · ecbb54a
1 parent 706d976
commit ecbb54a
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 20 deletions.
diff --git a/source/common/config/version_converter.cc b/source/common/config/version_converter.cc
@@ -60,10 +60,19 @@ DynamicMessagePtr createForDescriptorWithCast(const Protobuf::Message& message,
   return dynamic_message;
 }
 
+} // namespace
+
+void VersionConverter::upgrade(const Protobuf::Message& prev_message,
+                               Protobuf::Message& next_message) {
+  wireCast(prev_message, next_message);
+  // Track original type to support recoverOriginal().
+  annotateWithOriginalType(*prev_message.GetDescriptor(), next_message);
+}
+
 // This needs to be recursive, since sub-messages are consumed and stored
 // internally, we later want to recover their original types.
-void annotateWithOriginalType(const Protobuf::Descriptor& prev_descriptor,
-                              Protobuf::Message& next_message) {
+void VersionConverter::annotateWithOriginalType(const Protobuf::Descriptor& prev_descriptor,
+                                                Protobuf::Message& upgraded_message) {
   class TypeAnnotatingProtoVisitor : public ProtobufMessage::ProtoVisitor {
   public:
     void onMessage(Protobuf::Message& message, const void* ctxt) override {
@@ -103,16 +112,7 @@ void annotateWithOriginalType(const Protobuf::Descriptor& prev_descriptor,
     }
   };
   TypeAnnotatingProtoVisitor proto_visitor;
-  ProtobufMessage::traverseMutableMessage(proto_visitor, next_message, &prev_descriptor);
-}
-
-} // namespace
-
-void VersionConverter::upgrade(const Protobuf::Message& prev_message,
-                               Protobuf::Message& next_message) {
-  wireCast(prev_message, next_message);
-  // Track original type to support recoverOriginal().
-  annotateWithOriginalType(*prev_message.GetDescriptor(), next_message);
+  ProtobufMessage::traverseMutableMessage(proto_visitor, upgraded_message, &prev_descriptor);
 }
 
 void VersionConverter::eraseOriginalTypeInformation(Protobuf::Message& message) {

diff --git a/source/common/config/version_converter.h b/source/common/config/version_converter.h
@@ -91,6 +91,15 @@ class VersionConverter {
   static void prepareMessageForGrpcWire(Protobuf::Message& message,
                                         envoy::config::core::v3::ApiVersion api_version);
 
+  /**
+   * Annotate an upgraded message with original message type information.
+   *
+   * @param prev_descriptor descriptor for original type.
+   * @param upgraded_message upgraded message.
+   */
+  static void annotateWithOriginalType(const Protobuf::Descriptor& prev_descriptor,
+                                       Protobuf::Message& upgraded_message);
+
   /**
    * For a message that may have been upgraded, recover the original message.
    * This is useful for config dump, debug output etc.

diff --git a/source/common/protobuf/utility.cc b/source/common/protobuf/utility.cc
@@ -577,16 +577,18 @@ void MessageUtil::unpackTo(const ProtobufWkt::Any& any_message, Protobuf::Messag
         Config::ApiTypeOracle::getEarlierVersionDescriptor(message.GetDescriptor()->full_name());
     // If the earlier version matches, unpack and upgrade.
     if (earlier_version_desc != nullptr && any_full_name == earlier_version_desc->full_name()) {
-      Protobuf::DynamicMessageFactory dmf;
-      auto earlier_message =
-          ProtobufTypes::MessagePtr(dmf.GetPrototype(earlier_version_desc)->New());
-      ASSERT(earlier_message != nullptr);
-      if (!any_message.UnpackTo(earlier_message.get())) {
+      // Take the Any message but adjust its type URL, since earlier/later versions are wire
+      // compatible.
+      ProtobufWkt::Any any_message_with_fixup;
+      any_message_with_fixup.MergeFrom(any_message);
+      any_message_with_fixup.set_type_url("type.googleapis.com/" +
+                                          message.GetDescriptor()->full_name());
+      if (!any_message_with_fixup.UnpackTo(&message)) {
         throw EnvoyException(fmt::format("Unable to unpack as {}: {}",
-                                         earlier_message->GetDescriptor()->full_name(),
-                                         any_message.DebugString()));
+                                         earlier_version_desc->full_name(),
+                                         any_message_with_fixup.DebugString()));
       }
-      Config::VersionConverter::upgrade(*earlier_message, message);
+      Config::VersionConverter::annotateWithOriginalType(*earlier_version_desc, message);
       return;
     }
   }