Skip to content

Commit

Permalink
Migrate repeated numeric fields to use bit field to track presence in…
Browse files Browse the repository at this point in the history 
…stead of using it to indicate the field mutability to potentially skip work on the field during build operations.

PiperOrigin-RevId: 531589847
  • Loading branch information
@protobuf-github-bot @copybara-github
protobuf-github-bot authored and copybara-github committed May 12, 2023
1 parent ad67472 commit f0de774
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 35 deletions.
9 changes: 6 additions & 3 deletions src/google/protobuf/compiler/java/message_builder.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,13 @@ bool BitfieldTracksMutability(const FieldDescriptor* const descriptor) {
// Once all repeated fields are held in ProtobufLists, this method shouldn't
// be needed.
switch (descriptor->type()) {
case FieldDescriptor::TYPE_STRING:
return false;
default:
case FieldDescriptor::TYPE_GROUP:
case FieldDescriptor::TYPE_MESSAGE:
case FieldDescriptor::TYPE_ENUM:
case FieldDescriptor::TYPE_BYTES:
return true;
default:
return false;
}
}
} // namespace
Expand Down
114 changes: 82 additions & 32 deletions src/google/protobuf/compiler/java/primitive_field.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,10 @@ void SetPrimitiveVariables(
break;
}
}

