Skip to content

Commit 70c216b

Browse files
jckingcopybara-github
jcking
authored and
copybara-github
committed
Devirtualize cel::Data
PiperOrigin-RevId: 733802177
1 parent 18b6b38 commit 70c216b

File tree

4 files changed

+63
-65
lines changed

4 files changed

+63
-65
lines changed

‎common/data.h‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#ifndef THIRD_PARTY_CEL_CPP_COMMON_DATA_H_
1616
#define THIRD_PARTY_CEL_CPP_COMMON_DATA_H_
1717

18+
#include <cstddef>
1819
#include <cstdint>
1920

2021
#include "absl/base/nullability.h"
@@ -34,22 +35,25 @@ namespace common_internal {
3435

3536
class ReferenceCount;
3637

37-
void SetDataReferenceCount(
38-
absl::Nonnull<const Data*> data,
39-
absl::Nonnull<const ReferenceCount*> refcount) noexcept;
38+
void SetDataReferenceCount(absl::Nonnull<const Data*> data,
39+
absl::Nonnull<const ReferenceCount*> refcount);
4040

4141
absl::Nullable<const ReferenceCount*> GetDataReferenceCount(
42-
absl::Nonnull<const Data*> data) noexcept;
42+
absl::Nonnull<const Data*> data);
4343

4444
} // namespace common_internal
4545

