Skip to content

Commit 2326aef

Browse files
Use bit-field int values in buildPartial to skip work on unset groups of fields.
Changes to make this improvement: 1) All non-repeated builder fields (including maps) now have a presence bit regardless of syntax. 2) The buildPartial method is split into one method per 32-field block (aligned with bit-mask ints) 3) If a presence bit-mask int is set to 0, no fields are present, so we can skip the logic for all of those fields in the buildPartial step. For messages with a lot of fields (> 100) that are sparsely populated this can result in a significant improvement. Not only does it potentially skip a lot of field copying logic, but also breaks the buildPartial method into chunks that should be more easily digested by the JIT compiler as discussed in this issue: #10247. PiperOrigin-RevId: 485952448
1 parent 9542560 commit 2326aef

File tree

13 files changed

+595
-496
lines changed

13 files changed

+595
-496
lines changed

src/google/protobuf/compiler/java/enum_field.cc

Lines changed: 37 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -91,22 +91,15 @@ void SetEnumVariables(
9191
if (HasHasbit(descriptor)) {
9292
// For singular messages and builders, one bit is used for the hasField bit.
9393
(*variables)["get_has_field_bit_message"] = GenerateGetBit(messageBitIndex);
94-
(*variables)["get_has_field_bit_builder"] = GenerateGetBit(builderBitIndex);
95-
9694
// Note that these have a trailing ";".
9795
(*variables)["set_has_field_bit_message"] =
9896
GenerateSetBit(messageBitIndex) + ";";
99-
(*variables)["set_has_field_bit_builder"] =
100-
GenerateSetBit(builderBitIndex) + ";";
101-
(*variables)["clear_has_field_bit_builder"] =
102-
GenerateClearBit(builderBitIndex) + ";";
103-
97+
(*variables)["set_has_field_bit_to_local"] =
98+
GenerateSetBitToLocal(messageBitIndex);
10499
(*variables)["is_field_present_message"] = GenerateGetBit(messageBitIndex);
105100
} else {
106101
(*variables)["set_has_field_bit_message"] = "";
107-
(*variables)["set_has_field_bit_builder"] = "";
108-
(*variables)["clear_has_field_bit_builder"] = "";
109-
102+
(*variables)["set_has_field_bit_to_local"] = "";
110103
variables->insert({"is_field_present_message",
111104
absl::StrCat((*variables)["name"], "_ != ",
112105
(*variables)["default"], ".getNumber()")});
@@ -117,10 +110,15 @@ void SetEnumVariables(
117110
(*variables)["set_mutable_bit_builder"] = GenerateSetBit(builderBitIndex);
118111
(*variables)["clear_mutable_bit_builder"] = GenerateClearBit(builderBitIndex);
119112

113+
(*variables)["get_has_field_bit_builder"] = GenerateGetBit(builderBitIndex);
114+
115+
// Note that these have a trailing ";".
116+
(*variables)["set_has_field_bit_builder"] =
117+
GenerateSetBit(builderBitIndex) + ";";
118+
(*variables)["clear_has_field_bit_builder"] =
119+
GenerateClearBit(builderBitIndex) + ";";
120120
(*variables)["get_has_field_bit_from_local"] =
121121
GenerateGetBitFromLocal(builderBitIndex);
122-
(*variables)["set_has_field_bit_to_local"] =
123-
GenerateSetBitToLocal(messageBitIndex);
124122

125123
if (SupportUnknownEnumValue(descriptor->file())) {
126124
variables->insert(
@@ -137,21 +135,30 @@ void SetEnumVariables(
137135
ImmutableEnumFieldGenerator::ImmutableEnumFieldGenerator(
138136
const FieldDescriptor* descriptor, int messageBitIndex, int builderBitIndex,
139137
Context* context)
140-
: descriptor_(descriptor), name_resolver_(context->GetNameResolver()) {
138+
: descriptor_(descriptor),
139+
message_bit_index_(messageBitIndex),
140+
builder_bit_index_(builderBitIndex),
141+
name_resolver_(context->GetNameResolver()) {
141142
SetEnumVariables(descriptor, messageBitIndex, builderBitIndex,
142143
context->GetFieldGeneratorInfo(descriptor), name_resolver_,
143144
&variables_, context);
144145
}
145146

146147
ImmutableEnumFieldGenerator::~ImmutableEnumFieldGenerator() {}
147148

149+
int ImmutableEnumFieldGenerator::GetMessageBitIndex() const {
150+
return message_bit_index_;
151+
}
152+
153+
int ImmutableEnumFieldGenerator::GetBuilderBitIndex() const {
154+
return builder_bit_index_;
155+
}
156+
148157
int ImmutableEnumFieldGenerator::GetNumBitsForMessage() const {
149158
return HasHasbit(descriptor_) ? 1 : 0;
150159
}
151160

152-
int ImmutableEnumFieldGenerator::GetNumBitsForBuilder() const {
153-
return GetNumBitsForMessage();
154-
}
161+
int ImmutableEnumFieldGenerator::GetNumBitsForBuilder() const { return 1; }
155162

156163
void ImmutableEnumFieldGenerator::GenerateInterfaceMembers(
157164
io::Printer* printer) const {
@@ -170,7 +177,7 @@ void ImmutableEnumFieldGenerator::GenerateInterfaceMembers(
170177
}
171178

172179
void ImmutableEnumFieldGenerator::GenerateMembers(io::Printer* printer) const {
173-
printer->Print(variables_, "private int $name$_;\n");
180+
printer->Print(variables_, "private int $name$_ = $default_number$;\n");
174181
PrintExtraFieldInfo(variables_, printer);
175182
if (HasHazzer(descriptor_)) {
176183
WriteFieldAccessorDocComment(printer, descriptor_, HAZZER);
@@ -225,8 +232,8 @@ void ImmutableEnumFieldGenerator::GenerateBuilderMembers(
225232
printer->Print(variables_,
226233
"$deprecation$public Builder "
227234
"${$set$capitalized_name$Value$}$(int value) {\n"
228-
" $set_has_field_bit_builder$\n"
229235
" $name$_ = value;\n"
236+
" $set_has_field_bit_builder$\n"
230237
" onChanged();\n"
231238
" return this;\n"
232239
"}\n");
@@ -321,9 +328,7 @@ void ImmutableEnumFieldGenerator::GenerateInitializationCode(
321328

322329
void ImmutableEnumFieldGenerator::GenerateBuilderClearCode(
323330
io::Printer* printer) const {
324-
printer->Print(variables_,
325-
"$name$_ = $default_number$;\n"
326-
"$clear_has_field_bit_builder$\n");
331+
printer->Print(variables_, "$name$_ = $default_number$;\n");
327332
}
328333

329334
void ImmutableEnumFieldGenerator::GenerateMergingCode(
@@ -346,13 +351,13 @@ void ImmutableEnumFieldGenerator::GenerateMergingCode(
346351

347352
void ImmutableEnumFieldGenerator::GenerateBuildingCode(
348353
io::Printer* printer) const {
349-
if (HasHazzer(descriptor_)) {
350-
printer->Print(variables_,
351-
"if ($get_has_field_bit_from_local$) {\n"
352-
" $set_has_field_bit_to_local$;\n"
353-
"}\n");
354+
printer->Print(variables_,
355+
"if ($get_has_field_bit_from_local$) {\n"
356+
" result.$name$_ = $name$_;\n");
357+
if (GetNumBitsForMessage() > 0) {
358+
printer->Print(variables_, " $set_has_field_bit_to_local$;\n");
354359
}
355-
printer->Print(variables_, "result.$name$_ = $name$_;\n");
360+
printer->Print("}\n");
356361
}
357362

358363
void ImmutableEnumFieldGenerator::GenerateBuilderParsingCode(
@@ -504,6 +509,7 @@ void ImmutableEnumOneofFieldGenerator::GenerateBuilderMembers(
504509
" return $default$;\n"
505510
"}\n");
506511
printer->Annotate("{", "}", descriptor_);
512+
507513
WriteFieldAccessorDocComment(printer, descriptor_, SETTER,
508514
/* builder */ true);
509515
printer->Print(variables_,
@@ -518,6 +524,7 @@ void ImmutableEnumOneofFieldGenerator::GenerateBuilderMembers(
518524
" return this;\n"
519525
"}\n");
520526
printer->Annotate("{", "}", descriptor_);
527+
521528
WriteFieldAccessorDocComment(printer, descriptor_, CLEARER,
522529
/* builder */ true);
523530
printer->Print(
@@ -540,10 +547,7 @@ void ImmutableEnumOneofFieldGenerator::GenerateBuilderClearCode(
540547

541548
void ImmutableEnumOneofFieldGenerator::GenerateBuildingCode(
542549
io::Printer* printer) const {
543-
printer->Print(variables_,
544-
"if ($has_oneof_case_message$) {\n"
545-
" result.$oneof_name$_ = $oneof_name$_;\n"
546-
"}\n");
550+
// No-Op: Handled by single statement for the oneof
547551
}
548552

549553
void ImmutableEnumOneofFieldGenerator::GenerateMergingCode(
@@ -632,11 +636,8 @@ void ImmutableEnumOneofFieldGenerator::GenerateHashCode(
632636
RepeatedImmutableEnumFieldGenerator::RepeatedImmutableEnumFieldGenerator(
633637
const FieldDescriptor* descriptor, int messageBitIndex, int builderBitIndex,
634638
Context* context)
635-
: descriptor_(descriptor), name_resolver_(context->GetNameResolver()) {
636-
SetEnumVariables(descriptor, messageBitIndex, builderBitIndex,
637-
context->GetFieldGeneratorInfo(descriptor), name_resolver_,
638-
&variables_, context);
639-
}
639+
: ImmutableEnumFieldGenerator(descriptor, messageBitIndex, builderBitIndex,
640+
context) {}
640641

641642
RepeatedImmutableEnumFieldGenerator::~RepeatedImmutableEnumFieldGenerator() {}
642643

src/google/protobuf/compiler/java/enum_field.h

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ class ImmutableEnumFieldGenerator : public ImmutableFieldGenerator {
6868

6969
// implements ImmutableFieldGenerator
7070
// ---------------------------------------
71+
int GetMessageBitIndex() const override;
72+
int GetBuilderBitIndex() const override;
7173
int GetNumBitsForMessage() const override;
7274
int GetNumBitsForBuilder() const override;
7375
void GenerateInterfaceMembers(io::Printer* printer) const override;
@@ -90,6 +92,8 @@ class ImmutableEnumFieldGenerator : public ImmutableFieldGenerator {
9092

9193
protected:
9294
const FieldDescriptor* descriptor_;
95+
int message_bit_index_;
96+
int builder_bit_index_;
9397
absl::flat_hash_map<absl::string_view, std::string> variables_;
9498
ClassNameResolver* name_resolver_;
9599
};
@@ -117,7 +121,7 @@ class ImmutableEnumOneofFieldGenerator : public ImmutableEnumFieldGenerator {
117121
void GenerateHashCode(io::Printer* printer) const override;
118122
};
119123

120-
class RepeatedImmutableEnumFieldGenerator : public ImmutableFieldGenerator {
124+
class RepeatedImmutableEnumFieldGenerator : public ImmutableEnumFieldGenerator {
121125
public:
122126
explicit RepeatedImmutableEnumFieldGenerator(
123127
const FieldDescriptor* descriptor, int messageBitIndex,
@@ -150,11 +154,6 @@ class RepeatedImmutableEnumFieldGenerator : public ImmutableFieldGenerator {
150154
void GenerateKotlinDslMembers(io::Printer* printer) const override;
151155

152156
std::string GetBoxedType() const override;
153-
154-
private:
155-
const FieldDescriptor* descriptor_;
156-
absl::flat_hash_map<absl::string_view, std::string> variables_;
157-
ClassNameResolver* name_resolver_;
158157
};
159158

160159
} // namespace java

src/google/protobuf/compiler/java/field.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ class ImmutableFieldGenerator {
7171
ImmutableFieldGenerator& operator=(const ImmutableFieldGenerator&) = delete;
7272
virtual ~ImmutableFieldGenerator();
7373

74+
virtual int GetMessageBitIndex() const = 0;
75+
virtual int GetBuilderBitIndex() const = 0;
7476
virtual int GetNumBitsForMessage() const = 0;
7577
virtual int GetNumBitsForBuilder() const = 0;
7678
virtual void GenerateInterfaceMembers(io::Printer* printer) const = 0;

0 commit comments

Comments
 (0)