// For repeated builders, one bit is used for whether the array is immutable.
// For repeated bytes builders, one bit is used for whether the array is
// mutable.
// TODO(b/255468704): migrate bytes field to ProtobufList so that we can
// use the bit field to track their presence instead of mutability.
(*variables)["get_mutable_bit_builder"] = GenerateGetBit(builderBitIndex);
(*variables)["set_mutable_bit_builder"] = GenerateSetBit(builderBitIndex);
(*variables)["clear_mutable_bit_builder"] = GenerateClearBit(builderBitIndex);
Expand Down Expand Up @@ -692,8 +694,10 @@ void RepeatedImmutablePrimitiveFieldGenerator::GenerateInterfaceMembers(

void RepeatedImmutablePrimitiveFieldGenerator::GenerateMembers(
io::Printer* printer) const {
printer->Print(variables_, "@SuppressWarnings(\"serial\")\n"
"private $field_list_type$ $name$_;\n");
printer->Print(variables_,
"@SuppressWarnings(\"serial\")\n"
"private $field_list_type$ $name$_ =\n"
" $empty_list$;\n");
PrintExtraFieldInfo(variables_, printer);
WriteFieldAccessorDocComment(printer, descriptor_, LIST_GETTER);
printer->Print(variables_,
Expand Down Expand Up @@ -738,26 +742,45 @@ void RepeatedImmutablePrimitiveFieldGenerator::GenerateBuilderMembers(
printer->Print(variables_,
"private $field_list_type$ $name$_ = $empty_list$;\n");

printer->Print(variables_,
"private void ensure$capitalized_name$IsMutable() {\n"
" if (!$get_mutable_bit_builder$) {\n"
" $name$_ = $mutable_copy_list$;\n"
" $set_mutable_bit_builder$;\n"
" }\n"
"}\n");
if (descriptor_->type() == FieldDescriptor::TYPE_BYTES) {
printer->Print(variables_,
"private void ensure$capitalized_name$IsMutable() {\n"
" if (!$get_mutable_bit_builder$) {\n"
" $name$_ = $mutable_copy_list$;\n"
" $set_mutable_bit_builder$;\n"
" }\n"
"}\n");
} else {
printer->Print(variables_,
"private void ensure$capitalized_name$IsMutable() {\n"
" if (!$name$_.isModifiable()) {\n"
" $name$_ = $mutable_copy_list$;\n"
" }\n"
" $set_has_field_bit_builder$\n"
"}\n");
}

// Note: We return an unmodifiable list because otherwise the caller
// could hold on to the returned list and modify it after the message
// has been built, thus mutating the message which is supposed to be
// immutable.
WriteFieldAccessorDocComment(printer, descriptor_, LIST_GETTER);
printer->Print(
variables_,
"$deprecation$public java.util.List<$boxed_type$>\n"
" ${$get$capitalized_name$List$}$() {\n"
" return $get_mutable_bit_builder$ ?\n"
" java.util.Collections.unmodifiableList($name$_) : $name$_;\n"
"}\n");
if (descriptor_->type() == FieldDescriptor::TYPE_BYTES) {
printer->Print(variables_,
"$deprecation$public java.util.List<$boxed_type$>\n"
" ${$get$capitalized_name$List$}$() {\n"
" return $get_mutable_bit_builder$ ?\n"
" java.util.Collections.unmodifiableList($name$_) "
": $name$_;\n"
"}\n");
} else {
printer->Print(variables_,
"$deprecation$public java.util.List<$boxed_type$>\n"
" ${$get$capitalized_name$List$}$() {\n"
" $name$_.makeImmutable();\n"
" return $name$_;\n"
"}\n");
}
printer->Annotate("{", "}", descriptor_);
WriteFieldAccessorDocComment(printer, descriptor_, LIST_COUNT);
printer->Print(
Expand All @@ -780,35 +803,47 @@ void RepeatedImmutablePrimitiveFieldGenerator::GenerateBuilderMembers(
" int index, $type$ value) {\n"
" $null_check$\n"
" ensure$capitalized_name$IsMutable();\n"
" $repeated_set$(index, value);\n"
" $repeated_set$(index, value);\n");
printer->Annotate("{", "}", descriptor_);
if (descriptor_->type() != FieldDescriptor::TYPE_BYTES) {
printer->Print(variables_, " $set_has_field_bit_builder$\n");
}
printer->Print(variables_,
" $on_changed$\n"
" return this;\n"
"}\n");
printer->Annotate("{", "}", descriptor_);
WriteFieldAccessorDocComment(printer, descriptor_, LIST_ADDER,
/* builder */ true);
printer->Print(variables_,
"$deprecation$public Builder "
"${$add$capitalized_name$$}$($type$ value) {\n"
" $null_check$\n"
" ensure$capitalized_name$IsMutable();\n"
" $repeated_add$(value);\n"
" $repeated_add$(value);\n");
printer->Annotate("{", "}", descriptor_);
if (descriptor_->type() != FieldDescriptor::TYPE_BYTES) {
printer->Print(variables_, " $set_has_field_bit_builder$\n");
}
printer->Print(variables_,
" $on_changed$\n"
" return this;\n"
"}\n");
printer->Annotate("{", "}", descriptor_);
WriteFieldAccessorDocComment(printer, descriptor_, LIST_MULTI_ADDER,
/* builder */ true);
printer->Print(variables_,
"$deprecation$public Builder ${$addAll$capitalized_name$$}$(\n"
" java.lang.Iterable<? extends $boxed_type$> values) {\n"
" ensure$capitalized_name$IsMutable();\n"
" com.google.protobuf.AbstractMessageLite.Builder.addAll(\n"
" values, $name$_);\n"
" values, $name$_);\n");
printer->Annotate("{", "}", descriptor_);
if (descriptor_->type() != FieldDescriptor::TYPE_BYTES) {
printer->Print(variables_, " $set_has_field_bit_builder$\n");
}
printer->Print(variables_,
" $on_changed$\n"
" return this;\n"
"}\n");
printer->Annotate("{", "}", descriptor_);
WriteFieldAccessorDocComment(printer, descriptor_, CLEARER,
/* builder */ true);
printer->Print(
Expand Down Expand Up @@ -941,8 +976,15 @@ void RepeatedImmutablePrimitiveFieldGenerator::GenerateMergingCode(
printer->Print(variables_,
"if (!other.$name$_.isEmpty()) {\n"
" if ($name$_.isEmpty()) {\n"
" $name$_ = other.$name$_;\n"
" $clear_mutable_bit_builder$;\n"
" $name$_ = other.$name$_;\n");
if (descriptor_->type() == FieldDescriptor::TYPE_BYTES) {
printer->Print(variables_, " $clear_mutable_bit_builder$;\n");
} else {
printer->Print(variables_,
" $name$_.makeImmutable();\n"
" $set_has_field_bit_builder$\n");
}
printer->Print(variables_,
" } else {\n"
" ensure$capitalized_name$IsMutable();\n"
" $name$_.addAll(other.$name$_);\n"
Expand All @@ -955,12 +997,20 @@ void RepeatedImmutablePrimitiveFieldGenerator::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_make_immutable$;\n"
" $clear_mutable_bit_builder$;\n"
"}\n"
"result.$name$_ = $name$_;\n");
if (descriptor_->type() == FieldDescriptor::TYPE_BYTES) {
printer->Print(variables_,
"if ($get_mutable_bit_builder$) {\n"
" $name_make_immutable$;\n"
" $clear_mutable_bit_builder$;\n"
"}\n"
"result.$name$_ = $name$_;\n");
} else {
printer->Print(variables_,
"if ($get_has_field_bit_from_local$) {\n"
" $name_make_immutable$;\n"
" result.$name$_ = $name$_;\n"
"}\n");
}
}

void RepeatedImmutablePrimitiveFieldGenerator::GenerateBuilderParsingCode(
Expand Down

0 comments on commit f0de774

Please sign in to comment.