From ad66689f2bf50db4ca6830cbcd70c69d2874a5e6 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Thu, 20 Aug 2026 09:34:40 -0700 Subject: [PATCH] Internal change. PiperOrigin-RevId: 967874213 --- .../compiler/java/full/field_generator.cc | 13 +++++ .../compiler/java/full/field_generator.h | 4 ++ .../compiler/java/full/make_field_gens.cc | 40 ++++++++++++-- .../compiler/java/full/message_builder.cc | 53 ++++++------------- .../compiler/java/full/message_builder.h | 5 +- .../protobuf/compiler/java/generator_common.h | 23 +++++--- src/google/protobuf/compiler/java/helpers.cc | 22 +++++++- src/google/protobuf/compiler/java/helpers.h | 2 + 8 files changed, 110 insertions(+), 52 deletions(-) diff --git a/src/google/protobuf/compiler/java/full/field_generator.cc b/src/google/protobuf/compiler/java/full/field_generator.cc index a72b7be360..21501e9995 100644 --- a/src/google/protobuf/compiler/java/full/field_generator.cc +++ b/src/google/protobuf/compiler/java/full/field_generator.cc @@ -1,6 +1,7 @@ #include "google/protobuf/compiler/java/full/field_generator.h" #include "google/protobuf/compiler/java/context.h" +#include "google/protobuf/compiler/java/helpers.h" #include "google/protobuf/compiler/java/name_resolver.h" namespace google { @@ -15,6 +16,18 @@ ImmutableFieldGenerator::ImmutableFieldGenerator( context_(context), name_resolver_(context->GetNameResolver()) {} +bool ImmutableFieldGenerator::HasHasbit() const { + return ::google::protobuf::compiler::java::HasHasbit(descriptor_); +} + +bool ImmutableFieldGenerator::IsRealOneof() const { + return ::google::protobuf::compiler::java::IsRealOneof(descriptor_); +} + +bool ImmutableFieldGenerator::BitfieldTracksMutability() const { + return ::google::protobuf::compiler::java::BitfieldTracksMutability(descriptor_); +} + } // namespace java } // namespace compiler } // namespace protobuf diff --git a/src/google/protobuf/compiler/java/full/field_generator.h b/src/google/protobuf/compiler/java/full/field_generator.h index fddc8d58f2..56f8c9484c 100644 --- a/src/google/protobuf/compiler/java/full/field_generator.h +++ b/src/google/protobuf/compiler/java/full/field_generator.h @@ -23,6 +23,10 @@ class ImmutableFieldGenerator : public FieldGenerator { ImmutableFieldGenerator& operator=(const ImmutableFieldGenerator&) = delete; ~ImmutableFieldGenerator() override = default; + bool HasHasbit() const; + bool IsRealOneof() const; + bool BitfieldTracksMutability() const; + int GetBitIndex() const { return bit_index_; } constexpr int GetNumBits() const { return 1; } virtual void GenerateInterfaceMembers(io::Printer* printer) const = 0; diff --git a/src/google/protobuf/compiler/java/full/make_field_gens.cc b/src/google/protobuf/compiler/java/full/make_field_gens.cc index e88deb6a3b..30cbff12c8 100644 --- a/src/google/protobuf/compiler/java/full/make_field_gens.cc +++ b/src/google/protobuf/compiler/java/full/make_field_gens.cc @@ -86,7 +86,15 @@ std::unique_ptr MakeImmutableGenerator( } } } - +bool HasExplicitPresence(const FieldDescriptor* field) { + return HasHasbit(field); +} +bool HasNoPresence(const FieldDescriptor* field) { + return IsRealOneof(field) || BitfieldTracksMutability(field); +} +bool HasHintBitFields(const FieldDescriptor* field) { + return !HasExplicitPresence(field) && !HasNoPresence(field); +} } // namespace FieldGeneratorMap MakeImmutableFieldGenerators( @@ -95,11 +103,35 @@ FieldGeneratorMap MakeImmutableFieldGenerators( // bit fields. int bit_index = 0; FieldGeneratorMap ret(descriptor); + + // First pass: fields with real presence bits. for (int i = 0; i < descriptor->field_count(); i++) { const FieldDescriptor* field = descriptor->field(i); - auto generator = MakeImmutableGenerator(field, bit_index, context); - bit_index += generator->GetNumBits(); - ret.Add(field, std::move(generator)); + if (HasExplicitPresence(field)) { + auto generator = MakeImmutableGenerator(field, bit_index, context); + bit_index += generator->GetNumBits(); + ret.Add(field, std::move(generator)); + } + } + + // Second pass: fields with hint presence bits. + for (int i = 0; i < descriptor->field_count(); i++) { + const FieldDescriptor* field = descriptor->field(i); + if (HasHintBitFields(field)) { + auto generator = MakeImmutableGenerator(field, bit_index, context); + bit_index += generator->GetNumBits(); + ret.Add(field, std::move(generator)); + } + } + + // Third pass: fields with no presence tracking. + for (int i = 0; i < descriptor->field_count(); i++) { + const FieldDescriptor* field = descriptor->field(i); + if (HasNoPresence(field)) { + auto generator = MakeImmutableGenerator(field, bit_index, context); + bit_index += generator->GetNumBits(); + ret.Add(field, std::move(generator)); + } } return ret; } diff --git a/src/google/protobuf/compiler/java/full/message_builder.cc b/src/google/protobuf/compiler/java/full/message_builder.cc index 67814722fc..801e67f6e8 100644 --- a/src/google/protobuf/compiler/java/full/message_builder.cc +++ b/src/google/protobuf/compiler/java/full/message_builder.cc @@ -18,10 +18,7 @@ #include #include "absl/container/btree_map.h" -#include "absl/container/btree_set.h" -#include "absl/container/flat_hash_map.h" #include "absl/log/absl_check.h" -#include "absl/strings/ascii.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_replace.h" #include "absl/strings/string_view.h" @@ -30,7 +27,7 @@ #include "google/protobuf/compiler/java/context.h" #include "google/protobuf/compiler/java/doc_comment.h" #include "google/protobuf/compiler/java/field_common.h" -#include "google/protobuf/compiler/java/generator_factory.h" +#include "google/protobuf/compiler/java/generator_common.h" #include "google/protobuf/compiler/java/helpers.h" #include "google/protobuf/compiler/java/full/enum.h" #include "google/protobuf/compiler/java/full/extension.h" @@ -584,9 +581,8 @@ void MessageBuilderGenerator::GenerateBuildPartial(io::Printer* printer) { "classname", name_resolver_->GetImmutableClassName(descriptor_)); // Build all fields in shards organized by bitfield membership. - int start_field = 0; for (int i = 0; i < totalInts; i++) { - start_field = GenerateBuildPartialShard(printer, i, start_field); + GenerateBuildPartialShard(printer, i); } // Build Oneofs @@ -603,50 +599,35 @@ void MessageBuilderGenerator::GenerateBuildPartial(io::Printer* printer) { } } -int MessageBuilderGenerator::GenerateBuildPartialShard(io::Printer* printer, - int shard, - int first_field) { +void MessageBuilderGenerator::GenerateBuildPartialShard(io::Printer* printer, + int shard) { printer->Print( "private void buildPartial_autosplit_$shard$($classname$ result) {\n" - " int from_$bit_field_name$ = $bit_field_name$;\n", + " int from_$bit_field_name$ = $bit_field_name$;\n" + " int to_$bit_field_name$ = 0;\n", "classname", name_resolver_->GetImmutableClassName(descriptor_), "shard", absl::StrCat(shard), "bit_field_name", GetBitFieldName(shard)); printer->Indent(); - absl::btree_set declared_to_bitfields; - - int bit = 0; - int next = first_field; - for (; bit < 32 && next < descriptor_->field_count(); ++next, ++bit) { + int i = shard * 32; + int shard_end = std::min(i + 32, static_cast(field_generators_.size())); + for (; i < shard_end; ++i) { const ImmutableFieldGenerator& field = - field_generators_.get(descriptor_->field(next)); + field_generators_.getInInsertOrder(i); - // Skip oneof fields that are handled separately - if (IsRealOneof(descriptor_->field(next))) { + // Currently oneofs are not built in shards. + if (field.IsRealOneof()) { continue; } - - // Track message bits if necessary - int to_bitfield = field.GetBitIndex() / 32; - if (declared_to_bitfields.count(to_bitfield) == 0) { - printer->Print("int to_$bit_field_name$ = 0;\n", "bit_field_name", - GetBitFieldName(to_bitfield)); - declared_to_bitfields.insert(to_bitfield); - } - // Copy the field from the builder to the message field.GenerateBuildingCode(printer); } + printer->Outdent(); // Copy the bit field results to the generated message - for (int to_bitfield : declared_to_bitfields) { - printer->Print("result.$bit_field_name$ |= to_$bit_field_name$;\n", - "bit_field_name", GetBitFieldName(to_bitfield)); - } - - printer->Outdent(); - printer->Print("}\n\n"); - - return next; + printer->Print( + " result.$bit_field_name$ |= to_$bit_field_name$;\n" + "}\n\n", + "bit_field_name", GetBitFieldName(shard)); } // =================================================================== diff --git a/src/google/protobuf/compiler/java/full/message_builder.h b/src/google/protobuf/compiler/java/full/message_builder.h index e793fe3f12..b338f90454 100644 --- a/src/google/protobuf/compiler/java/full/message_builder.h +++ b/src/google/protobuf/compiler/java/full/message_builder.h @@ -14,11 +14,11 @@ #include #include -#include #include "absl/container/btree_map.h" #include "absl/strings/string_view.h" #include "absl/types/span.h" +#include "google/protobuf/compiler/java/generator_common.h" #include "google/protobuf/compiler/java/full/field_generator.h" #include "google/protobuf/compiler/java/full/oneof_generator.h" #include "google/protobuf/descriptor.h" @@ -67,8 +67,7 @@ class MessageBuilderGenerator { io::Printer* printer, absl::Span merging_code_blocks, absl::string_view method_suffix); void GenerateBuildPartial(io::Printer* printer); - int GenerateBuildPartialShard(io::Printer* printer, int shard, - int first_field); + void GenerateBuildPartialShard(io::Printer* printer, int shard); void GenerateDescriptorMethods(io::Printer* printer); void GenerateBuilderParsingMethods(io::Printer* printer); void GenerateBuilderFieldParsingCases(io::Printer* printer); diff --git a/src/google/protobuf/compiler/java/generator_common.h b/src/google/protobuf/compiler/java/generator_common.h index 7ec12c16f5..0ff66d9e60 100644 --- a/src/google/protobuf/compiler/java/generator_common.h +++ b/src/google/protobuf/compiler/java/generator_common.h @@ -30,11 +30,12 @@ class FieldGeneratorMap { public: explicit FieldGeneratorMap(const Descriptor* descriptor) : descriptor_(descriptor) { - field_generators_.reserve(static_cast(descriptor->field_count())); + insert_order_.reserve(static_cast(descriptor->field_count())); + index_order_.resize(static_cast(descriptor->field_count())); } ~FieldGeneratorMap() { - for (const auto* g : field_generators_) { + for (const auto* g : insert_order_) { delete g; } } @@ -45,21 +46,28 @@ class FieldGeneratorMap { FieldGeneratorMap(const FieldGeneratorMap&) = delete; FieldGeneratorMap& operator=(const FieldGeneratorMap&) = delete; + size_t size() const { return insert_order_.size(); } + void Add(const FieldDescriptor* field, std::unique_ptr field_generator) { ABSL_CHECK_EQ(field->containing_type(), descriptor_); - field_generators_.push_back(field_generator.release()); + insert_order_.push_back(field_generator.release()); + index_order_[static_cast(field->index())] = insert_order_.back(); } const FieldGeneratorType& get(const FieldDescriptor* field) const { ABSL_CHECK_EQ(field->containing_type(), descriptor_); - return *field_generators_[static_cast(field->index())]; + return *index_order_[static_cast(field->index())]; + } + + const FieldGeneratorType& getInInsertOrder(int index) const { + return *insert_order_[static_cast(index)]; } std::vector field_generators() const { std::vector field_generators; - field_generators.reserve(field_generators_.size()); - for (const auto* g : field_generators_) { + field_generators.reserve(index_order_.size()); + for (const auto* g : index_order_) { field_generators.push_back(g); } return field_generators; @@ -67,7 +75,8 @@ class FieldGeneratorMap { private: const Descriptor* descriptor_; - std::vector field_generators_; + std::vector insert_order_; + std::vector index_order_; }; inline void ReportUnexpectedPackedFieldsCall() { diff --git a/src/google/protobuf/compiler/java/helpers.cc b/src/google/protobuf/compiler/java/helpers.cc index bd3ed9a627..2dfe9d67fc 100644 --- a/src/google/protobuf/compiler/java/helpers.cc +++ b/src/google/protobuf/compiler/java/helpers.cc @@ -831,15 +831,33 @@ bool HasRequiredFields(const Descriptor* type, return false; } -bool HasRequiredFields(const Descriptor* type) { +bool HasRequiredFields(const Descriptor* descriptor) { absl::flat_hash_set already_seen; - return HasRequiredFields(type, &already_seen); + return HasRequiredFields(descriptor, &already_seen); } bool IsRealOneof(const FieldDescriptor* descriptor) { return descriptor->real_containing_oneof(); } +bool BitfieldTracksMutability(const FieldDescriptor* const descriptor) { + if (!descriptor->is_repeated() || IsMapField(descriptor)) { + return false; + } + // TODO: update this to migrate repeated fields to use + // ProtobufList (which tracks immutability internally). That allows us to use + // the presence bit to skip work on the repeated field if it is not populated. + // Once all repeated fields are held in ProtobufLists, this method shouldn't + // be needed. + switch (descriptor->type()) { + case FieldDescriptor::TYPE_GROUP: + case FieldDescriptor::TYPE_MESSAGE: + return true; + default: + return false; + } +} + bool HasRepeatedFields(const Descriptor* descriptor) { for (int i = 0; i < descriptor->field_count(); ++i) { const FieldDescriptor* field = descriptor->field(i); diff --git a/src/google/protobuf/compiler/java/helpers.h b/src/google/protobuf/compiler/java/helpers.h index 13060f17c3..9403b1159e 100644 --- a/src/google/protobuf/compiler/java/helpers.h +++ b/src/google/protobuf/compiler/java/helpers.h @@ -351,6 +351,8 @@ inline bool HasHasbit(const FieldDescriptor* descriptor) { return descriptor->has_presence() && !descriptor->real_containing_oneof(); } +bool BitfieldTracksMutability(const FieldDescriptor* descriptor); + // Whether unknown enum values are kept (i.e., not stored in UnknownFieldSet // but in the message and can be queried using additional getters that return // ints.