Avoid calling GetArena() multiple times.
PiperOrigin-RevId: 583188151
diff --git a/src/google/protobuf/compiler/cpp/field_generators/cord_field.cc b/src/google/protobuf/compiler/cpp/field_generators/cord_field.cc
index 62cbbae..732a60c 100644
--- a/src/google/protobuf/compiler/cpp/field_generators/cord_field.cc
+++ b/src/google/protobuf/compiler/cpp/field_generators/cord_field.cc
@@ -352,8 +352,9 @@
clear_$oneof_name$();
set_has_$name$();
$field$ = new ::absl::Cord;
- if (GetArena() != nullptr) {
- GetArena()->Own($field$);
+ ::$proto_ns$::Arena* arena = GetArena();
+ if (arena != nullptr) {
+ arena->Own($field$);
}
}
*$field$ = value;
@@ -372,8 +373,9 @@
clear_$oneof_name$();
set_has_$name$();
$field$ = new ::absl::Cord;
- if (GetArena() != nullptr) {
- GetArena()->Own($field$);
+ ::$proto_ns$::Arena* arena = GetArena();
+ if (arena != nullptr) {
+ arena->Own($field$);
}
}
*$field$ = value;
@@ -387,8 +389,9 @@
clear_$oneof_name$();
set_has_$name$();
$field$ = new ::absl::Cord;
- if (GetArena() != nullptr) {
- GetArena()->Own($field$);
+ ::$proto_ns$::Arena* arena = GetArena();
+ if (arena != nullptr) {
+ arena->Own($field$);
}
}
return $field$;
diff --git a/src/google/protobuf/dynamic_message.cc b/src/google/protobuf/dynamic_message.cc
index 94d7e45..7872866 100644
--- a/src/google/protobuf/dynamic_message.cc
+++ b/src/google/protobuf/dynamic_message.cc
@@ -351,6 +351,7 @@
// constructor.)
const Descriptor* descriptor = type_info_->type;
+ Arena* arena = GetArena();
// Initialize oneof cases.
int oneof_count = 0;
for (int i = 0; i < descriptor->oneof_decl_count(); ++i) {
@@ -360,7 +361,7 @@
}
if (type_info_->extensions_offset != -1) {
- new (MutableExtensionsRaw()) ExtensionSet(GetArena());
+ new (MutableExtensionsRaw()) ExtensionSet(arena);
}
for (int i = 0; i < descriptor->field_count(); i++) {
const FieldDescriptor* field = descriptor->field(i);
@@ -374,7 +375,7 @@
if (!field->is_repeated()) { \
new (field_ptr) TYPE(field->default_value_##TYPE()); \
} else { \
- new (field_ptr) RepeatedField<TYPE>(GetArena()); \
+ new (field_ptr) RepeatedField<TYPE>(arena); \
} \
break;
@@ -391,7 +392,7 @@
if (!field->is_repeated()) {
new (field_ptr) int{field->default_value_enum()->number()};
} else {
- new (field_ptr) RepeatedField<int>(GetArena());
+ new (field_ptr) RepeatedField<int>(arena);
}
break;
@@ -403,7 +404,7 @@
ArenaStringPtr* asp = new (field_ptr) ArenaStringPtr();
asp->InitDefault();
} else {
- new (field_ptr) RepeatedPtrField<std::string>(GetArena());
+ new (field_ptr) RepeatedPtrField<std::string>(arena);
}
break;
}
@@ -418,20 +419,20 @@
// when the constructor is called inside GetPrototype(), in which
// case we have already locked the factory.
if (lock_factory) {
- if (GetArena() != nullptr) {
+ if (arena != nullptr) {
new (field_ptr) DynamicMapField(
type_info_->factory->GetPrototype(field->message_type()),
- GetArena());
+ arena);
} else {
new (field_ptr) DynamicMapField(
type_info_->factory->GetPrototype(field->message_type()));
}
} else {
- if (GetArena() != nullptr) {
+ if (arena != nullptr) {
new (field_ptr)
DynamicMapField(type_info_->factory->GetPrototypeNoLock(
field->message_type()),
- GetArena());
+ arena);
} else {
new (field_ptr)
DynamicMapField(type_info_->factory->GetPrototypeNoLock(
@@ -439,7 +440,7 @@
}
}
} else {
- new (field_ptr) RepeatedPtrField<Message>(GetArena());
+ new (field_ptr) RepeatedPtrField<Message>(arena);
}
}
break;
diff --git a/src/google/protobuf/extension_set.cc b/src/google/protobuf/extension_set.cc
index 32528d4..0a67b05 100644
--- a/src/google/protobuf/extension_set.cc
+++ b/src/google/protobuf/extension_set.cc
@@ -630,38 +630,40 @@
ClearExtension(number);
return;
}
- ABSL_DCHECK(message->GetArena() == nullptr || message->GetArena() == arena_);
- Arena* message_arena = message->GetArena();
+ Arena* const arena = arena_;
+ Arena* const message_arena = message->GetArena();
+ ABSL_DCHECK(message_arena == nullptr || message_arena == arena);
+
Extension* extension;
if (MaybeNewExtension(number, descriptor, &extension)) {
extension->type = type;
ABSL_DCHECK_EQ(cpp_type(extension->type), WireFormatLite::CPPTYPE_MESSAGE);
extension->is_repeated = false;
extension->is_lazy = false;
- if (message_arena == arena_) {
+ if (message_arena == arena) {
extension->message_value = message;
} else if (message_arena == nullptr) {
extension->message_value = message;
- arena_->Own(message); // not nullptr because not equal to message_arena
+ arena->Own(message); // not nullptr because not equal to message_arena
} else {
- extension->message_value = message->New(arena_);
+ extension->message_value = message->New(arena);
extension->message_value->CheckTypeAndMergeFrom(*message);
}
} else {
ABSL_DCHECK_TYPE(*extension, OPTIONAL_FIELD, MESSAGE);
if (extension->is_lazy) {
- extension->lazymessage_value->SetAllocatedMessage(message, arena_);
+ extension->lazymessage_value->SetAllocatedMessage(message, arena);
} else {
- if (arena_ == nullptr) {
+ if (arena == nullptr) {
delete extension->message_value;
}
- if (message_arena == arena_) {
+ if (message_arena == arena) {
extension->message_value = message;
} else if (message_arena == nullptr) {
extension->message_value = message;
- arena_->Own(message); // not nullptr because not equal to message_arena
+ arena->Own(message); // not nullptr because not equal to message_arena
} else {
- extension->message_value = message->New(arena_);
+ extension->message_value = message->New(arena);
extension->message_value->CheckTypeAndMergeFrom(*message);
}
}
@@ -708,8 +710,9 @@
ABSL_DCHECK_TYPE(*extension, OPTIONAL_FIELD, MESSAGE);
MessageLite* ret = nullptr;
if (extension->is_lazy) {
- ret = extension->lazymessage_value->ReleaseMessage(prototype, arena_);
- if (arena_ == nullptr) {
+ Arena* const arena = arena_;
+ ret = extension->lazymessage_value->ReleaseMessage(prototype, arena);
+ if (arena == nullptr) {
delete extension->lazymessage_value;
}
} else {
@@ -737,9 +740,10 @@
ABSL_DCHECK_TYPE(*extension, OPTIONAL_FIELD, MESSAGE);
MessageLite* ret = nullptr;
if (extension->is_lazy) {
+ Arena* const arena = arena_;
ret = extension->lazymessage_value->UnsafeArenaReleaseMessage(prototype,
- arena_);
- if (arena_ == nullptr) {
+ arena);
+ if (arena == nullptr) {
delete extension->lazymessage_value;
}
} else {
@@ -994,10 +998,11 @@
HANDLE_TYPE(STRING, string, RepeatedPtrField<std::string>);
#undef HANDLE_TYPE
- case WireFormatLite::CPPTYPE_MESSAGE:
+ case WireFormatLite::CPPTYPE_MESSAGE: {
+ Arena* const arena = arena_;
if (is_new) {
extension->repeated_message_value =
- Arena::CreateMessage<RepeatedPtrField<MessageLite>>(arena_);
+ Arena::CreateMessage<RepeatedPtrField<MessageLite>>(arena);
}
// We can't call RepeatedPtrField<MessageLite>::MergeFrom() because
// it would attempt to allocate new objects.
@@ -1010,12 +1015,13 @@
extension->repeated_message_value)
->AddFromCleared<GenericTypeHandler<MessageLite>>();
if (target == nullptr) {
- target = other_message.New(arena_);
+ target = other_message.New(arena);
extension->repeated_message_value->AddAllocated(target);
}
target->CheckTypeAndMergeFrom(other_message);
}
break;
+ }
}
} else {
if (!other_extension.is_cleared) {
@@ -1041,6 +1047,7 @@
other_extension.descriptor);
break;
case WireFormatLite::CPPTYPE_MESSAGE: {
+ Arena* const arena = arena_;
Extension* extension;
bool is_new =
MaybeNewExtension(number, other_extension.descriptor, &extension);
@@ -1051,14 +1058,14 @@
if (other_extension.is_lazy) {
extension->is_lazy = true;
extension->lazymessage_value =
- other_extension.lazymessage_value->New(arena_);
+ other_extension.lazymessage_value->New(arena);
extension->lazymessage_value->MergeFrom(
GetPrototypeForLazyMessage(extendee, number),
- *other_extension.lazymessage_value, arena_);
+ *other_extension.lazymessage_value, arena);
} else {
extension->is_lazy = false;
extension->message_value =
- other_extension.message_value->New(arena_);
+ other_extension.message_value->New(arena);
extension->message_value->CheckTypeAndMergeFrom(
*other_extension.message_value);
}
@@ -1070,7 +1077,7 @@
if (extension->is_lazy) {
extension->lazymessage_value->MergeFrom(
GetPrototypeForLazyMessage(extendee, number),
- *other_extension.lazymessage_value, arena_);
+ *other_extension.lazymessage_value, arena);
} else {
extension->message_value->CheckTypeAndMergeFrom(
other_extension.lazymessage_value->GetMessage(
@@ -1079,7 +1086,7 @@
} else {
if (extension->is_lazy) {
extension->lazymessage_value
- ->MutableMessage(*other_extension.message_value, arena_)
+ ->MutableMessage(*other_extension.message_value, arena)
->CheckTypeAndMergeFrom(*other_extension.message_value);
} else {
extension->message_value->CheckTypeAndMergeFrom(
@@ -1097,9 +1104,9 @@
void ExtensionSet::Swap(const MessageLite* extendee, ExtensionSet* other) {
#ifdef PROTOBUF_FORCE_COPY_IN_SWAP
- if (GetArena() != nullptr && GetArena() == other->GetArena()) {
+ if (arena_ != nullptr && arena_ == other->arena_) {
#else // PROTOBUF_FORCE_COPY_IN_SWAP
- if (GetArena() == other->GetArena()) {
+ if (arena_ == other->arena_) {
#endif // !PROTOBUF_FORCE_COPY_IN_SWAP
InternalSwap(other);
} else {
@@ -1127,7 +1134,9 @@
ExtensionSet* other, int number) {
if (this == other) return;
- if (GetArena() == other->GetArena()) {
+ Arena* const arena = arena_;
+ Arena* const other_arena = other->arena_;
+ if (arena == other_arena) {
UnsafeShallowSwapExtension(other, number);
return;
}
@@ -1144,23 +1153,20 @@
// We do it this way to reuse the copy-across-arenas logic already
// implemented in ExtensionSet's MergeFrom.
ExtensionSet temp;
- temp.InternalExtensionMergeFrom(extendee, number, *other_ext,
- other->GetArena());
+ temp.InternalExtensionMergeFrom(extendee, number, *other_ext, other_arena);
Extension* temp_ext = temp.FindOrNull(number);
other_ext->Clear();
- other->InternalExtensionMergeFrom(extendee, number, *this_ext,
- this->GetArena());
+ other->InternalExtensionMergeFrom(extendee, number, *this_ext, arena);
this_ext->Clear();
InternalExtensionMergeFrom(extendee, number, *temp_ext, temp.GetArena());
} else if (this_ext == nullptr) {
- InternalExtensionMergeFrom(extendee, number, *other_ext, other->GetArena());
- if (other->GetArena() == nullptr) other_ext->Free();
+ InternalExtensionMergeFrom(extendee, number, *other_ext, other_arena);
+ if (other_arena == nullptr) other_ext->Free();
other->Erase(number);
} else {
- other->InternalExtensionMergeFrom(extendee, number, *this_ext,
- this->GetArena());
- if (GetArena() == nullptr) this_ext->Free();
+ other->InternalExtensionMergeFrom(extendee, number, *this_ext, arena);
+ if (arena == nullptr) this_ext->Free();
Erase(number);
}
}
@@ -1173,7 +1179,7 @@
if (this_ext == other_ext) return;
- ABSL_DCHECK_EQ(GetArena(), other->GetArena());
+ ABSL_DCHECK_EQ(arena_, other->arena_);
if (this_ext != nullptr && other_ext != nullptr) {
std::swap(*this_ext, *other_ext);
@@ -1189,16 +1195,17 @@
bool ExtensionSet::IsInitialized(const MessageLite* extendee) const {
// Extensions are never required. However, we need to check that all
// embedded messages are initialized.
+ Arena* const arena = arena_;
if (PROTOBUF_PREDICT_FALSE(is_large())) {
for (const auto& kv : *map_.large) {
- if (!kv.second.IsInitialized(this, extendee, kv.first, arena_)) {
+ if (!kv.second.IsInitialized(this, extendee, kv.first, arena)) {
return false;
}
}
return true;
}
for (const KeyValue* it = flat_begin(); it != flat_end(); ++it) {
- if (!it->second.IsInitialized(this, extendee, it->first, arena_)) {
+ if (!it->second.IsInitialized(this, extendee, it->first, arena)) {
return false;
}
}
@@ -1639,8 +1646,9 @@
const KeyValue* begin = flat_begin();
const KeyValue* end = flat_end();
AllocatedData new_map;
+ Arena* const arena = arena_;
if (new_flat_capacity > kMaximumFlatCapacity) {
- new_map.large = Arena::Create<LargeMap>(arena_);
+ new_map.large = Arena::Create<LargeMap>(arena);
LargeMap::iterator hint = new_map.large->begin();
for (const KeyValue* it = begin; it != end; ++it) {
hint = new_map.large->insert(hint, {it->first, it->second});
@@ -1648,11 +1656,11 @@
flat_size_ = static_cast<uint16_t>(-1);
ABSL_DCHECK(is_large());
} else {
- new_map.flat = Arena::CreateArray<KeyValue>(arena_, new_flat_capacity);
+ new_map.flat = Arena::CreateArray<KeyValue>(arena, new_flat_capacity);
std::copy(begin, end, new_map.flat);
}
- if (arena_ == nullptr) {
+ if (arena == nullptr) {
DeleteFlatMap(begin, flat_capacity_);
}
flat_capacity_ = new_flat_capacity;
diff --git a/src/google/protobuf/generated_message_reflection.cc b/src/google/protobuf/generated_message_reflection.cc
index eb60d5f..c75c290 100644
--- a/src/google/protobuf/generated_message_reflection.cc
+++ b/src/google/protobuf/generated_message_reflection.cc
@@ -1044,21 +1044,24 @@
}
}
-void Reflection::Swap(Message* message1, Message* message2) const {
- if (message1 == message2) return;
+void Reflection::Swap(Message* lhs, Message* rhs) const {
+ if (lhs == rhs) return;
+
+ Arena* lhs_arena = lhs->GetArena();
+ Arena* rhs_arena = rhs->GetArena();
// TODO: Other Reflection methods should probably check this too.
- ABSL_CHECK_EQ(message1->GetReflection(), this)
+ ABSL_CHECK_EQ(lhs->GetReflection(), this)
<< "First argument to Swap() (of type \""
- << message1->GetDescriptor()->full_name()
+ << lhs->GetDescriptor()->full_name()
<< "\") is not compatible with this reflection object (which is for type "
"\""
<< descriptor_->full_name()
<< "\"). Note that the exact same class is required; not just the same "
"descriptor.";
- ABSL_CHECK_EQ(message2->GetReflection(), this)
+ ABSL_CHECK_EQ(rhs->GetReflection(), this)
<< "Second argument to Swap() (of type \""
- << message2->GetDescriptor()->full_name()
+ << rhs->GetDescriptor()->full_name()
<< "\") is not compatible with this reflection object (which is for type "
"\""
<< descriptor_->full_name()
@@ -1068,32 +1071,31 @@
// Check that both messages are in the same arena (or both on the heap). We
// need to copy all data if not, due to ownership semantics.
#ifdef PROTOBUF_FORCE_COPY_IN_SWAP
- if (message1->GetArena() == nullptr ||
- message1->GetArena() != message2->GetArena()) {
+ if (lhs_arena == nullptr || lhs_arena != rhs_arena) {
#else // PROTOBUF_FORCE_COPY_IN_SWAP
- if (message1->GetArena() != message2->GetArena()) {
+ if (lhs_arena != rhs_arena) {
#endif // !PROTOBUF_FORCE_COPY_IN_SWAP
// One of the two is guaranteed to have an arena. Switch things around
- // to guarantee that message1 has an arena.
- Arena* arena = message1->GetArena();
+ // to guarantee that lhs has an arena.
+ Arena* arena = lhs_arena;
if (arena == nullptr) {
- arena = message2->GetArena();
- std::swap(message1, message2); // Swapping names for pointers!
+ arena = rhs_arena;
+ std::swap(lhs, rhs); // Swapping names for pointers!
}
- Message* temp = message1->New(arena);
- temp->MergeFrom(*message2);
- message2->CopyFrom(*message1);
+ Message* temp = lhs->New(arena);
+ temp->MergeFrom(*rhs);
+ rhs->CopyFrom(*lhs);
#ifdef PROTOBUF_FORCE_COPY_IN_SWAP
- message1->CopyFrom(*temp);
+ lhs->CopyFrom(*temp);
if (arena == nullptr) delete temp;
#else // PROTOBUF_FORCE_COPY_IN_SWAP
- Swap(message1, temp);
+ Swap(lhs, temp);
#endif // !PROTOBUF_FORCE_COPY_IN_SWAP
return;
}
- UnsafeArenaSwap(message1, message2);
+ UnsafeArenaSwap(lhs, rhs);
}
template <bool unsafe_shallow_swap>
@@ -2318,27 +2320,34 @@
ABSL_DCHECK(sub_message == nullptr || sub_message->GetArena() == nullptr ||
sub_message->GetArena() == message->GetArena());
+ if (sub_message == nullptr) {
+ UnsafeArenaSetAllocatedMessage(message, nullptr, field);
+ return;
+ }
+
+ Arena* arena = message->GetArena();
+ Arena* sub_arena = sub_message->GetArena();
+ if (arena == sub_arena) {
+ UnsafeArenaSetAllocatedMessage(message, sub_message, field);
+ return;
+ }
+
// If message and sub-message are in different memory ownership domains
// (different arenas, or one is on heap and one is not), then we may need to
// do a copy.
- if (sub_message != nullptr &&
- sub_message->GetArena() != message->GetArena()) {
- if (sub_message->GetArena() == nullptr && message->GetArena() != nullptr) {
- // Case 1: parent is on an arena and child is heap-allocated. We can add
- // the child to the arena's Own() list to free on arena destruction, then
- // set our pointer.
- message->GetArena()->Own(sub_message);
- UnsafeArenaSetAllocatedMessage(message, sub_message, field);
- } else {
- // Case 2: all other cases. We need to make a copy. MutableMessage() will
- // either get the existing message object, or instantiate a new one as
- // appropriate w.r.t. our arena.
- Message* sub_message_copy = MutableMessage(message, field);
- sub_message_copy->CopyFrom(*sub_message);
- }
- } else {
- // Same memory ownership domains.
+ if (sub_arena == nullptr) {
+ ABSL_DCHECK_NE(arena, nullptr);
+ // Case 1: parent is on an arena and child is heap-allocated. We can add
+ // the child to the arena's Own() list to free on arena destruction, then
+ // set our pointer.
+ arena->Own(sub_message);
UnsafeArenaSetAllocatedMessage(message, sub_message, field);
+ } else {
+ // Case 2: all other cases. We need to make a copy. MutableMessage() will
+ // either get the existing message object, or instantiate a new one as
+ // appropriate w.r.t. our arena.
+ Message* sub_message_copy = MutableMessage(message, field);
+ sub_message_copy->CopyFrom(*sub_message);
}
}
diff --git a/src/google/protobuf/repeated_ptr_field.cc b/src/google/protobuf/repeated_ptr_field.cc
index 2afe3ad..929131c 100644
--- a/src/google/protobuf/repeated_ptr_field.cc
+++ b/src/google/protobuf/repeated_ptr_field.cc
@@ -97,10 +97,11 @@
template <typename F>
auto* RepeatedPtrFieldBase::AddInternal(F factory) {
- using Result = decltype(factory(GetArena()));
+ Arena* const arena = GetArena();
+ using Result = decltype(factory(arena));
if (tagged_rep_or_elem_ == nullptr) {
ExchangeCurrentSize(1);
- tagged_rep_or_elem_ = factory(GetArena());
+ tagged_rep_or_elem_ = factory(arena);
return static_cast<Result>(tagged_rep_or_elem_);
}
if (using_sso()) {
@@ -122,7 +123,7 @@
Rep* r = rep();
++r->allocated_size;
void*& result = r->elements[ExchangeCurrentSize(current_size_ + 1)];
- result = factory(GetArena());
+ result = factory(arena);
return static_cast<Result>(result);
}
