Skip to content

Commit 6e1eba6

Browse files
jckingcopybara-github
jcking
authored and
copybara-github
committed
Switch from cel::Shared<T> to cel::Owned<T>
PiperOrigin-RevId: 735791355
1 parent c8887b6 commit 6e1eba6

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+895
-1053
lines changed

‎common/BUILD‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -585,6 +585,7 @@ cc_library(
585585
deps = [
586586
":allocator",
587587
":any",
588+
":arena",
588589
":casting",
589590
":kind",
590591
":memory",
@@ -595,7 +596,6 @@ cc_library(
595596
":value_kind",
596597
"//base:attributes",
597598
"//common/internal:byte_string",
598-
"//common/internal:reference_count",
599599
"//eval/internal:cel_value_equal",
600600
"//eval/public:cel_value",
601601
"//eval/public:message_wrapper",
@@ -609,6 +609,7 @@ cc_library(
609609
"//extensions/protobuf/internal:qualify",
610610
"//internal:casts",
611611
"//internal:json",
612+
"//internal:manual",
612613
"//internal:message_equality",
613614
"//internal:number",
614615
"//internal:protobuf_runtime_version",
@@ -696,7 +697,6 @@ cc_library(
696697
hdrs = ["arena.h"],
697698
deps = [
698699
"@com_google_absl//absl/base:nullability",
699-
"@com_google_absl//absl/meta:type_traits",
700700
"@com_google_protobuf//:protobuf",
701701
],
702702
)
@@ -712,6 +712,7 @@ cc_library(
712712
hdrs = ["allocator.h"],
713713
deps = [
714714
":arena",
715+
":data",
715716
"//internal:new",
716717
"@com_google_absl//absl/base:core_headers",
717718
"@com_google_absl//absl/base:nullability",

‎common/allocator.h‎

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include "absl/log/die_if_null.h"
2828
#include "absl/numeric/bits.h"
2929
#include "common/arena.h"
30+
#include "common/data.h"
3031
#include "internal/new.h"
3132
#include "google/protobuf/arena.h"
3233

@@ -287,9 +288,29 @@ class ArenaAllocator<void> {
287288
// called.
288289
template <typename T, typename... Args>
289290
ABSL_MUST_USE_RESULT T* new_object(Args&&... args) {
290-
auto* object = google::protobuf::Arena::Create<std::remove_const_t<T>>(
291-
arena(), std::forward<Args>(args)...);
292-
if constexpr (IsArenaConstructible<T>::value) {
291+
using U = std::remove_const_t<T>;
292+
U* object;
293+
if constexpr (google::protobuf::Arena::is_arena_constructable<U>::value) {
294+
// Classes derived from `cel::Data` are manually allocated and constructed
295+
// as those class support determining whether the destructor is skippable
296+
// at runtime.
297+
object = google::protobuf::Arena::Create<U>(arena(), std::forward<Args>(args)...);
298+
} else {
299+
if constexpr (ArenaTraits<>::constructible<U>()) {
300+
object = ::new (static_cast<void*>(arena()->AllocateAligned(
301+
sizeof(U), alignof(U)))) U(arena(), std::forward<Args>(args)...);
302+
} else {
303+
object = ::new (static_cast<void*>(arena()->AllocateAligned(
304+
sizeof(U), alignof(U)))) U(std::forward<Args>(args)...);
305+
}
306+
if constexpr (!ArenaTraits<>::always_trivially_destructible<U>()) {
307+
if (!ArenaTraits<>::trivially_destructible(*object)) {
308+
arena()->OwnDestructor(object);
309+
}
310+
}
311+
}
312+
if constexpr (google::protobuf::Arena::is_arena_constructable<U>::value ||
313+
std::is_base_of_v<Data, U>) {
293314
ABSL_DCHECK_EQ(object->GetArena(), arena());
294315
}
295316
return object;
@@ -299,8 +320,10 @@ class ArenaAllocator<void> {
299320
// memory, `p` must have been previously returned by `new_object`.
300321
template <typename T>
301322
void delete_object(T* p) noexcept {
323+
using U = std::remove_const_t<T>;
302324
ABSL_DCHECK(p != nullptr);
303-
if constexpr (IsArenaConstructible<T>::value) {
325+
if constexpr (google::protobuf::Arena::is_arena_constructable<U>::value ||
326+
std::is_base_of_v<Data, U>) {
304327
ABSL_DCHECK_EQ(p->GetArena(), arena());
305328
}
306329
}
@@ -359,13 +382,13 @@ class ArenaAllocator : public ArenaAllocator<void> {
359382

360383
template <typename U, typename... Args>
361384
void construct(U* p, Args&&... args) {
362-
static_assert(!IsArenaConstructible<U>::value);
385+
static_assert(!google::protobuf::Arena::is_arena_constructable<U>::value);
363386
::new (static_cast<void*>(p)) U(std::forward<Args>(args)...);
364387
}
365388

366389
template <typename U>
367390
void destroy(U* p) noexcept {
368-
static_assert(!IsArenaConstructible<U>::value);
391+
static_assert(!google::protobuf::Arena::is_arena_constructable<U>::value);
369392
std::destroy_at(p);
370393
}
371394
};

‎common/arena.h‎

Lines changed: 68 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,38 +16,95 @@
1616
#define THIRD_PARTY_CEL_CPP_COMMON_ARENA_H_
1717

1818
#include <type_traits>
19+
#include <utility>
1920

2021
#include "absl/base/nullability.h"
21-
#include "absl/meta/type_traits.h"
2222
#include "google/protobuf/arena.h"
2323

2424
namespace cel {
2525

26-
template <typename T>
27-
using IsArenaConstructible = google::protobuf::Arena::is_arena_constructable<T>;
26+
template <typename T = void>
27+
struct ArenaTraits;
28+
29+
namespace common_internal {
2830

2931
template <typename T>
30-
using IsArenaDestructorSkippable =
31-
absl::conjunction<IsArenaConstructible<T>,
32-
google::protobuf::Arena::is_destructor_skippable<T>>;
32+
struct AssertArenaType : std::false_type {
33+
static_assert(!std::is_void_v<T>, "T must not be void");
34+
static_assert(!std::is_reference_v<T>, "T must not be a reference");
35+
static_assert(!std::is_volatile_v<T>, "T must not be volatile qualified");
36+
static_assert(!std::is_const_v<T>, "T must not be const qualified");
37+
static_assert(!std::is_array_v<T>, "T must not be an array");
38+
};
3339

34-
namespace common_internal {
40+
template <typename, typename = void>
41+
struct ArenaTraitsConstructible {
42+
using type = std::false_type;
43+
};
3544

3645
template <typename T>
37-
std::enable_if_t<IsArenaConstructible<T>::value, absl::Nullable<google::protobuf::Arena*>>
38-
GetArena(const T* ptr) {
46+
struct ArenaTraitsConstructible<
47+
T, std::void_t<decltype(ArenaTraits<T>::constructible)>> {
48+
using type = typename ArenaTraits<T>::constructible;
49+
};
50+
51+
template <typename T>
52+
std::enable_if_t<google::protobuf::Arena::is_arena_constructable<T>::value,
53+
absl::Nullable<google::protobuf::Arena*>>
54+
GetArena(absl::Nullable<const T*> ptr) {
3955
return ptr != nullptr ? ptr->GetArena() : nullptr;
4056
}
4157

4258
template <typename T>
43-
std::enable_if_t<!IsArenaConstructible<T>::value,
59+
std::enable_if_t<!google::protobuf::Arena::is_arena_constructable<T>::value,
4460
absl::Nullable<google::protobuf::Arena*>>
45-
GetArena([[maybe_unused]] const T* ptr) {
61+
GetArena([[maybe_unused]] absl::Nullable<const T*> ptr) {
4662
return nullptr;
4763
}
4864

65+
template <typename, typename = void>
66+
struct HasArenaTraitsTriviallyDestructible : std::false_type {};
67+
68+
template <typename T>
69+
struct HasArenaTraitsTriviallyDestructible<
70+
T, std::void_t<decltype(ArenaTraits<T>::trivially_destructible(
71+
std::declval<const T&>()))>> : std::true_type {};
72+
4973
} // namespace common_internal
5074

75+
template <>
76+
struct ArenaTraits<void> {
77+
template <typename U>
78+
using constructible = std::disjunction<
79+
typename common_internal::AssertArenaType<U>::type,
80+
typename common_internal::ArenaTraitsConstructible<U>::type>;
81+
82+
template <typename U>
83+
using always_trivially_destructible =
84+
std::disjunction<typename common_internal::AssertArenaType<U>::type,
85+
std::is_trivially_destructible<U>>;
86+
87+
template <typename U>
88+
static bool trivially_destructible(const U& obj) {
89+
static_assert(!std::is_void_v<U>, "T must not be void");
90+
static_assert(!std::is_reference_v<U>, "T must not be a reference");
91+
static_assert(!std::is_volatile_v<U>, "T must not be volatile qualified");
92+
static_assert(!std::is_const_v<U>, "T must not be const qualified");
93+
static_assert(!std::is_array_v<U>, "T must not be an array");
94+
95+
if constexpr (always_trivially_destructible<U>()) {
96+
return true;
97+
} else if constexpr (google::protobuf::Arena::is_destructor_skippable<U>::value) {
98+
return obj.GetArena() != nullptr;
99+
} else if constexpr (common_internal::HasArenaTraitsTriviallyDestructible<
100+
U>::value) {
101+
return ArenaTraits<U>::trivially_destructible(obj);
102+
} else {
103+
return false;
104+
}
105+
}
106+
};
107+
51108
} // namespace cel
52109

53110
#endif // THIRD_PARTY_CEL_CPP_COMMON_ARENA_H_

‎common/internal/BUILD‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ cc_library(
3232
srcs = ["reference_count.cc"],
3333
hdrs = ["reference_count.h"],
3434
deps = [
35-
"//common:arena",
3635
"//common:data",
3736
"//internal:new",
3837
"@com_google_absl//absl/base:core_headers",
@@ -69,6 +68,7 @@ cc_library(
6968
":metadata",
7069
":reference_count",
7170
"//common:allocator",
71+
"//common:arena",
7272
"//common:memory",
7373
"@com_google_absl//absl/base:core_headers",
7474
"@com_google_absl//absl/base:nullability",

‎common/internal/byte_string.cc‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -921,7 +921,7 @@ void ByteString::SetLarge(absl::Cord&& cord) {
921921
::new (static_cast<void*>(&rep_.large.data[0])) absl::Cord(std::move(cord));
922922
}
923923

924-
absl::string_view LegacyByteString(const ByteString& string,
924+
absl::string_view LegacyByteString(const ByteString& string, bool stable,
925925
absl::Nonnull<google::protobuf::Arena*> arena) {
926926
ABSL_DCHECK(arena != nullptr);
927927
if (string.empty()) {
@@ -936,6 +936,9 @@ absl::string_view LegacyByteString(const ByteString& string,
936936
return string.GetMedium();
937937
}
938938
}
939+
if (stable && kind == ByteStringKind::kSmall) {
940+
return string.GetSmall();
941+
}
939942
absl::Nonnull<std::string*> result =
940943
google::protobuf::Arena::Create<std::string>(arena);
941944
switch (kind) {

‎common/internal/byte_string.h‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
#include "absl/strings/string_view.h"
3434
#include "absl/types/optional.h"
3535
#include "common/allocator.h"
36+
#include "common/arena.h"
3637
#include "common/internal/metadata.h"
3738
#include "common/internal/reference_count.h"
3839
#include "common/memory.h"
@@ -45,8 +46,6 @@ class BytesValueOutputStream;
4546

4647
namespace common_internal {
4748

48-
class TrivialValue;
49-
5049
// absl::Cord is trivially relocatable IFF we are not using ASan or MSan. When
5150
// using ASan or MSan absl::Cord will poison/unpoison its inline storage.
5251
#if defined(ABSL_HAVE_ADDRESS_SANITIZER) || defined(ABSL_HAVE_MEMORY_SANITIZER)
@@ -155,7 +154,11 @@ union CEL_COMMON_INTERNAL_BYTE_STRING_TRIVIAL_ABI ByteStringRep final {
155154
LargeByteStringRep large;
156155
};
157156

158-
absl::string_view LegacyByteString(const ByteString& string,
157+
// Returns a `absl::string_view` from `ByteString`, using `arena` to make memory
158+
// allocations if necessary. `stable` indicates whether `cel::Value` is in a
159+
// location where it will not be moved, so that inline string/bytes storage can
160+
// be referenced.
161+
absl::string_view LegacyByteString(const ByteString& string, bool stable,
159162
absl::Nonnull<google::protobuf::Arena*> arena);
160163

161164
// `ByteString` is an vocabulary type capable of representing copy-on-write
@@ -328,11 +331,12 @@ ByteString final {
328331
private:
329332
friend class ByteStringView;
330333
friend struct ByteStringTestFriend;
331-
friend class TrivialValue;
332334
friend class cel::BytesValueInputStream;
333335
friend class cel::BytesValueOutputStream;
334336
friend absl::string_view LegacyByteString(
335-
const ByteString& string, absl::Nonnull<google::protobuf::Arena*> arena);
337+
const ByteString& string, bool stable,
338+
absl::Nonnull<google::protobuf::Arena*> arena);
339+
friend struct cel::ArenaTraits<ByteString>;
336340

337341
static ByteString Borrowed(Borrower borrower,
338342
absl::string_view string
@@ -804,6 +808,23 @@ inline ByteString& ByteString::operator=(ByteStringView other) {
804808

805809
} // namespace common_internal
806810

811+
template <>
812+
struct ArenaTraits<common_internal::ByteString> {
813+
using constructible = std::true_type;
814+
815+
static bool trivially_destructible(
816+
const common_internal::ByteString& byte_string) {
817+
switch (byte_string.GetKind()) {
818+
case common_internal::ByteStringKind::kSmall:
819+
return true;
820+
case common_internal::ByteStringKind::kMedium:
821+
return byte_string.GetMediumReferenceCount() == nullptr;
822+
case common_internal::ByteStringKind::kLarge:
823+
return false;
824+
}
825+
}
826+
};
827+
807828
} // namespace cel
808829

809830
#endif // THIRD_PARTY_CEL_CPP_COMMON_INTERNAL_BYTE_STRING_H_

‎common/internal/byte_string_test.cc‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1142,19 +1142,28 @@ TEST_P(ByteStringTest, CloneLarge) {
11421142
TEST_P(ByteStringTest, LegacyByteStringSmall) {
11431143
google::protobuf::Arena arena;
11441144
ByteString byte_string = ByteString(GetAllocator(), GetSmallStringView());
1145-
EXPECT_EQ(LegacyByteString(byte_string, &arena), GetSmallStringView());
1145+
EXPECT_EQ(LegacyByteString(byte_string, /*stable=*/false, &arena),
1146+
GetSmallStringView());
1147+
EXPECT_EQ(LegacyByteString(byte_string, /*stable=*/true, &arena),
1148+
GetSmallStringView());
11461149
}
11471150

11481151
TEST_P(ByteStringTest, LegacyByteStringMedium) {
11491152
google::protobuf::Arena arena;
11501153
ByteString byte_string = ByteString(GetAllocator(), GetMediumStringView());
1151-
EXPECT_EQ(LegacyByteString(byte_string, &arena), GetMediumStringView());
1154+
EXPECT_EQ(LegacyByteString(byte_string, /*stable=*/false, &arena),
1155+
GetMediumStringView());
1156+
EXPECT_EQ(LegacyByteString(byte_string, /*stable=*/true, &arena),
1157+
GetMediumStringView());
11521158
}
11531159

11541160
TEST_P(ByteStringTest, LegacyByteStringLarge) {
11551161
google::protobuf::Arena arena;
11561162
ByteString byte_string = ByteString(GetAllocator(), GetMediumOrLargeCord());
1157-
EXPECT_EQ(LegacyByteString(byte_string, &arena), GetMediumOrLargeCord());
1163+
EXPECT_EQ(LegacyByteString(byte_string, /*stable=*/false, &arena),
1164+
GetMediumOrLargeCord());
1165+
EXPECT_EQ(LegacyByteString(byte_string, /*stable=*/true, &arena),
1166+
GetMediumOrLargeCord());
11581167
}
11591168

11601169
TEST_P(ByteStringTest, HashValue) {

‎common/internal/reference_count.h‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
#include "absl/base/optimization.h"
3333
#include "absl/log/absl_check.h"
3434
#include "absl/strings/string_view.h"
35-
#include "common/arena.h"
3635
#include "common/data.h"
3736
#include "google/protobuf/arena.h"
3837
#include "google/protobuf/message_lite.h"
@@ -216,7 +215,7 @@ extern template class DeletingReferenceCount<google::protobuf::MessageLite>;
216215
template <typename T>
217216
absl::Nonnull<const ReferenceCount*> MakeDeletingReferenceCount(
218217
absl::Nonnull<const T*> to_delete) {
219-
if constexpr (IsArenaConstructible<T>::value) {
218+
if constexpr (google::protobuf::Arena::is_arena_constructable<T>::value) {
220219
ABSL_DCHECK_EQ(to_delete->GetArena(), nullptr);
221220
}
222221
if constexpr (std::is_base_of_v<google::protobuf::MessageLite, T>) {
@@ -237,7 +236,7 @@ MakeEmplacedReferenceCount(Args&&... args) {
237236
U* pointer;
238237
auto* const refcount =
239238
new EmplacedReferenceCount<U>(pointer, std::forward<Args>(args)...);
240-
if constexpr (IsArenaConstructible<U>::value) {
239+
if constexpr (google::protobuf::Arena::is_arena_constructable<U>::value) {
241240
ABSL_DCHECK_EQ(pointer->GetArena(), nullptr);
242241
}
243242
if constexpr (std::is_base_of_v<Data, T>) {

0 commit comments

Comments
 (0)