Improvements to MergeFrom for weak fields:

- Remove _Internal accessors. They only have one caller and it's better to inline it.
 - Remove redundant has bit setting. MergeFrom does it globally instead of per field.
 - Reuse existing arena object. Avoids redundant calls to `GetArena()`.
 - Use rhs object instead of weak instance for calling `New()`.
 - Change repeated fields to use the generic MergeFrom instead of Arena::CopyConstruct.

PiperOrigin-RevId: 586680126
pull/14900/head
Protobuf Team Bot 1 year ago committed by Copybara-Service
parent 07194fcfae
commit bb8958d180
  1. 16
      src/google/protobuf/compiler/cpp/field.h
  2. 53
      src/google/protobuf/compiler/cpp/field_generators/message_field.cc
  3. 7
      src/google/protobuf/compiler/cpp/message.cc
  4. 6
      src/google/protobuf/repeated_ptr_field.h

@ -121,10 +121,6 @@ class FieldGeneratorBase {
virtual void GenerateNonInlineAccessorDefinitions(io::Printer* p) const {}
virtual void GenerateInternalAccessorDefinitions(io::Printer* p) const {}
virtual void GenerateInternalAccessorDeclarations(io::Printer* p) const {}
virtual void GenerateClearingCode(io::Printer* p) const = 0;
virtual void GenerateMessageClearingCode(io::Printer* p) const {
@ -334,18 +330,6 @@ class FieldGenerator {
impl_->GenerateNonInlineAccessorDefinitions(p);
}
// Generates declarations of accessors that are for internal purposes only.
void GenerateInternalAccessorDefinitions(io::Printer* p) const {
auto vars = PushVarsForCall(p);
impl_->GenerateInternalAccessorDefinitions(p);
}
// Generates definitions of accessors that are for internal purposes only.
void GenerateInternalAccessorDeclarations(io::Printer* p) const {
auto vars = PushVarsForCall(p);
impl_->GenerateInternalAccessorDeclarations(p);
}
// Generates statements which clear the field.
//
// This is used to define the clear_$name$() method.

@ -108,8 +108,6 @@ class SingularMessage : public FieldGeneratorBase {
void GenerateAccessorDeclarations(io::Printer* p) const override;
void GenerateInlineAccessorDefinitions(io::Printer* p) const override;
void GenerateInternalAccessorDeclarations(io::Printer* p) const override;
void GenerateInternalAccessorDefinitions(io::Printer* p) const override;
void GenerateClearingCode(io::Printer* p) const override;
void GenerateMessageClearingCode(io::Printer* p) const override;
void GenerateMergingCode(io::Printer* p) const override;
@ -307,47 +305,6 @@ void SingularMessage::GenerateInlineAccessorDefinitions(io::Printer* p) const {
)cc");
}
void SingularMessage::GenerateInternalAccessorDeclarations(
io::Printer* p) const {
if (!is_weak()) return;
p->Emit(R"cc(
static $pb$::MessageLite* mutable_$name$($Msg$* msg);
)cc");
}
void SingularMessage::GenerateInternalAccessorDefinitions(
io::Printer* p) const {
// In theory, these accessors could be inline in _Internal. However, in
// practice, the linker is then not able to throw them out making implicit
// weak dependencies not work at all.
if (!is_weak() || is_oneof()) return;
// These private accessors are used by MergeFrom and
// MergePartialFromCodedStream, and their purpose is to provide access to
// the field without creating a strong dependency on the message type.
p->Emit(
{
{"update_hasbit",
[&] {
if (!has_hasbit_) return;
p->Emit(R"cc(
msg->$set_hasbit$;
)cc");
}},
},
R"cc(
$pb$::MessageLite* $Msg$::_Internal::mutable_$name$($Msg$* msg) {
$update_hasbit$;
if (msg->$field_$ == nullptr) {
msg->$field_$ = $kDefaultPtr$->New(msg->GetArena());
}
return msg->$field_$;
}
)cc");
}
void SingularMessage::GenerateClearingCode(io::Printer* p) const {
if (!has_hasbit_) {
// If we don't have has-bits, message presence is indicated only by ptr !=
@ -381,7 +338,7 @@ void SingularMessage::GenerateMessageClearingCode(io::Printer* p) const {
bool SingularMessage::RequiresArena(GeneratorFunction function) const {
switch (function) {
case GeneratorFunction::kMergeFrom:
return !(is_weak() || should_split());
return !should_split();
}
return false;
}
@ -389,8 +346,12 @@ bool SingularMessage::RequiresArena(GeneratorFunction function) const {
void SingularMessage::GenerateMergingCode(io::Printer* p) const {
if (is_weak()) {
p->Emit(
"_Internal::mutable_$name$(_this)->CheckTypeAndMergeFrom(\n"
" *from.$field_$);\n");
R"cc(
if (_this->$field_$ == nullptr) {
_this->$field_$ = from.$field_$->New(arena);
}
_this->$field_$->CheckTypeAndMergeFrom(*from.$field_$);
)cc");
} else if (should_split()) {
p->Emit(
"_this->_internal_mutable_$name$()->$Submsg$::MergeFrom(\n"

@ -2121,10 +2121,6 @@ void MessageGenerator::GenerateClassMethods(io::Printer* p) {
"static constexpr ::int32_t kOneofCaseOffset =\n"
" PROTOBUF_FIELD_OFFSET($classtype$, $oneof_case$);\n");
}
for (auto field : FieldRange(descriptor_)) {
auto t = p->WithVars(MakeTrackerCalls(field, options_));
field_generators_.get(field).GenerateInternalAccessorDeclarations(p);
}
if (num_required_fields_ > 0) {
const std::vector<uint32_t> masks_for_has_bits = RequiredFieldsBitMask();
format(
@ -2137,9 +2133,6 @@ void MessageGenerator::GenerateClassMethods(io::Printer* p) {
format.Outdent();
format("};\n\n");
for (auto field : FieldRange(descriptor_)) {
field_generators_.get(field).GenerateInternalAccessorDefinitions(p);
}
// Generate non-inline field definitions.
for (auto field : FieldRange(descriptor_)) {

@ -259,6 +259,12 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase {
template <typename T>
void MergeFrom(const RepeatedPtrFieldBase& from) {
static_assert(std::is_base_of<MessageLite, T>::value, "");
#ifdef __cpp_if_constexpr
if constexpr (!std::is_base_of<Message, T>::value) {
// For LITE objects we use the generic MergeFrom to save on binary size.
return MergeFrom<MessageLite>(from);
}
#endif
MergeFromConcreteMessage(from, Arena::CopyConstruct<T>);
}

Loading…
Cancel
Save