From 8f6a7a2ceba6792b2b5d6fb01888f177a09afe35 Mon Sep 17 00:00:00 2001 From: Vitaly Goldshteyn Date: Mon, 14 Oct 2024 09:29:26 -0700 Subject: [PATCH] Avoid field id comparisons in ExtensionSet serialization in case of single range. PiperOrigin-RevId: 685733801 --- src/google/protobuf/compiler/cpp/message.cc | 43 ++++++++++----- src/google/protobuf/compiler/cpp/message.h | 1 + src/google/protobuf/descriptor.pb.cc | 60 ++++++++++----------- src/google/protobuf/extension_set.cc | 35 +++++++++--- src/google/protobuf/extension_set.h | 22 +++++++- 5 files changed, 111 insertions(+), 50 deletions(-) diff --git a/src/google/protobuf/compiler/cpp/message.cc b/src/google/protobuf/compiler/cpp/message.cc index 2af5856224a8f..7a7d8c656b289 100644 --- a/src/google/protobuf/compiler/cpp/message.cc +++ b/src/google/protobuf/compiler/cpp/message.cc @@ -4511,12 +4511,23 @@ void MessageGenerator::GenerateSerializeOneField(io::Printer* p, void MessageGenerator::GenerateSerializeOneExtensionRange(io::Printer* p, int start, int end) { auto v = p->WithVars(variables_); - p->Emit({{"start", start}, {"end", end}}, - R"cc( - // Extension range [$start$, $end$) - target = this_.$extensions$._InternalSerialize( - internal_default_instance(), $start$, $end$, target, stream); - )cc"); + p->Emit( // + {{"start", start}, {"end", end}}, + R"cc( + // Extension range [$start$, $end$) + target = this_.$extensions$._InternalSerialize( + internal_default_instance(), $start$, $end$, target, stream); + )cc"); +} + +void MessageGenerator::GenerateSerializeAllExtensions(io::Printer* p) { + auto v = p->WithVars(variables_); + p->Emit( + R"cc( + // All extensions. + target = this_.$extensions$._InternalSerializeAll( + internal_default_instance(), target, stream); + )cc"); } void MessageGenerator::GenerateSerializeWithCachedSizesToArray(io::Printer* p) { @@ -4682,16 +4693,23 @@ void MessageGenerator::GenerateSerializeWithCachedSizesBody(io::Printer* p) { } } - void Flush() { - if (has_current_range_) { - mg_->GenerateSerializeOneExtensionRange(p_, min_start_, max_end_); + void Flush(bool is_last_range) { + if (!has_current_range_) { + return; } has_current_range_ = false; + ++range_count_; + if (is_last_range && range_count_ == 1) { + mg_->GenerateSerializeAllExtensions(p_); + } else { + mg_->GenerateSerializeOneExtensionRange(p_, min_start_, max_end_); + } } private: MessageGenerator* mg_; io::Printer* p_; + int range_count_ = 0; bool has_current_range_ = false; int min_start_ = 0; int max_end_ = 0; @@ -4749,12 +4767,13 @@ void MessageGenerator::GenerateSerializeWithCachedSizesBody(io::Printer* p) { size_t i, j; for (i = 0, j = 0; i < ordered_fields.size() || j < sorted_extensions.size();) { - if ((j == sorted_extensions.size()) || + bool no_more_extensions = j == sorted_extensions.size(); + if (no_more_extensions || (i < static_cast(descriptor_->field_count()) && ordered_fields[i]->number() < sorted_extensions[j]->start_number())) { const FieldDescriptor* field = ordered_fields[i++]; - re.Flush(); + re.Flush(no_more_extensions); if (field->options().weak()) { largest_weak_field.ReplaceIfLarger(field); PrintFieldComment(Formatter{p}, field, options_); @@ -4768,7 +4787,7 @@ void MessageGenerator::GenerateSerializeWithCachedSizesBody(io::Printer* p) { re.AddToRange(sorted_extensions[j++]); } } - re.Flush(); + re.Flush(/*is_last_range=*/true); e.EmitIfNotNull(largest_weak_field.Release()); }}, {"handle_unknown_fields", diff --git a/src/google/protobuf/compiler/cpp/message.h b/src/google/protobuf/compiler/cpp/message.h index f77eaadd88968..d47231243977d 100644 --- a/src/google/protobuf/compiler/cpp/message.h +++ b/src/google/protobuf/compiler/cpp/message.h @@ -157,6 +157,7 @@ class MessageGenerator { void GenerateSerializeOneofFields( io::Printer* p, const std::vector& fields); void GenerateSerializeOneExtensionRange(io::Printer* p, int start, int end); + void GenerateSerializeAllExtensions(io::Printer* p); // Generates has_foo() functions and variables for singular field has-bits. void GenerateSingularFieldHasBits(const FieldDescriptor* field, diff --git a/src/google/protobuf/descriptor.pb.cc b/src/google/protobuf/descriptor.pb.cc index db30e9a460815..b71e10ec11dbe 100644 --- a/src/google/protobuf/descriptor.pb.cc +++ b/src/google/protobuf/descriptor.pb.cc @@ -5381,9 +5381,9 @@ PROTOBUF_NOINLINE void ExtensionRangeOptions::Clear() { target, stream); } - // Extension range [1000, 536870912) - target = this_._impl_._extensions_._InternalSerialize( - internal_default_instance(), 1000, 536870912, target, stream); + // All extensions. + target = this_._impl_._extensions_._InternalSerializeAll( + internal_default_instance(), target, stream); if (PROTOBUF_PREDICT_FALSE(this_._internal_metadata_.have_unknown_fields())) { target = ::_pbi::WireFormat::InternalSerializeUnknownFieldsToArray( @@ -8757,9 +8757,9 @@ PROTOBUF_NOINLINE void FileOptions::Clear() { target, stream); } - // Extension range [1000, 536870912) - target = this_._impl_._extensions_._InternalSerialize( - internal_default_instance(), 1000, 536870912, target, stream); + // All extensions. + target = this_._impl_._extensions_._InternalSerializeAll( + internal_default_instance(), target, stream); if (PROTOBUF_PREDICT_FALSE(this_._internal_metadata_.have_unknown_fields())) { target = ::_pbi::WireFormat::InternalSerializeUnknownFieldsToArray( @@ -9337,9 +9337,9 @@ PROTOBUF_NOINLINE void MessageOptions::Clear() { target, stream); } - // Extension range [1000, 536870912) - target = this_._impl_._extensions_._InternalSerialize( - internal_default_instance(), 1000, 536870912, target, stream); + // All extensions. + target = this_._impl_._extensions_._InternalSerializeAll( + internal_default_instance(), target, stream); if (PROTOBUF_PREDICT_FALSE(this_._internal_metadata_.have_unknown_fields())) { target = ::_pbi::WireFormat::InternalSerializeUnknownFieldsToArray( @@ -10536,9 +10536,9 @@ PROTOBUF_NOINLINE void FieldOptions::Clear() { target, stream); } - // Extension range [1000, 536870912) - target = this_._impl_._extensions_._InternalSerialize( - internal_default_instance(), 1000, 536870912, target, stream); + // All extensions. + target = this_._impl_._extensions_._InternalSerializeAll( + internal_default_instance(), target, stream); if (PROTOBUF_PREDICT_FALSE(this_._internal_metadata_.have_unknown_fields())) { target = ::_pbi::WireFormat::InternalSerializeUnknownFieldsToArray( @@ -10979,9 +10979,9 @@ PROTOBUF_NOINLINE void OneofOptions::Clear() { target, stream); } - // Extension range [1000, 536870912) - target = this_._impl_._extensions_._InternalSerialize( - internal_default_instance(), 1000, 536870912, target, stream); + // All extensions. + target = this_._impl_._extensions_._InternalSerializeAll( + internal_default_instance(), target, stream); if (PROTOBUF_PREDICT_FALSE(this_._internal_metadata_.have_unknown_fields())) { target = ::_pbi::WireFormat::InternalSerializeUnknownFieldsToArray( @@ -11364,9 +11364,9 @@ PROTOBUF_NOINLINE void EnumOptions::Clear() { target, stream); } - // Extension range [1000, 536870912) - target = this_._impl_._extensions_._InternalSerialize( - internal_default_instance(), 1000, 536870912, target, stream); + // All extensions. + target = this_._impl_._extensions_._InternalSerializeAll( + internal_default_instance(), target, stream); if (PROTOBUF_PREDICT_FALSE(this_._internal_metadata_.have_unknown_fields())) { target = ::_pbi::WireFormat::InternalSerializeUnknownFieldsToArray( @@ -11790,9 +11790,9 @@ PROTOBUF_NOINLINE void EnumValueOptions::Clear() { target, stream); } - // Extension range [1000, 536870912) - target = this_._impl_._extensions_._InternalSerialize( - internal_default_instance(), 1000, 536870912, target, stream); + // All extensions. + target = this_._impl_._extensions_._InternalSerializeAll( + internal_default_instance(), target, stream); if (PROTOBUF_PREDICT_FALSE(this_._internal_metadata_.have_unknown_fields())) { target = ::_pbi::WireFormat::InternalSerializeUnknownFieldsToArray( @@ -12177,9 +12177,9 @@ PROTOBUF_NOINLINE void ServiceOptions::Clear() { target, stream); } - // Extension range [1000, 536870912) - target = this_._impl_._extensions_._InternalSerialize( - internal_default_instance(), 1000, 536870912, target, stream); + // All extensions. + target = this_._impl_._extensions_._InternalSerializeAll( + internal_default_instance(), target, stream); if (PROTOBUF_PREDICT_FALSE(this_._internal_metadata_.have_unknown_fields())) { target = ::_pbi::WireFormat::InternalSerializeUnknownFieldsToArray( @@ -12570,9 +12570,9 @@ PROTOBUF_NOINLINE void MethodOptions::Clear() { target, stream); } - // Extension range [1000, 536870912) - target = this_._impl_._extensions_._InternalSerialize( - internal_default_instance(), 1000, 536870912, target, stream); + // All extensions. + target = this_._impl_._extensions_._InternalSerializeAll( + internal_default_instance(), target, stream); if (PROTOBUF_PREDICT_FALSE(this_._internal_metadata_.have_unknown_fields())) { target = ::_pbi::WireFormat::InternalSerializeUnknownFieldsToArray( @@ -13713,9 +13713,9 @@ PROTOBUF_NOINLINE void FeatureSet::Clear() { 6, this_._internal_json_format(), target); } - // Extension range [1000, 10001) - target = this_._impl_._extensions_._InternalSerialize( - internal_default_instance(), 1000, 10001, target, stream); + // All extensions. + target = this_._impl_._extensions_._InternalSerializeAll( + internal_default_instance(), target, stream); if (PROTOBUF_PREDICT_FALSE(this_._internal_metadata_.have_unknown_fields())) { target = ::_pbi::WireFormat::InternalSerializeUnknownFieldsToArray( diff --git a/src/google/protobuf/extension_set.cc b/src/google/protobuf/extension_set.cc index bad7d59e2bcdd..f1c51746f9696 100644 --- a/src/google/protobuf/extension_set.cc +++ b/src/google/protobuf/extension_set.cc @@ -13,6 +13,7 @@ #include #include +#include #include #include #include @@ -1279,13 +1280,8 @@ uint8_t* ExtensionSet::_InternalSerializeImpl( const MessageLite* extendee, int start_field_number, int end_field_number, uint8_t* target, io::EpsCopyOutputStream* stream) const { if (PROTOBUF_PREDICT_FALSE(is_large())) { - const auto& end = map_.large->end(); - for (auto it = map_.large->lower_bound(start_field_number); - it != end && it->first < end_field_number; ++it) { - target = it->second.InternalSerializeFieldWithCachedSizesToArray( - extendee, this, it->first, target, stream); - } - return target; + return _InternalSerializeImplLarge(extendee, start_field_number, + end_field_number, target, stream); } const KeyValue* end = flat_end(); const KeyValue* it = flat_begin(); @@ -1297,6 +1293,31 @@ uint8_t* ExtensionSet::_InternalSerializeImpl( return target; } +uint8_t* ExtensionSet::_InternalSerializeAllImpl( + const MessageLite* extendee, uint8_t* target, + io::EpsCopyOutputStream* stream) const { + ForEach( + [&target, extendee, stream, this](int number, const Extension& ext) { + target = ext.InternalSerializeFieldWithCachedSizesToArray( + extendee, this, number, target, stream); + }, + Prefetch{}); + return target; +} + +uint8_t* ExtensionSet::_InternalSerializeImplLarge( + const MessageLite* extendee, int start_field_number, int end_field_number, + uint8_t* target, io::EpsCopyOutputStream* stream) const { + assert(is_large()); + const auto& end = map_.large->end(); + for (auto it = map_.large->lower_bound(start_field_number); + it != end && it->first < end_field_number; ++it) { + target = it->second.InternalSerializeFieldWithCachedSizesToArray( + extendee, this, it->first, target, stream); + } + return target; +} + uint8_t* ExtensionSet::InternalSerializeMessageSetWithCachedSizesToArray( const MessageLite* extendee, uint8_t* target, io::EpsCopyOutputStream* stream) const { diff --git a/src/google/protobuf/extension_set.h b/src/google/protobuf/extension_set.h index 526522e84fdb6..e54aa68addc01 100644 --- a/src/google/protobuf/extension_set.h +++ b/src/google/protobuf/extension_set.h @@ -479,7 +479,6 @@ class PROTOBUF_EXPORT ExtensionSet { // serialized extensions. // // Returns a pointer past the last written byte. - uint8_t* _InternalSerialize(const MessageLite* extendee, int start_field_number, int end_field_number, uint8_t* target, @@ -492,6 +491,16 @@ class PROTOBUF_EXPORT ExtensionSet { end_field_number, target, stream); } + // Same as _InternalSerialize, but do not verify the range of field numbers. + uint8_t* _InternalSerializeAll(const MessageLite* extendee, uint8_t* target, + io::EpsCopyOutputStream* stream) const { + if (flat_size_ == 0) { + assert(!is_large()); + return target; + } + return _InternalSerializeAllImpl(extendee, target, stream); + } + // Like above but serializes in MessageSet format. void SerializeMessageSetWithCachedSizes(const MessageLite* extendee, io::CodedOutputStream* output) const { @@ -586,6 +595,17 @@ class PROTOBUF_EXPORT ExtensionSet { int start_field_number, int end_field_number, uint8_t* target, io::EpsCopyOutputStream* stream) const; + // Implementation of _InternalSerializeAll for non-empty map_. + uint8_t* _InternalSerializeAllImpl(const MessageLite* extendee, + uint8_t* target, + io::EpsCopyOutputStream* stream) const; + // Implementation of _InternalSerialize for large map_. + // Extracted as a separate method to avoid inlining and to reuse in + // _InternalSerializeAllImpl. + uint8_t* _InternalSerializeImplLarge(const MessageLite* extendee, + int start_field_number, + int end_field_number, uint8_t* target, + io::EpsCopyOutputStream* stream) const; // Interface of a lazily parsed singular message extension. class PROTOBUF_EXPORT LazyMessageExtension { public: