Skip to content

Commit

Permalink
protobuf: optimize unpackTo() for message upgrades. (envoyproxy#12026)
Browse files Browse the repository at this point in the history
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 <htuch@google.com>
Signed-off-by: scheler <santosh.cheler@appdynamics.com>
  • Loading branch information
htuch authored and scheler committed Aug 4, 2020
1 parent 8eff8f6 commit aabc6a2
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 20 deletions.
24 changes: 12 additions & 12 deletions source/common/config/version_converter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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) {
Expand Down
9 changes: 9 additions & 0 deletions source/common/config/version_converter.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
18 changes: 10 additions & 8 deletions source/common/protobuf/utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down

0 comments on commit aabc6a2

Please sign in to comment.