Skip to content

Commit e902180

Browse files
ClaytonKnittelcopybara-github
ClaytonKnittel
authored and
copybara-github
committed
Add types RepeatedPtrFieldWithArena and RepeatedPtrFieldWithArenaBase, and update the proto runtime to use them when the feature is enabled.
Note: this is an incomplete change, and will not compile with `PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS_REPEATED_PTR_FIELD` enabled. It is being split to minimize code review burden. This change is a no-op and is protected by the `PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS_REPEATED_PTR_FIELD` feature flag. PiperOrigin-RevId: 807916588
1 parent a739f21 commit e902180

22 files changed

+1007
-170
lines changed

‎src/google/protobuf/BUILD.bazel‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -659,6 +659,7 @@ cc_library(
659659
"has_bits.h",
660660
"implicit_weak_message.h",
661661
"inlined_string_field.h",
662+
"internal_metadata_locator.h",
662663
"map.h",
663664
"map_field_lite.h",
664665
"map_type_handler.h",
@@ -887,6 +888,25 @@ cc_test(
887888
],
888889
)
889890

891+
cc_test(
892+
name = "internal_metadata_locator_test",
893+
size = "small",
894+
srcs = [
895+
"internal_metadata_locator_test.cc",
896+
],
897+
deps = [
898+
":arena",
899+
":port",
900+
":protobuf",
901+
":protobuf_lite",
902+
":test_util2",
903+
":unittest_cc_proto",
904+
"//src/google/protobuf/io",
905+
"@googletest//:gtest",
906+
"@googletest//:gtest_main",
907+
],
908+
)
909+
890910
# This provides just the header files for use in projects that need to build
891911
# shared libraries for dynamic loading. This target is available until Bazel
892912
# adds native support for such use cases.

‎src/google/protobuf/arena.h‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -446,10 +446,19 @@ class PROTOBUF_EXPORT PROTOBUF_ALIGNAS(8)
446446
Arena* PROTOBUF_NULLABLE arena,
447447
Args&&... args) {
448448
if constexpr (internal::IsRepeatedPtrFieldType<T>::value) {
449-
using ArenaRepT = typename internal::RepeatedPtrFieldArenaRep<T>::Type;
450-
auto* arena_repr =
451-
new (ptr) ArenaRepT(arena, static_cast<Args&&>(args)...);
452-
return arena_repr;
449+
if (ABSL_PREDICT_FALSE(arena == nullptr)) {
450+
return new (ptr) T(static_cast<Args&&>(args)...);
451+
} else {
452+
using ArenaRepT =
453+
typename internal::RepeatedPtrFieldArenaRep<T>::Type;
454+
auto* arena_repr =
455+
new (ptr) ArenaRepT(arena, static_cast<Args&&>(args)...);
456+
#ifdef PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS_REPEATED_PTR_FIELD
457+
return &arena_repr->field();
458+
#else
459+
return arena_repr;
460+
#endif
461+
}
453462
} else {
454463
return new (ptr) T(arena, static_cast<Args&&>(args)...);
455464
}
@@ -559,7 +568,11 @@ class PROTOBUF_EXPORT PROTOBUF_ALIGNAS(8)
559568
using ArenaRepT = typename internal::RepeatedPtrFieldArenaRep<T>::Type;
560569
auto* arena_repr =
561570
arena->DoCreateMessage<ArenaRepT>(static_cast<Args&&>(args)...);
571+
#ifdef PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS_REPEATED_PTR_FIELD
572+
return &arena_repr->field();
573+
#else
562574
return arena_repr;
575+
#endif
563576
}
564577
}
565578

