Skip to content

Commit 02115d8

Browse files
jckingcopybara-github
jcking
authored and
copybara-github
committed
Optimize BytesValueInputStream by avoiding std::visit
PiperOrigin-RevId: 991073619
1 parent 9602b67 commit 02115d8

3 files changed

Lines changed: 77 additions & 166 deletions

File tree

‎common/values/bytes_value_input_stream.h‎

Lines changed: 25 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,12 @@
2121
#include <cstddef>
2222
#include <cstdint>
2323
#include <limits>
24-
#include <new>
24+
#include <utility>
25+
#include <variant>
2526

26-
#include "absl/base/attributes.h"
27-
#include "absl/base/nullability.h"
2827
#include "absl/log/absl_check.h"
2928
#include "absl/strings/cord.h"
3029
#include "absl/strings/string_view.h"
31-
#include "absl/types/variant.h"
32-
#include "absl/utility/utility.h"
3330
#include "common/internal/byte_string.h"
3431
#include "common/values/bytes_value.h"
3532
#include "google/protobuf/io/zero_copy_stream.h"
@@ -39,108 +36,62 @@ namespace cel {
3936

4037
class BytesValueInputStream final : public google::protobuf::io::ZeroCopyInputStream {
4138
public:
42-
explicit BytesValueInputStream(
43-
const BytesValue* absl_nonnull value ABSL_ATTRIBUTE_LIFETIME_BOUND) {
44-
Construct(value);
45-
}
46-
47-
~BytesValueInputStream() override { AsVariant().~variant(); }
39+
explicit BytesValueInputStream(const BytesValue& value) { Construct(value); }
4840

4941
bool Next(const void** data, int* size) override {
50-
return absl::visit(
51-
[&data, &size](auto& alternative) -> bool {
52-
return alternative.stream.Next(data, size);
53-
},
54-
AsVariant());
42+
return stream_->Next(data, size);
5543
}
5644

57-
void BackUp(int count) override {
58-
absl::visit(
59-
[&count](auto& alternative) -> void {
60-
alternative.stream.BackUp(count);
61-
},
62-
AsVariant());
63-
}
45+
void BackUp(int count) override { stream_->BackUp(count); }
6446

65-
bool Skip(int count) override {
66-
return absl::visit(
67-
[&count](auto& alternative) -> bool {
68-
return alternative.stream.Skip(count);
69-
},
70-
AsVariant());
71-
}
47+
bool Skip(int count) override { return stream_->Skip(count); }
7248

73-
int64_t ByteCount() const override {
74-
return absl::visit(
75-
[](const auto& alternative) -> int64_t {
76-
return alternative.stream.ByteCount();
77-
},
78-
AsVariant());
79-
}
49+
int64_t ByteCount() const override { return stream_->ByteCount(); }
8050

8151
bool ReadCord(absl::Cord* cord, int count) override {
82-
return absl::visit(
83-
[&cord, &count](auto& alternative) -> bool {
84-
return alternative.stream.ReadCord(cord, count);
85-
},
86-
AsVariant());
52+
return stream_->ReadCord(cord, count);
8753
}
8854

8955
private:
90-
struct ArrayStream {
91-
ArrayStream(const char* data, int size) : stream(data, size) {}
92-
93-
google::protobuf::io::ArrayInputStream stream;
94-
};
56+
using ArrayStream = google::protobuf::io::ArrayInputStream;
9557
struct CordStream {
96-
explicit CordStream(const absl::Cord& cord)
97-
: cord(cord), stream(&this->cord) {}
58+
explicit CordStream(absl::Cord cord)
59+
: cord(std::move(cord)), stream(&this->cord) {}
9860

9961
absl::Cord cord;
10062
google::protobuf::io::CordInputStream stream;
10163
};
102-
using Variant = absl::variant<ArrayStream, CordStream>;
64+
using Variant = std::variant<std::monostate, ArrayStream, CordStream>;
10365

104-
void Construct(const BytesValue* absl_nonnull value) {
105-
ABSL_DCHECK(value != nullptr);
106-
107-
switch (value->value_.GetKind()) {
66+
void Construct(const BytesValue& value) {
67+
switch (value.value_.GetKind()) {
10868
case common_internal::ByteStringKind::kSmall:
109-
Construct(value->value_.GetSmall());
69+
small_ = value.value_.rep_.small;
70+
Construct(absl::string_view(small_.data, small_.size));
11071
break;
11172
case common_internal::ByteStringKind::kMedium:
112-
Construct(value->value_.GetMedium());
73+
Construct(value.value_.GetMedium());
11374
break;
11475
case common_internal::ByteStringKind::kLarge:
115-
Construct(value->value_.GetLarge());
76+
Construct(value.value_.GetLarge());
11677
break;
11778
}
11879
}
11980

12081
void Construct(absl::string_view value) {
12182
ABSL_DCHECK_LE(value.size(),
12283
static_cast<size_t>(std::numeric_limits<int>::max()));
123-
::new (static_cast<void*>(&impl_[0]))
124-
Variant(absl::in_place_type<ArrayStream>, value.data(),
125-
static_cast<int>(value.size()));
126-
}
127-
128-
void Construct(const absl::Cord& value) {
129-
::new (static_cast<void*>(&impl_[0]))
130-
Variant(absl::in_place_type<CordStream>, value);
131-
}
132-
133-
void Destruct() { AsVariant().~variant(); }
134-
135-
Variant& AsVariant() ABSL_ATTRIBUTE_LIFETIME_BOUND {
136-
return *std::launder(reinterpret_cast<Variant*>(&impl_[0]));
84+
stream_ = &variant_.emplace<ArrayStream>(value.data(),
85+
static_cast<int>(value.size()));
13786
}
13887

139-
const Variant& AsVariant() const ABSL_ATTRIBUTE_LIFETIME_BOUND {
140-
return *std::launder(reinterpret_cast<const Variant*>(&impl_[0]));
88+
void Construct(absl::Cord value) {
89+
stream_ = &variant_.emplace<CordStream>(std::move(value)).stream;
14190
}
14291

143-
alignas(Variant) char impl_[sizeof(Variant)];
92+
google::protobuf::io::ZeroCopyInputStream* stream_;
93+
common_internal::SmallByteStringRep small_;
94+
Variant variant_;
14495
};
14596

14697
} // namespace cel

‎common/values/bytes_value_output_stream.h‎

Lines changed: 31 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -19,18 +19,16 @@
1919
#define THIRD_PARTY_CEL_CPP_COMMON_VALUES_BYTES_VALUE_OUTPUT_STREAM_H_
2020

2121
#include <cstdint>
22-
#include <new>
2322
#include <string>
2423
#include <utility>
24+
#include <variant>
2525

26-
#include "absl/base/attributes.h"
2726
#include "absl/base/nullability.h"
27+
#include "absl/base/optimization.h"
2828
#include "absl/functional/overload.h"
2929
#include "absl/log/absl_check.h"
3030
#include "absl/strings/cord.h"
3131
#include "absl/strings/string_view.h"
32-
#include "absl/types/variant.h"
33-
#include "absl/utility/utility.h"
3432
#include "common/internal/byte_string.h"
3533
#include "common/values/bytes_value.h"
3634
#include "google/protobuf/arena.h"
@@ -41,99 +39,55 @@ namespace cel {
4139

4240
class BytesValueOutputStream final : public google::protobuf::io::ZeroCopyOutputStream {
4341
public:
42+
BytesValueOutputStream() { Construct(); }
43+
4444
explicit BytesValueOutputStream(const BytesValue& value) { Construct(value); }
4545

4646
bool Next(void** data, int* size) override {
47-
return absl::visit(absl::Overload(
48-
[&data, &size](String& string) -> bool {
49-
return string.stream.Next(data, size);
50-
},
51-
[&data, &size](Cord& cord) -> bool {
52-
return cord.stream.Next(data, size);
53-
}),
54-
AsVariant());
47+
return stream_->Next(data, size);
5548
}
5649

57-
void BackUp(int count) override {
58-
absl::visit(
59-
absl::Overload(
60-
[&count](String& string) -> void { string.stream.BackUp(count); },
61-
[&count](Cord& cord) -> void { cord.stream.BackUp(count); }),
62-
AsVariant());
63-
}
50+
void BackUp(int count) override { stream_->BackUp(count); }
6451

65-
int64_t ByteCount() const override {
66-
return absl::visit(absl::Overload(
67-
[](const String& string) -> int64_t {
68-
return string.stream.ByteCount();
69-
},
70-
[](const Cord& cord) -> int64_t {
71-
return cord.stream.ByteCount();
72-
}),
73-
AsVariant());
74-
}
52+
int64_t ByteCount() const override { return stream_->ByteCount(); }
7553

7654
bool WriteAliasedRaw(const void* data, int size) override {
77-
return absl::visit(absl::Overload(
78-
[&data, &size](String& string) -> bool {
79-
return string.stream.WriteAliasedRaw(data, size);
80-
},
81-
[&data, &size](Cord& cord) -> bool {
82-
return cord.stream.WriteAliasedRaw(data, size);
83-
}),
84-
AsVariant());
55+
return stream_->WriteAliasedRaw(data, size);
8556
}
8657

87-
bool AllowsAliasing() const override {
88-
return absl::visit(absl::Overload(
89-
[](const String& string) -> bool {
90-
return string.stream.AllowsAliasing();
91-
},
92-
[](const Cord& cord) -> bool {
93-
return cord.stream.AllowsAliasing();
94-
}),
95-
AsVariant());
96-
}
58+
bool AllowsAliasing() const override { return stream_->AllowsAliasing(); }
9759

9860
bool WriteCord(const absl::Cord& out) override {
99-
return absl::visit(
100-
absl::Overload(
101-
[&out](String& string) -> bool {
102-
return string.stream.WriteCord(out);
103-
},
104-
[&out](Cord& cord) -> bool { return cord.stream.WriteCord(out); }),
105-
AsVariant());
61+
return stream_->WriteCord(out);
10662
}
10763

64+
[[nodiscard]]
10865
BytesValue Consume(google::protobuf::Arena* absl_nonnull arena) && {
10966
ABSL_DCHECK(arena != nullptr);
110-
return absl::visit(
111-
absl::Overload(
112-
[arena](String& string) -> BytesValue {
113-
return BytesValue::From(std::move(string.target), arena);
114-
},
115-
[arena](Cord& cord) -> BytesValue {
116-
return BytesValue::From(cord.stream.Consume(), arena);
117-
}),
118-
AsVariant());
67+
return std::visit(
68+
absl::Overload([](std::monostate) -> BytesValue { ABSL_UNREACHABLE(); },
69+
[arena](StringStream& stream) -> BytesValue {
70+
return BytesValue::From(std::move(stream.target),
71+
arena);
72+
},
73+
[arena](CordStream& stream) -> BytesValue {
74+
return BytesValue::From(stream.Consume(), arena);
75+
}),
76+
variant_);
11977
}
12078

12179
private:
122-
struct String final {
123-
explicit String(absl::string_view target)
80+
struct StringStream final {
81+
explicit StringStream(absl::string_view target)
12482
: target(target), stream(&this->target) {}
12583

12684
std::string target;
12785
google::protobuf::io::StringOutputStream stream;
12886
};
87+
using CordStream = google::protobuf::io::CordOutputStream;
88+
using Variant = std::variant<std::monostate, StringStream, CordStream>;
12989

130-
struct Cord final {
131-
explicit Cord(const absl::Cord& cord) : stream(cord) {}
132-
133-
google::protobuf::io::CordOutputStream stream;
134-
};
135-
136-
using Variant = absl::variant<String, Cord>;
90+
void Construct() { stream_ = &variant_.emplace<CordStream>(); }
13791

13892
void Construct(const BytesValue& value) {
13993
switch (value.value_.GetKind()) {
@@ -150,26 +104,15 @@ class BytesValueOutputStream final : public google::protobuf::io::ZeroCopyOutput
150104
}
151105

152106
void Construct(absl::string_view value) {
153-
::new (static_cast<void*>(&impl_[0]))
154-
Variant(absl::in_place_type<String>, value);
155-
}
156-
157-
void Construct(const absl::Cord& value) {
158-
::new (static_cast<void*>(&impl_[0]))
159-
Variant(absl::in_place_type<Cord>, value);
160-
}
161-
162-
void Destruct() { AsVariant().~variant(); }
163-
164-
Variant& AsVariant() ABSL_ATTRIBUTE_LIFETIME_BOUND {
165-
return *std::launder(reinterpret_cast<Variant*>(&impl_[0]));
107+
stream_ = &variant_.emplace<StringStream>(value).stream;
166108
}
167109

168-
const Variant& AsVariant() const ABSL_ATTRIBUTE_LIFETIME_BOUND {
169-
return *std::launder(reinterpret_cast<const Variant*>(&impl_[0]));
110+
void Construct(absl::Cord value) {
111+
stream_ = &variant_.emplace<CordStream>(std::move(value));
170112
}
171113

172-
alignas(Variant) char impl_[sizeof(Variant)];
114+
google::protobuf::io::ZeroCopyOutputStream* stream_;
115+
Variant variant_;
173116
};
174117

175118
} // namespace cel

‎common/values/bytes_value_test.cc‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -168,9 +168,25 @@ TEST_F(BytesValueTest, Comparison) {
168168
EXPECT_FALSE(BytesValue::WrapUnsafe("foo") < BytesValue::WrapUnsafe("bar"));
169169
}
170170

171-
TEST_F(BytesValueTest, StringInputStream) {
171+
TEST_F(BytesValueTest, SmallStringInputStream) {
172+
BytesValue value = BytesValue::From("foo", arena());
173+
BytesValueInputStream stream(value);
174+
const void* data;
175+
int size;
176+
absl::Cord cord;
177+
ASSERT_TRUE(stream.Next(&data, &size));
178+
EXPECT_THAT(data, NotNull());
179+
EXPECT_EQ(size, 3);
180+
EXPECT_EQ(stream.ByteCount(), 3);
181+
stream.BackUp(size);
182+
ASSERT_TRUE(stream.Skip(3));
183+
EXPECT_FALSE(stream.ReadCord(&cord, 3));
184+
EXPECT_FALSE(stream.Next(&data, &size));
185+
}
186+
187+
TEST_F(BytesValueTest, MediumStringInputStream) {
172188
BytesValue value = BytesValue::WrapUnsafe("foo");
173-
BytesValueInputStream stream(&value);
189+
BytesValueInputStream stream(value);
174190
const void* data;
175191
int size;
176192
absl::Cord cord;
@@ -185,8 +201,9 @@ TEST_F(BytesValueTest, StringInputStream) {
185201
}
186202

187203
TEST_F(BytesValueTest, CordInputStream) {
188-
BytesValue value = BytesValue::From(absl::Cord("foo"), arena());
189-
BytesValueInputStream stream(&value);
204+
absl::Cord value_cord("foo");
205+
BytesValue value = BytesValue::WrapUnsafe(&value_cord);
206+
BytesValueInputStream stream(value);
190207
const void* data;
191208
int size;
192209
absl::Cord cord;

0 commit comments

Comments
 (0)