Skip to content

Commit

Permalink
Use LazyStringArrayList directly in gencode -- LazyStringArrayList tr…
Browse files Browse the repository at this point in the history 
…acks mutability internally, so we can use the current "mutable bit" as a "has bit" to potentially skip work on the field during build operations.

PiperOrigin-RevId: 501083441
  • Loading branch information
@protobuf-github-bot @copybara-github
protobuf-github-bot authored and copybara-github committed Jan 10, 2023
1 parent 0517b71 commit 9dee1ca
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 34 deletions.
26 changes: 20 additions & 6 deletions src/google/protobuf/compiler/java/message_builder.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,23 @@ std::string MapValueImmutableClassdName(const Descriptor* descriptor,
GOOGLE_ABSL_CHECK_EQ(FieldDescriptor::TYPE_MESSAGE, value_field->type());
return name_resolver->GetImmutableClassName(value_field->message_type());
}

bool BitfieldTracksMutability(const FieldDescriptor* const descriptor) {
if (!descriptor->is_repeated() || IsMapField(descriptor)) {
return false;
}
// TODO(b/255468704): 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_STRING:
return false;
default:
return true;
}
}
} // namespace

MessageBuilderGenerator::MessageBuilderGenerator(const Descriptor* descriptor,
Expand Down Expand Up @@ -579,8 +596,7 @@ void MessageBuilderGenerator::GenerateBuildPartial(io::Printer* printer) {
// Handle the repeated fields first so that the "mutable bits" are cleared.
bool has_repeated_fields = false;
for (int i = 0; i < descriptor_->field_count(); ++i) {
if (descriptor_->field(i)->is_repeated() &&
!IsMapField(descriptor_->field(i))) {
if (BitfieldTracksMutability(descriptor_->field(i))) {
has_repeated_fields = true;
printer->Print("buildPartialRepeatedFields(result);\n");
break;
Expand Down Expand Up @@ -616,8 +632,7 @@ void MessageBuilderGenerator::GenerateBuildPartial(io::Printer* printer) {
"classname", name_resolver_->GetImmutableClassName(descriptor_));
printer->Indent();
for (int i = 0; i < descriptor_->field_count(); ++i) {
if (descriptor_->field(i)->is_repeated() &&
!IsMapField(descriptor_->field(i))) {
if (BitfieldTracksMutability(descriptor_->field(i))) {
const ImmutableFieldGenerator& field =
field_generators_.get(descriptor_->field(i));
field.GenerateBuildingCode(printer);
Expand Down Expand Up @@ -683,8 +698,7 @@ int MessageBuilderGenerator::GenerateBuildPartialPiece(io::Printer* printer,

// Skip repeated fields because they are currently handled
// in separate buildPartial sub-methods.
if (descriptor_->field(next)->is_repeated() &&
!IsMapField(descriptor_->field(next))) {
if (BitfieldTracksMutability(descriptor_->field(next))) {
continue;
}
// Skip fields without presence bits in the builder
Expand Down
58 changes: 30 additions & 28 deletions src/google/protobuf/compiler/java/string_field.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ void SetPrimitiveVariables(
Context* context) {
SetCommonFieldVariables(descriptor, info, variables);

(*variables)["empty_list"] = "com.google.protobuf.LazyStringArrayList.EMPTY";
(*variables)["empty_list"] =
"com.google.protobuf.LazyStringArrayList.emptyList()";

(*variables)["default"] =
ImmutableDefaultValue(descriptor, name_resolver, context->options());
Expand Down Expand Up @@ -121,11 +122,6 @@ void SetPrimitiveVariables(
(*variables)["name"], "_)")});
}

// For repeated builders, one bit is used for whether the array is immutable.
(*variables)["get_mutable_bit_builder"] = GenerateGetBit(builderBitIndex);
(*variables)["set_mutable_bit_builder"] = GenerateSetBit(builderBitIndex);
(*variables)["clear_mutable_bit_builder"] = GenerateClearBit(builderBitIndex);

(*variables)["get_has_field_bit_builder"] = GenerateGetBit(builderBitIndex);
(*variables)["get_has_field_bit_from_local"] =
GenerateGetBitFromLocal(builderBitIndex);
Expand Down Expand Up @@ -197,8 +193,7 @@ int ImmutableStringFieldGenerator::GetNumBitsForBuilder() const { return 1; }
// separate fields but rather use dynamic type checking.
//
// For single fields, the logic for this is done inside the generated code. For
// repeated fields, the logic is done in LazyStringArrayList and
// UnmodifiableLazyStringList.
// repeated fields, the logic is done in LazyStringArrayList.
void ImmutableStringFieldGenerator::GenerateInterfaceMembers(
io::Printer* printer) const {
if (HasHazzer(descriptor_)) {
Expand Down Expand Up @@ -794,7 +789,8 @@ void RepeatedImmutableStringFieldGenerator::GenerateMembers(
io::Printer* printer) const {
printer->Print(variables_,
"@SuppressWarnings(\"serial\")\n"
"private com.google.protobuf.LazyStringList $name$_;\n");
"private com.google.protobuf.LazyStringArrayList $name$_ =\n"
" $empty_list$;\n");
PrintExtraFieldInfo(variables_, printer);
WriteFieldAccessorDocComment(printer, descriptor_, LIST_GETTER);
printer->Print(variables_,
Expand Down Expand Up @@ -838,17 +834,17 @@ void RepeatedImmutableStringFieldGenerator::GenerateBuilderMembers(
// memory allocations. Note, immutable is a strong guarantee here -- not
// just that the list cannot be modified via the reference but that the
// list can never be modified.
printer->Print(
variables_,
"private com.google.protobuf.LazyStringList $name$_ = $empty_list$;\n");
printer->Print(variables_,
"private com.google.protobuf.LazyStringArrayList $name$_ =\n"
" $empty_list$;\n");

printer->Print(
variables_,
"private void ensure$capitalized_name$IsMutable() {\n"
" if (!$get_mutable_bit_builder$) {\n"
" if (!$name$_.isModifiable()) {\n"
" $name$_ = new com.google.protobuf.LazyStringArrayList($name$_);\n"
" $set_mutable_bit_builder$;\n"
" }\n"
" }\n"
" $set_has_field_bit_builder$\n"
"}\n");

// Note: We return an unmodifiable list because otherwise the caller
Expand All @@ -859,7 +855,8 @@ void RepeatedImmutableStringFieldGenerator::GenerateBuilderMembers(
printer->Print(variables_,
"$deprecation$public com.google.protobuf.ProtocolStringList\n"
" ${$get$capitalized_name$List$}$() {\n"
" return $name$_.getUnmodifiableView();\n"
" $name$_.makeImmutable();\n"
" return $name$_;\n"
"}\n");
printer->Annotate("{", "}", descriptor_);
WriteFieldAccessorDocComment(printer, descriptor_, LIST_COUNT);
Expand Down Expand Up @@ -892,6 +889,7 @@ void RepeatedImmutableStringFieldGenerator::GenerateBuilderMembers(
" $null_check$\n"
" ensure$capitalized_name$IsMutable();\n"
" $name$_.set(index, value);\n"
" $set_has_field_bit_builder$\n"
" $on_changed$\n"
" return this;\n"
"}\n");
Expand All @@ -904,6 +902,7 @@ void RepeatedImmutableStringFieldGenerator::GenerateBuilderMembers(
" $null_check$\n"
" ensure$capitalized_name$IsMutable();\n"
" $name$_.add(value);\n"
" $set_has_field_bit_builder$\n"
" $on_changed$\n"
" return this;\n"
"}\n");
Expand All @@ -916,6 +915,7 @@ void RepeatedImmutableStringFieldGenerator::GenerateBuilderMembers(
" ensure$capitalized_name$IsMutable();\n"
" com.google.protobuf.AbstractMessageLite.Builder.addAll(\n"
" values, $name$_);\n"
" $set_has_field_bit_builder$\n"
" $on_changed$\n"
" return this;\n"
"}\n");
Expand All @@ -925,8 +925,9 @@ void RepeatedImmutableStringFieldGenerator::GenerateBuilderMembers(
printer->Print(
variables_,
"$deprecation$public Builder ${$clear$capitalized_name$$}$() {\n"
" $name$_ = $empty_list$;\n"
" $clear_mutable_bit_builder$;\n"
" $name$_ =\n"
" $empty_list$;\n"
" $clear_has_field_bit_builder$;\n"
" $on_changed$\n"
" return this;\n"
"}\n");
Expand All @@ -946,6 +947,7 @@ void RepeatedImmutableStringFieldGenerator::GenerateBuilderMembers(
printer->Print(variables_,
" ensure$capitalized_name$IsMutable();\n"
" $name$_.add(value);\n"
" $set_has_field_bit_builder$\n"
" $on_changed$\n"
" return this;\n"
"}\n");
Expand Down Expand Up @@ -1060,14 +1062,16 @@ void RepeatedImmutableStringFieldGenerator::

void RepeatedImmutableStringFieldGenerator::GenerateInitializationCode(
io::Printer* printer) const {
printer->Print(variables_, "$name$_ = $empty_list$;\n");
printer->Print(variables_,
"$name$_ =\n"
" $empty_list$;\n");
}

void RepeatedImmutableStringFieldGenerator::GenerateBuilderClearCode(
io::Printer* printer) const {
printer->Print(variables_,
"$name$_ = $empty_list$;\n"
"$clear_mutable_bit_builder$;\n");
"$name$_ =\n"
" $empty_list$;\n");
}

void RepeatedImmutableStringFieldGenerator::GenerateMergingCode(
Expand All @@ -1081,7 +1085,7 @@ void RepeatedImmutableStringFieldGenerator::GenerateMergingCode(
"if (!other.$name$_.isEmpty()) {\n"
" if ($name$_.isEmpty()) {\n"
" $name$_ = other.$name$_;\n"
" $clear_mutable_bit_builder$;\n"
" $set_has_field_bit_builder$\n"
" } else {\n"
" ensure$capitalized_name$IsMutable();\n"
" $name$_.addAll(other.$name$_);\n"
Expand All @@ -1094,13 +1098,11 @@ void RepeatedImmutableStringFieldGenerator::GenerateBuildingCode(
io::Printer* printer) const {
// The code below ensures that the result has an immutable list. If our
// list is immutable, we can just reuse it. If not, we make it immutable.

printer->Print(variables_,
"if ($get_mutable_bit_builder$) {\n"
" $name$_ = $name$_.getUnmodifiableView();\n"
" $clear_mutable_bit_builder$;\n"
"}\n"
"result.$name$_ = $name$_;\n");
"if ($get_has_field_bit_from_local$) {\n"
" $name$_.makeImmutable();\n"
" result.$name$_ = $name$_;\n"
"}\n");
}

void RepeatedImmutableStringFieldGenerator::GenerateBuilderParsingCode(
Expand Down

0 comments on commit 9dee1ca

Please sign in to comment.