4646
// `Data` is one of the base classes of objects that can be managed by
4747
// `MemoryManager`, the other is `google::protobuf::MessageLite`.
4848
class Data {
4949
public:
50-
virtual ~Data() = default;
50+
Data(const Data&) = default;
51+
Data(Data&&) = default;
52+
~Data() = default;
53+
Data& operator=(const Data&) = default;
54+
Data& operator=(Data&&) = default;
5155

52-
absl::Nullable<google::protobuf::Arena*> GetArena() const noexcept {
56+
absl::Nullable<google::protobuf::Arena*> GetArena() const {
5357
return (owner_ & kOwnerBits) == kOwnerArenaBit
5458
? reinterpret_cast<google::protobuf::Arena*>(owner_ & kOwnerPointerMask)
5559
: nullptr;
@@ -61,14 +65,11 @@ class Data {
6165
// reference count ahead of time and then update it with the data it has to
6266
// delete, but that is a bit counter intuitive. Doing it this way is also
6367
// similar to how std::enable_shared_from_this works.
64-
Data() noexcept : Data(nullptr) {}
68+
Data() = default;
6569

66-
Data(const Data&) = default;
67-
Data(Data&&) = default;
68-
Data& operator=(const Data&) = default;
69-
Data& operator=(Data&&) = default;
70+
Data(std::nullptr_t) = delete;
7071

71-
explicit Data(absl::Nullable<google::protobuf::Arena*> arena) noexcept
72+
explicit Data(absl::Nullable<google::protobuf::Arena*> arena)
7273
: owner_(reinterpret_cast<uintptr_t>(arena) |
7374
(arena != nullptr ? kOwnerArenaBit : kOwnerNone)) {}
7475

@@ -84,10 +85,9 @@ class Data {
8485

8586
friend void common_internal::SetDataReferenceCount(
8687
absl::Nonnull<const Data*> data,
87-
absl::Nonnull<const common_internal::ReferenceCount*> refcount) noexcept;
88+
absl::Nonnull<const common_internal::ReferenceCount*> refcount);
8889
friend absl::Nullable<const common_internal::ReferenceCount*>
89-
common_internal::GetDataReferenceCount(
90-
absl::Nonnull<const Data*> data) noexcept;
90+
common_internal::GetDataReferenceCount(absl::Nonnull<const Data*> data);
9191
template <typename T>
9292
friend struct Ownable;
9393
template <typename T>
@@ -100,14 +100,14 @@ namespace common_internal {
100100

101101
inline void SetDataReferenceCount(
102102
absl::Nonnull<const Data*> data,
103-
absl::Nonnull<const ReferenceCount*> refcount) noexcept {
103+
absl::Nonnull<const ReferenceCount*> refcount) {
104104
ABSL_DCHECK_EQ(data->owner_, Data::kOwnerNone);
105105
data->owner_ =
106106
reinterpret_cast<uintptr_t>(refcount) | Data::kOwnerReferenceCountBit;
107107
}
108108

109109
inline absl::Nullable<const ReferenceCount*> GetDataReferenceCount(
110-
absl::Nonnull<const Data*> data) noexcept {
110+
absl::Nonnull<const Data*> data) {
111111
return (data->owner_ & Data::kOwnerBits) == Data::kOwnerReferenceCountBit
112112
? reinterpret_cast<const ReferenceCount*>(data->owner_ &
113113
Data::kOwnerPointerMask)

‎common/internal/reference_count.cc‎

Lines changed: 34 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -31,27 +31,27 @@
3131
namespace cel::common_internal {
3232

3333
template class DeletingReferenceCount<google::protobuf::MessageLite>;
34-
template class DeletingReferenceCount<Data>;
3534

3635
namespace {
3736

3837
class ReferenceCountedStdString final : public ReferenceCounted {
3938
public:
39+
static std::pair<absl::Nonnull<const ReferenceCount*>, absl::string_view> New(
40+
std::string&& string) {
41+
const auto* const refcount =
42+
new ReferenceCountedStdString(std::move(string));
43+
const auto* const refcount_string = std::launder(
44+
reinterpret_cast<const std::string*>(&refcount->string_[0]));
45+
return std::pair{
46+
static_cast<absl::Nonnull<const ReferenceCount*>>(refcount),
47+
absl::string_view(*refcount_string)};
48+
}
49+
4050
explicit ReferenceCountedStdString(std::string&& string) {
4151
(::new (static_cast<void*>(&string_[0])) std::string(std::move(string)))
4252
->shrink_to_fit();
4353
}
4454

45-
const char* data() const noexcept {
46-
return std::launder(reinterpret_cast<const std::string*>(&string_[0]))
47-
->data();
48-
}
49-
50-
size_t size() const noexcept {
51-
return std::launder(reinterpret_cast<const std::string*>(&string_[0]))
52-
->size();
53-
}
54-
5555
private:
5656
void Finalize() noexcept override {
5757
std::destroy_at(std::launder(reinterpret_cast<std::string*>(&string_[0])));
@@ -60,6 +60,19 @@ class ReferenceCountedStdString final : public ReferenceCounted {
6060
alignas(std::string) char string_[sizeof(std::string)];
6161
};
6262

63+
class ReferenceCountedString final : public ReferenceCounted {
64+
public:
65+
static std::pair<absl::Nonnull<const ReferenceCount*>, absl::string_view> New(
66+
absl::string_view string) {
67+
const auto* const refcount =
68+
::new (internal::New(Overhead() + string.size()))
69+
ReferenceCountedString(string);
70+
return std::pair{
71+
static_cast<absl::Nonnull<const ReferenceCount*>>(refcount),
72+
absl::string_view(refcount->data_, refcount->size_)};
73+
}
74+
75+
private:
6376
// ReferenceCountedString is non-standard-layout due to having virtual functions
6477
// from a base class. This causes compilers to warn about the use of offsetof(),
6578
// but it still works here, so silence the warning and proceed.
@@ -68,54 +81,40 @@ class ReferenceCountedStdString final : public ReferenceCounted {
6881
#pragma GCC diagnostic ignored "-Winvalid-offsetof"
6982
#endif
7083

71-
class ReferenceCountedString final : public ReferenceCounted {
72-
public:
73-
static const ReferenceCountedString* New(const char* data, size_t size) {
74-
return ::new (internal::New(offsetof(ReferenceCountedString, data_) + size))
75-
ReferenceCountedString(size, data);
76-
}
77-
78-
const char* data() const noexcept { return data_; }
84+
static size_t Overhead() { return offsetof(ReferenceCountedString, data_); }
7985

80-
size_t size() const noexcept { return size_; }
86+
#if defined(__GNUC__) || defined(__clang__)
87+
#pragma GCC diagnostic pop
88+
#endif
8189

82-
private:
83-
ReferenceCountedString(size_t size, const char* data) noexcept : size_(size) {
84-
std::memcpy(data_, data, size);
90+
explicit ReferenceCountedString(absl::string_view string)
91+
: size_(string.size()) {
92+
std::memcpy(data_, string.data(), size_);
8593
}
8694

8795
void Delete() noexcept override {
8896
void* const that = this;
8997
const auto size = size_;
9098
std::destroy_at(this);
91-
internal::SizedDelete(that, offsetof(ReferenceCountedString, data_) + size);
99+
internal::SizedDelete(that, Overhead() + size);
92100
}
93101

94102
const size_t size_;
95103
char data_[];
96104
};
97105

98-
#if defined(__GNUC__) || defined(__clang__)
99-
#pragma GCC diagnostic pop
100-
#endif
101-
102106
} // namespace
103107

104108
std::pair<absl::Nonnull<const ReferenceCount*>, absl::string_view>
105109
MakeReferenceCountedString(absl::string_view value) {
106110
ABSL_DCHECK(!value.empty());
107-
const auto* refcount =
108-
ReferenceCountedString::New(value.data(), value.size());
109-
return std::pair{refcount,
110-
absl::string_view(refcount->data(), refcount->size())};
111+
return ReferenceCountedString::New(value);
111112
}
112113

113114
std::pair<absl::Nonnull<const ReferenceCount*>, absl::string_view>
114115
MakeReferenceCountedString(std::string&& value) {
115116
ABSL_DCHECK(!value.empty());
116-
const auto* refcount = new ReferenceCountedStdString(std::move(value));
117-
return std::pair{refcount,
118-
absl::string_view(refcount->data(), refcount->size())};
117+
return ReferenceCountedStdString::New(std::move(value));
119118
}
120119

121120
} // namespace cel::common_internal

‎common/internal/reference_count.h‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ class EmplacedReferenceCount final : public ReferenceCounted {
174174
static_assert(!std::is_reference_v<T>, "T must not be a reference");
175175
static_assert(!std::is_volatile_v<T>, "T must not be volatile qualified");
176176
static_assert(!std::is_const_v<T>, "T must not be const qualified");
177+
static_assert(!std::is_array_v<T>, "T must not be an array");
177178

178179
template <typename... Args>
179180
explicit EmplacedReferenceCount(T*& value, Args&&... args) noexcept(
@@ -184,7 +185,7 @@ class EmplacedReferenceCount final : public ReferenceCounted {
184185

185186
private:
186187
void Finalize() noexcept override {
187-
std::launder(reinterpret_cast<T*>(&value_[0]))->~T();
188+
std::destroy_at(std::launder(reinterpret_cast<T*>(&value_[0])));
188189
}
189190

190191
// We store the instance of `T` in a char buffer and use placement new and
@@ -205,15 +206,12 @@ class DeletingReferenceCount final : public ReferenceCounted {
205206
: to_delete_(to_delete) {}
206207

207208
private:
208-
void Finalize() noexcept override {
209-
delete std::exchange(to_delete_, nullptr);
210-
}
209+
void Finalize() noexcept override { delete to_delete_; }
211210

212-
const T* to_delete_;
211+
absl::Nonnull<const T*> const to_delete_;
213212
};
214213

215214
extern template class DeletingReferenceCount<google::protobuf::MessageLite>;
216-
extern template class DeletingReferenceCount<Data>;
217215

218216
template <typename T>
219217
absl::Nonnull<const ReferenceCount*> MakeDeletingReferenceCount(
@@ -223,12 +221,12 @@ absl::Nonnull<const ReferenceCount*> MakeDeletingReferenceCount(
223221
}
224222
if constexpr (std::is_base_of_v<google::protobuf::MessageLite, T>) {
225223
return new DeletingReferenceCount<google::protobuf::MessageLite>(to_delete);
226-
} else if constexpr (std::is_base_of_v<Data, T>) {
227-
auto* refcount = new DeletingReferenceCount<Data>(to_delete);
228-
common_internal::SetDataReferenceCount(to_delete, refcount);
229-
return refcount;
230224
} else {
231-
return new DeletingReferenceCount<T>(to_delete);
225+
auto* refcount = new DeletingReferenceCount<T>(to_delete);
226+
if constexpr (std::is_base_of_v<Data, T>) {
227+
common_internal::SetDataReferenceCount(to_delete, refcount);
228+
}
229+
return refcount;
232230
}
233231
}
234232

‎common/internal/reference_count_test.cc‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,9 @@ struct OtherObject final {
9999
TEST(DeletingReferenceCount, Data) {
100100
auto* data = new DataObject();
101101
const auto* refcount = MakeDeletingReferenceCount(data);
102-
EXPECT_THAT(refcount, WhenDynamicCastTo<const DeletingReferenceCount<Data>*>(
103-
NotNull()));
102+
EXPECT_THAT(
103+
refcount,
104+
WhenDynamicCastTo<const DeletingReferenceCount<DataObject>*>(NotNull()));
104105
EXPECT_EQ(common_internal::GetDataReferenceCount(data), refcount);
105106
StrongUnref(refcount);
106107
}

0 commit comments

Comments
 (0)