‎src/google/protobuf/arena_unittest.cc‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -493,7 +493,12 @@ TEST(ArenaTest, RepeatedPtrFieldMoveCtorOnArena) {
493493
TestUtil::ExpectAllFieldsSet(moved->Get(0));
494494

495495
// The only extra allocation with moves is sizeof(RepeatedPtrField).
496+
#ifdef PROTOBUF_INTERNAL_REMOVE_ARENA_PTRS_REPEATED_PTR_FIELD
497+
EXPECT_EQ(usage_by_move,
498+
sizeof(internal::RepeatedPtrFieldWithArena<TestAllTypes>));
499+
#else
496500
EXPECT_EQ(usage_by_move, sizeof(internal::RepeatedPtrFieldBase));
501+
#endif
497502
EXPECT_LT(usage_by_move + sizeof(TestAllTypes), usage_original);
498503

499504
// Status after move is unspecified and must not be assumed. It's merely

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

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -740,6 +740,8 @@ class RepeatedMessage : public FieldGeneratorBase {
740740
void GenerateIsInitialized(io::Printer* p) const override;
741741
bool NeedsIsInitialized() const override;
742742

743+
bool RequiresArena(GeneratorFunction function) const override;
744+
743745
private:
744746
const Options* opts_;
745747
bool has_required_;
@@ -831,7 +833,8 @@ void RepeatedMessage::GenerateInlineAccessorDefinitions(io::Printer* p) const {
831833
ABSL_ATTRIBUTE_LIFETIME_BOUND {
832834
$WeakDescriptorSelfPin$;
833835
$TsanDetectConcurrentMutation$;
834-
$Submsg$* _add = _internal_mutable_$name_internal$()->Add();
836+
$Submsg$* _add = _internal_mutable_$name_internal$()->AddWithArena(
837+
$pb$::MessageLite::internal_visibility(), GetArena());
835838
$set_hasbit$;
836839
$annotate_add_mutable$;
837840
// @@protoc_insertion_point(field_add:$pkg.Msg.field$)
@@ -908,7 +911,8 @@ void RepeatedMessage::GenerateMergingCode(io::Printer* p) const {
908911
// `if (!from.empty()) { body(); }` for both split and non-split cases.
909912
auto body = [&] {
910913
p->Emit(R"cc(
911-
_this->_internal_mutable$_weak$_$name$()->MergeFrom(
914+
_this->_internal_mutable$_weak$_$name$()->MergeFromWithArena(
915+
$pb$::MessageLite::internal_visibility(), arena,
912916
from._internal$_weak$_$name$());
913917
)cc");
914918
};
@@ -940,7 +944,9 @@ void RepeatedMessage::GenerateCopyConstructorCode(io::Printer* p) const {
940944
if (should_split()) {
941945
p->Emit(R"cc(
942946
if (!from._internal$_weak$_$name$().empty()) {
943-
_internal_mutable$_weak$_$name$()->MergeFrom(from._internal$_weak$_$name$());
947+
_internal_mutable$_weak$_$name$()->MergeFromWithArena(
948+
$pb$::MessageLite::internal_visibility(), arena,
949+
from._internal$_weak$_$name$());
944950
}
945951
)cc");
946952
}
@@ -1047,6 +1053,15 @@ void RepeatedMessage::GenerateIsInitialized(io::Printer* p) const {
10471053
}
10481054

10491055
bool RepeatedMessage::NeedsIsInitialized() const { return has_required_; }
1056+
1057+
bool RepeatedMessage::RequiresArena(GeneratorFunction func) const {
1058+
switch (func) {
1059+
case GeneratorFunction::kMergeFrom:
1060+
return true;
1061+
}
1062+
return false;
1063+
}
1064+
10501065
} // namespace
10511066

10521067
std::unique_ptr<FieldGeneratorBase> MakeSinguarMessageGenerator(

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

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -753,12 +753,22 @@ class RepeatedString : public FieldGeneratorBase {
753753
}
754754
}
755755

756+
bool RequiresArena(GeneratorFunction function) const override {
757+
switch (function) {
758+
case GeneratorFunction::kMergeFrom:
759+
return true;
760+
}
761+
return false;
762+
}
763+
756764
void GenerateMergingCode(io::Printer* p) const override {
757765
// TODO: experiment with simplifying this to be
758766
// `if (!from.empty()) { body(); }` for both split and non-split cases.
759767
auto body = [&] {
760768
p->Emit(R"cc(
761-
_this->_internal_mutable_$name$()->MergeFrom(from._internal_$name$());
769+
_this->_internal_mutable_$name$()->MergeFromWithArena(
770+
$pb$::MessageLite::internal_visibility(), arena,
771+
from._internal_$name$());
762772
)cc");
763773
};
764774
if (!should_split()) {
@@ -793,7 +803,9 @@ class RepeatedString : public FieldGeneratorBase {
793803
if (should_split()) {
794804
p->Emit(R"cc(
795805
if (!from._internal_$name$().empty()) {
796-
_internal_mutable_$name$()->MergeFrom(from._internal_$name$());
806+
_internal_mutable_$name$()->MergeFromWithArena(
807+
$pb$::MessageLite::internal_visibility(), arena,
808+
from._internal_$name$());
797809
}
798810
)cc");
799811
}
@@ -869,7 +881,8 @@ void RepeatedString::GenerateInlineAccessorDefinitions(io::Printer* p) const {
869881
ABSL_ATTRIBUTE_LIFETIME_BOUND {
870882
$WeakDescriptorSelfPin$;
871883
$TsanDetectConcurrentMutation$;
872-
::std::string* _s = _internal_mutable_$name_internal$()->Add();
884+
::std::string* _s = _internal_mutable_$name_internal$()->AddWithArena(
885+
$pb$::MessageLite::internal_visibility(), GetArena());
873886
$set_hasbit$;
874887
$annotate_add_mutable$;
875888
// @@protoc_insertion_point(field_add_mutable:$pkg.Msg.field$)
@@ -907,9 +920,10 @@ void RepeatedString::GenerateInlineAccessorDefinitions(io::Printer* p) const {
907920
inline void $Msg$::add_$name$(Arg_&& value, Args_... args) {
908921
$WeakDescriptorSelfPin$;
909922
$TsanDetectConcurrentMutation$;
910-
$pbi$::AddToRepeatedPtrField(*_internal_mutable_$name_internal$(),
911-
::std::forward<Arg_>(value),
912-
args... $bytes_tag$);
923+
$pbi$::AddToRepeatedPtrField(
924+
$pb$::MessageLite::internal_visibility(), GetArena(),
925+
*_internal_mutable_$name_internal$(), ::std::forward<Arg_>(value),
926+
args... $bytes_tag$);
913927
$set_hasbit$;
914928
$annotate_add$;
915929
// @@protoc_insertion_point(field_add:$pkg.Msg.field$)

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

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -630,12 +630,22 @@ class RepeatedStringView : public FieldGeneratorBase {
630630
}
631631
}
632632

633+
bool RequiresArena(GeneratorFunction function) const override {
634+
switch (function) {
635+
case GeneratorFunction::kMergeFrom:
636+
return true;
637+
}
638+
return false;
639+
}
640+
633641
void GenerateMergingCode(io::Printer* p) const override {
634642
// TODO: experiment with simplifying this to be
635643
// `if (!from.empty()) { body(); }` for both split and non-split cases.
636644
auto body = [&] {
637645
p->Emit(R"cc(
638-
_this->_internal_mutable_$name$()->MergeFrom(from._internal_$name$());
646+
_this->_internal_mutable_$name$()->MergeFromWithArena(
647+
$pb$::MessageLite::internal_visibility(), arena,
648+
from._internal_$name$());
639649
)cc");
640650
};
641651
if (!should_split()) {
@@ -670,7 +680,9 @@ class RepeatedStringView : public FieldGeneratorBase {
670680
if (should_split()) {
671681
p->Emit(R"cc(
672682
if (!from._internal_$name$().empty()) {
673-
_internal_mutable_$name$()->MergeFrom(from._internal_$name$());
683+
_internal_mutable_$name$()->MergeFromWithArena(
684+
$pb$::MessageLite::internal_visibility(), arena,
685+
from._internal_$name$());
674686
}
675687
)cc");
676688
}
@@ -757,7 +769,9 @@ void RepeatedStringView::GenerateInlineAccessorDefinitions(
757769
inline void $Msg$::add_$name$(Arg_&& value) {
758770
$WeakDescriptorSelfPin$;
759771
$TsanDetectConcurrentMutation$;
760-
$pbi$::AddToRepeatedPtrField(*_internal_mutable_$name_internal$(),
772+
$pbi$::AddToRepeatedPtrField($pb$::MessageLite::internal_visibility(),
773+
GetArena(),
774+
*_internal_mutable_$name_internal$(),
761775
::std::forward<Arg_>(value) $bytes_tag$);
762776
$set_hasbit$;
763777
$annotate_add$;

‎src/google/protobuf/compiler/plugin.pb.cc‎

Lines changed: 10 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/google/protobuf/compiler/plugin.pb.h‎

Lines changed: 12 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)