Change `FieldDescriptor::type()` to not have the call_once behavior.

The field is always set while building the descriptor, even for lazy descriptors.
The `type()` function is called extensively in reflection based code and
removing the call_once has significant performance impact.

PiperOrigin-RevId: 629723749
pull/16699/head
Protobuf Team Bot 11 months ago committed by Copybara-Service
parent 26cf1cb94c
commit a45e0d83ab
  1. 34
      src/google/protobuf/descriptor.cc
  2. 5
      src/google/protobuf/descriptor.h
  3. 16
      src/google/protobuf/descriptor_unittest.cc

@ -2808,7 +2808,7 @@ bool DescriptorPool::TryFindExtensionInFallbackDatabase(
// ===================================================================
bool FieldDescriptor::is_map_message_type() const {
return type_descriptor_.message_type->options().map_entry();
return message_type()->options().map_entry();
}
std::string FieldDescriptor::DefaultValueAsString(
@ -6503,12 +6503,8 @@ void DescriptorBuilder::BuildFieldOrExtension(const FieldDescriptorProto& proto,
result->has_json_name_ = proto.has_json_name();
// Some compilers do not allow static_cast directly between two enum types,
// so we must cast to int first.
result->type_ = static_cast<FieldDescriptor::Type>(
absl::implicit_cast<int>(proto.type()));
result->label_ = static_cast<FieldDescriptor::Label>(
absl::implicit_cast<int>(proto.label()));
result->type_ = proto.type();
result->label_ = proto.label();
result->is_repeated_ = result->label_ == FieldDescriptor::LABEL_REPEATED;
if (result->label() == FieldDescriptor::LABEL_REQUIRED) {
@ -7388,6 +7384,10 @@ void DescriptorBuilder::CrossLinkField(FieldDescriptor* field,
if (type.IsNull()) {
if (is_lazy) {
ABSL_CHECK(field->type_ == FieldDescriptor::TYPE_MESSAGE ||
field->type_ == FieldDescriptor::TYPE_GROUP ||
field->type_ == FieldDescriptor::TYPE_ENUM)
<< proto;
// Save the symbol names for later for lookup, and allocate the once
// object needed for the accessors.
const std::string& name = proto.type_name();
@ -9584,19 +9584,23 @@ void FieldDescriptor::TypeOnceInit(const FieldDescriptor* to_init) {
// all share the same absl::call_once init path to do lazy
// import building and cross linking of a field of a message.
const Descriptor* FieldDescriptor::message_type() const {
if (type_once_) {
absl::call_once(*type_once_, FieldDescriptor::TypeOnceInit, this);
if (type_ == TYPE_MESSAGE || type_ == TYPE_GROUP) {
if (type_once_) {
absl::call_once(*type_once_, FieldDescriptor::TypeOnceInit, this);
}
return type_descriptor_.message_type;
}
return type_ == TYPE_MESSAGE || type_ == TYPE_GROUP
? type_descriptor_.message_type
: nullptr;
return nullptr;
}
const EnumDescriptor* FieldDescriptor::enum_type() const {
if (type_once_) {
absl::call_once(*type_once_, FieldDescriptor::TypeOnceInit, this);
if (type_ == TYPE_ENUM) {
if (type_once_) {
absl::call_once(*type_once_, FieldDescriptor::TypeOnceInit, this);
}
return type_descriptor_.enum_type;
}
return type_ == TYPE_ENUM ? type_descriptor_.enum_type : nullptr;
return nullptr;
}
const EnumValueDescriptor* FieldDescriptor::default_value_enum() const {

@ -1090,7 +1090,7 @@ class PROTOBUF_EXPORT FieldDescriptor : private internal::SymbolBase,
uint8_t label_ : 2;
// Actually a `Type`, but stored as uint8_t to save space.
mutable uint8_t type_;
uint8_t type_;
// Logically:
// all_names_ = [name, full_name, lower, camel, json]
@ -2676,9 +2676,6 @@ inline FieldDescriptor::Label FieldDescriptor::label() const {
}
inline FieldDescriptor::Type FieldDescriptor::type() const {
if (type_once_) {
absl::call_once(*type_once_, &FieldDescriptor::TypeOnceInit, this);
}
return static_cast<Type>(type_);
}

@ -13035,7 +13035,7 @@ TEST_F(LazilyBuildDependenciesTest, Type) {
const FileDescriptor* file = pool_.FindFileByName("foo.proto");
// Verify calling type() on a field that is a message type will
// Verify calling type() on a field that is a message type will _not_
// build the type defined in another file.
EXPECT_FALSE(pool_.InternalIsFileLoaded("message1.proto"));
const Descriptor* desc = file->FindMessageTypeByName("Lazy");
@ -13043,31 +13043,31 @@ TEST_F(LazilyBuildDependenciesTest, Type) {
const FieldDescriptor* field = desc->FindFieldByName("message1");
EXPECT_TRUE(field != nullptr);
EXPECT_EQ(field->type(), FieldDescriptor::TYPE_MESSAGE);
EXPECT_TRUE(pool_.InternalIsFileLoaded("message1.proto"));
EXPECT_FALSE(pool_.InternalIsFileLoaded("message1.proto"));
// Verify calling cpp_type() on a field that is a message type will
// Verify calling cpp_type() on a field that is a message type will _not_
// build the type defined in another file.
EXPECT_FALSE(pool_.InternalIsFileLoaded("message2.proto"));
field = desc->FindFieldByName("message2");
EXPECT_TRUE(field != nullptr);
EXPECT_EQ(field->cpp_type(), FieldDescriptor::CPPTYPE_MESSAGE);
EXPECT_TRUE(pool_.InternalIsFileLoaded("message2.proto"));
EXPECT_FALSE(pool_.InternalIsFileLoaded("message2.proto"));
// Verify calling type() on a field that is an enum type will
// Verify calling type() on a field that is an enum type will _not_
// build the type defined in another file.
EXPECT_FALSE(pool_.InternalIsFileLoaded("enum1.proto"));
field = desc->FindFieldByName("enum1");
EXPECT_TRUE(field != nullptr);
EXPECT_EQ(field->type(), FieldDescriptor::TYPE_ENUM);
EXPECT_TRUE(pool_.InternalIsFileLoaded("enum1.proto"));
EXPECT_FALSE(pool_.InternalIsFileLoaded("enum1.proto"));
// Verify calling cpp_type() on a field that is an enum type will
// Verify calling cpp_type() on a field that is an enum type will _not_
// build the type defined in another file.
EXPECT_FALSE(pool_.InternalIsFileLoaded("enum2.proto"));
field = desc->FindFieldByName("enum2");
EXPECT_TRUE(field != nullptr);
EXPECT_EQ(field->cpp_type(), FieldDescriptor::CPPTYPE_ENUM);
EXPECT_TRUE(pool_.InternalIsFileLoaded("enum2.proto"));
EXPECT_FALSE(pool_.InternalIsFileLoaded("enum2.proto"));
}
TEST_F(LazilyBuildDependenciesTest, Extension) {

Loading…
Cancel
Save