Skip to content

Commit 143e3ea

Browse files
ClaytonKnittelcopybara-github
ClaytonKnittel
authored and
copybara-github
committed
Use arena offsets instead of holding an arena pointer in RepeatedField.
This does not reduce the size of `RepeatedField`, but does greatly simplify the SOO logic, and makes `size()` branchless. Additionally, `RepeatedField`s now need `memcpy` in `MessageCreator::PlacementNew`, but don't require arena seeding. All behavior changes introduced by this cl are flag-gated with `PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS_REPEATED_FIELD`, which is currently off. PiperOrigin-RevId: 819581961
1 parent 5f724e0 commit 143e3ea

File tree

10 files changed

+627
-136
lines changed

10 files changed

+627
-136
lines changed

‎src/google/protobuf/compiler/cpp/field.cc‎

Lines changed: 20 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -40,19 +40,9 @@ namespace protobuf {
4040
namespace compiler {
4141
namespace cpp {
4242

43-
namespace {
4443
using ::google::protobuf::internal::WireFormat;
4544
using Sub = ::google::protobuf::io::Printer::Sub;
4645

47-
void InternalMetadataOffsetFormatString(io::Printer* p) {
48-
p->Emit(R"cc(
49-
::_pbi::InternalMetadataOffset::Build<
50-
$classtype$, PROTOBUF_FIELD_OFFSET($classtype$, _impl_.$name$_)>()
51-
)cc");
52-
}
53-
54-
} // namespace
55-
5646
std::vector<Sub> FieldVars(const FieldDescriptor* field, const Options& opts) {
5747
bool split = ShouldSplit(field, opts);
5848
std::vector<Sub> vars = {
@@ -177,19 +167,15 @@ void FieldGeneratorBase::GenerateMemberConstexprConstructor(
177167
#endif
178168
)cc");
179169
} else if (field_->is_repeated()) {
180-
if (IsRepeatedPtrField(field_)) {
181-
p->Emit({{"internal_metadata_offset",
182-
[&] { InternalMetadataOffsetFormatString(p); }}},
183-
R"cc(
170+
p->Emit({{"internal_metadata_offset",
171+
[&] { InternalMetadataOffsetFormatString(p); }}},
172+
R"cc(
184173
#ifdef PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS_REPEATED_PTR_FIELD
185-
$name$_{visibility, $internal_metadata_offset$}
174+
$name$_{visibility, $internal_metadata_offset$}
186175
#else // !PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS_REPEATED_PTR_FIELD
187-
$name$_ {}
176+
$name$_ {}
188177
#endif
189-
)cc");
190-
} else {
191-
p->Emit("$name$_{}");
192-
}
178+
)cc");
193179
} else {
194180
p->Emit({{"default", DefaultValue(options_, field_)}},
195181
"$name$_{$default$}");
@@ -211,7 +197,7 @@ void FieldGeneratorBase::GenerateMemberConstructor(io::Printer* p) const {
211197
} else if (field_->is_repeated()) {
212198
if (ShouldSplit(field_, options_)) {
213199
p->Emit("$name$_{}"); // RawPtr<Repeated>
214-
} else if (IsRepeatedPtrField(field_)) {
200+
} else {
215201
p->Emit({{"internal_metadata_offset",
216202
[p] { InternalMetadataOffsetFormatString(p); }}},
217203
R"cc(
@@ -221,8 +207,6 @@ void FieldGeneratorBase::GenerateMemberConstructor(io::Printer* p) const {
221207
$name$_ { visibility, arena }
222208
#endif
223209
)cc");
224-
} else {
225-
p->Emit("$name$_{visibility, arena}");
226210
}
227211
} else {
228212
p->Emit({{"default", DefaultValue(options_, field_)}},
@@ -243,19 +227,15 @@ void FieldGeneratorBase::GenerateMemberCopyConstructor(io::Printer* p) const {
243227
#endif
244228
)cc");
245229
} else if (field_->is_repeated()) {
246-
if (IsRepeatedPtrField(field_)) {
247-
p->Emit({{"internal_metadata_offset",
248-
[p] { InternalMetadataOffsetFormatString(p); }}},
249-
R"cc(
230+
p->Emit({{"internal_metadata_offset",
231+
[p] { InternalMetadataOffsetFormatString(p); }}},
232+
R"cc(
250233
#ifdef PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS_REPEATED_PTR_FIELD
251-
$name$_{visibility, ($internal_metadata_offset$), from.$name$_}
234+
$name$_{visibility, ($internal_metadata_offset$), from.$name$_}
252235
#else
253-
$name$_ { visibility, arena, from.$name$_ }
236+
$name$_ { visibility, arena, from.$name$_ }
254237
#endif
255-
)cc");
256-
} else {
257-
p->Emit("$name$_{visibility, arena, from.$name$_}");
258-
}
238+
)cc");
259239
} else {
260240
p->Emit("$name$_{from.$name$_}");
261241
}
@@ -309,6 +289,13 @@ pb::CppFeatures::StringType FieldGeneratorBase::GetDeclaredStringType() const {
309289
.string_type();
310290
}
311291

292+
void FieldGeneratorBase::InternalMetadataOffsetFormatString(io::Printer* p) {
293+
p->Emit(R"cc(
294+
::_pbi::InternalMetadataOffset::Build<
295+
$classtype$, PROTOBUF_FIELD_OFFSET($classtype$, _impl_.$name$_)>()
296+
)cc");
297+
}
298+
312299
namespace {
313300
std::unique_ptr<FieldGeneratorBase> MakeGenerator(const FieldDescriptor* field,
314301
const Options& options,

‎src/google/protobuf/compiler/cpp/field.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,8 @@ class FieldGeneratorBase {
203203

204204
pb::CppFeatures::StringType GetDeclaredStringType() const;
205205

206+
static void InternalMetadataOffsetFormatString(io::Printer* p);
207+
206208

207209
private:
208210
bool should_split_ = false;

‎src/google/protobuf/compiler/cpp/field_generators/enum_field.cc‎

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -301,9 +301,15 @@ class RepeatedEnum : public FieldGeneratorBase {
301301
}
302302

303303
void GenerateAggregateInitializer(io::Printer* p) const override {
304-
p->Emit(R"cc(
305-
decltype($field_$){arena},
306-
)cc");
304+
p->Emit({{"internal_metadata_offset",
305+
[p] { InternalMetadataOffsetFormatString(p); }}},
306+
R"cc(
307+
#ifdef PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS_REPEATED_FIELD
308+
decltype($field_$){$internal_metadata_offset$},
309+
#else
310+
decltype($field_$){arena},
311+
#endif
312+
)cc");
307313
if (has_cached_size_) {
308314
// std::atomic has no copy constructor, which prevents explicit aggregate
309315
// initialization pre-C++17.
@@ -326,21 +332,44 @@ class RepeatedEnum : public FieldGeneratorBase {
326332
}
327333

328334
void GenerateMemberConstexprConstructor(io::Printer* p) const override {
329-
p->Emit("$name$_{}");
335+
p->Emit({{"internal_metadata_offset",
336+
[p] { InternalMetadataOffsetFormatString(p); }}},
337+
R"cc(
338+
#ifdef PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS_REPEATED_FIELD
339+
$name$_{visibility, $internal_metadata_offset$}
340+
#else
341+
$name$_ {}
342+
#endif
343+
)cc");
330344
if (has_cached_size_) {
331345
p->Emit(",\n_$name$_cached_byte_size_{0}");
332346
}
333347
}
334348

335349
void GenerateMemberConstructor(io::Printer* p) const override {
336-
p->Emit("$name$_{visibility, arena}");
350+
p->Emit({{"internal_metadata_offset",
351+
[p] { InternalMetadataOffsetFormatString(p); }}},
352+
R"cc(
353+
#ifdef PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS_REPEATED_FIELD
354+
$name$_{visibility, $internal_metadata_offset$}
355+
#else
356+
$name$_ { visibility, arena }
357+
#endif
358+
)cc");
337359
if (has_cached_size_) {
338360
p->Emit(",\n_$name$_cached_byte_size_{0}");
339361
}
340362
}
341363

342364
void GenerateMemberCopyConstructor(io::Printer* p) const override {
343-
p->Emit("$name$_{visibility, arena, from.$name$_}");
365+
p->Emit({{"internal_metadata_offset",
366+
[p] { InternalMetadataOffsetFormatString(p); }}},
367+
R"cc(
368+
#ifdef PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS_REPEATED_FIELD
369+
$name$_{visibility, ($internal_metadata_offset$), from.$name$_}
370+
#else
371+
$name$_ { visibility, arena, from.$name$_ }
372+
#endif)cc");
344373
if (has_cached_size_) {
345374
p->Emit(",\n_$name$_cached_byte_size_{0}");
346375
}

‎src/google/protobuf/compiler/cpp/field_generators/primitive_field.cc‎

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -351,9 +351,15 @@ class RepeatedPrimitive final : public FieldGeneratorBase {
351351

352352
void GenerateAggregateInitializer(io::Printer* p) const override {
353353
ABSL_CHECK(!should_split());
354-
p->Emit(R"cc(
355-
decltype($field_$){arena},
356-
)cc");
354+
p->Emit({{"internal_metadata_offset",
355+
[p] { InternalMetadataOffsetFormatString(p); }}},
356+
R"cc(
357+
#ifdef PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS_REPEATED_FIELD
358+
decltype($field_$){$internal_metadata_offset$},
359+
#else
360+
decltype($field_$){arena},
361+
#endif
362+
)cc");
357363
GenerateCacheSizeInitializer(p);
358364
}
359365

@@ -366,21 +372,44 @@ class RepeatedPrimitive final : public FieldGeneratorBase {
366372
}
367373

368374
void GenerateMemberConstexprConstructor(io::Printer* p) const override {
369-
p->Emit("$name$_{}");
375+
p->Emit({{"internal_metadata_offset",
376+
[p] { InternalMetadataOffsetFormatString(p); }}},
377+
R"cc(
378+
#ifdef PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS_REPEATED_FIELD
379+
$name$_{visibility, $internal_metadata_offset$}
380+
#else
381+
$name$_ {}
382+
#endif
383+
)cc");
370384
if (HasCachedSize()) {
371385
p->Emit(",\n_$name$_cached_byte_size_{0}");
372386
}
373387
}
374388

375389
void GenerateMemberConstructor(io::Printer* p) const override {
376-
p->Emit("$name$_{visibility, arena}");
390+
p->Emit({{"internal_metadata_offset",
391+
[p] { InternalMetadataOffsetFormatString(p); }}},
392+
R"cc(
393+
#ifdef PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS_REPEATED_FIELD
394+
$name$_{visibility, $internal_metadata_offset$}
395+
#else
396+
$name$_ { visibility, arena }
397+
#endif
398+
)cc");
377399
if (HasCachedSize()) {
378400
p->Emit(",\n_$name$_cached_byte_size_{0}");
379401
}
380402
}
381403

382404
void GenerateMemberCopyConstructor(io::Printer* p) const override {
383-
p->Emit("$name$_{visibility, arena, from.$name$_}");
405+
p->Emit({{"internal_metadata_offset",
406+
[p] { InternalMetadataOffsetFormatString(p); }}},
407+
R"cc(
408+
#ifdef PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS_REPEATED_FIELD
409+
$name$_{visibility, ($internal_metadata_offset$), from.$name$_}
410+
#else
411+
$name$_ { visibility, arena, from.$name$_ }
412+
#endif)cc");
384413
if (HasCachedSize()) {
385414
p->Emit(",\n_$name$_cached_byte_size_{0}");
386415
}

‎src/google/protobuf/compiler/cpp/message.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ void DebugAssertUniformLikelyPresence(
128128
bool MessageHasFieldUsingArenaOffset(const Descriptor* descriptor) {
129129
return absl::c_any_of(FieldRange(descriptor),
130130
[](const FieldDescriptor* field) {
131-
return IsRepeatedPtrField(field) || field->is_map();
131+
return field->is_repeated() || field->is_map();
132132
}) ||
133133
descriptor->extension_range_count() > 0;
134134
}
@@ -4065,7 +4065,7 @@ MessageGenerator::NewOpRequirements MessageGenerator::GetNewOp(
40654065
print_arena_offset();
40664066
}
40674067
} else if (field->is_repeated()) {
4068-
if (use_arena_offset && IsRepeatedPtrField(field)) {
4068+
if (use_arena_offset) {
40694069
op.needs_memcpy = true;
40704070
} else {
40714071
op.needs_arena_seeding = true;

0 commit comments

Comments
 (0)