Skip to content

Commit 53c7ab3

Browse files
jnthntatumcopybara-github
jnthntatum
authored and
copybara-github
committed
Add Unsafe factories for string values.
BytesValue::WrapUnsafe and StringValue::WrapUnsafe do not attempt to validate ownership, but avoid copying the underlying string data as much as possible. PiperOrigin-RevId: 836237389
1 parent a4c08b8 commit 53c7ab3

File tree

8 files changed

+364
-26
lines changed

8 files changed

+364
-26
lines changed

‎common/internal/byte_string.cc‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,19 @@ ByteString::ByteString(const ReferenceCount* absl_nonnull refcount,
174174
kMetadataOwnerReferenceCountBit);
175175
}
176176

177+
ByteString::ByteString(ByteString::ExternalStringTag,
178+
absl::string_view string) {
179+
if (string.size() <= kSmallByteStringCapacity) {
180+
SetSmall(nullptr, string);
181+
} else {
182+
SetExternalMedium(string);
183+
}
184+
}
185+
186+
ByteString ByteString::FromExternal(absl::string_view string) {
187+
return ByteString(ExternalStringTag{}, string);
188+
}
189+
177190
google::protobuf::Arena* absl_nullable ByteString::GetArena() const {
178191
switch (GetKind()) {
179192
case ByteStringKind::kSmall:
@@ -965,6 +978,14 @@ void ByteString::SetMedium(google::protobuf::Arena* absl_nullable arena,
965978
}
966979
}
967980

981+
void ByteString::SetExternalMedium(absl::string_view string) {
982+
ABSL_DCHECK_GT(string.size(), kSmallByteStringCapacity);
983+
rep_.header.kind = ByteStringKind::kMedium;
984+
rep_.medium.size = string.size();
985+
rep_.medium.data = string.data();
986+
rep_.medium.owner = 0;
987+
}
988+
968989
void ByteString::SetMedium(google::protobuf::Arena* absl_nullable arena,
969990
std::string&& string) {
970991
ABSL_DCHECK_GT(string.size(), kSmallByteStringCapacity);

‎common/internal/byte_string.h‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,10 @@ ByteString final {
232232
const absl::Cord& cord ABSL_ATTRIBUTE_LIFETIME_BOUND)
233233
: ByteString(Borrowed(borrower, cord)) {}
234234

235+
// Creates a medium byte string that is backed by an external string. Should
236+
// only be called from explicit 'Unsafe' factories.
237+
static ByteString FromExternal(absl::string_view string);
238+
235239
~ByteString() { Destroy(); }
236240

237241
ByteString& operator=(const ByteString& other) noexcept {
@@ -358,6 +362,8 @@ ByteString final {
358362
google::protobuf::Arena* absl_nonnull arena);
359363
friend struct cel::ArenaTraits<ByteString>;
360364

365+
struct ExternalStringTag {};
366+
361367
static ByteString Borrowed(Borrower borrower,
362368
absl::string_view string
363369
ABSL_ATTRIBUTE_LIFETIME_BOUND);
@@ -368,6 +374,8 @@ ByteString final {
368374
ByteString(const ReferenceCount* absl_nonnull refcount,
369375
absl::string_view string);
370376

377+
ByteString(ExternalStringTag, absl::string_view string);
378+
371379
constexpr ByteStringKind GetKind() const { return rep_.header.kind; }
372380

373381
absl::string_view GetSmall() const {
@@ -451,6 +459,10 @@ ByteString final {
451459

452460
void SetMedium(google::protobuf::Arena* absl_nullable arena, absl::string_view string);
453461

462+
// This is used to create a medium byte string that is backed by an external
463+
// string. Should only be called from explicit 'Unsafe' factories.
464+
void SetExternalMedium(absl::string_view string);
465+
454466
void SetMedium(google::protobuf::Arena* absl_nullable arena, std::string&& string);
455467

456468
void SetMedium(google::protobuf::Arena* absl_nonnull arena, const absl::Cord& cord);

‎common/internal/byte_string_test.cc‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,29 @@ TEST_P(ByteStringTest, CopyConstruct) {
322322
EXPECT_EQ(ByteString(large_byte_string), large_byte_string);
323323
}
324324

325+
TEST_P(ByteStringTest, CopyConstructFromExternal) {
326+
ByteString small_byte_string = ByteString::FromExternal(GetSmallStringView());
327+
ByteString medium_byte_string =
328+
ByteString::FromExternal(GetMediumStringView());
329+
330+
EXPECT_EQ(ByteString(NewDeleteAllocator(), small_byte_string),
331+
small_byte_string);
332+
EXPECT_EQ(ByteString(NewDeleteAllocator(), medium_byte_string),
333+
medium_byte_string);
334+
335+
google::protobuf::Arena arena;
336+
EXPECT_EQ(ByteString(ArenaAllocator(&arena), small_byte_string),
337+
small_byte_string);
338+
EXPECT_EQ(ByteString(ArenaAllocator(&arena), medium_byte_string),
339+
medium_byte_string);
340+
341+
EXPECT_EQ(ByteString(GetAllocator(), small_byte_string), small_byte_string);
342+
EXPECT_EQ(ByteString(GetAllocator(), medium_byte_string), medium_byte_string);
343+
344+
EXPECT_EQ(ByteString(small_byte_string), small_byte_string);
345+
EXPECT_EQ(ByteString(medium_byte_string), medium_byte_string);
346+
}
347+
325348
TEST_P(ByteStringTest, MoveConstruct) {
326349
const auto& small_byte_string = [this]() {
327350
return ByteString(GetAllocator(), GetSmallStringView());
@@ -360,6 +383,34 @@ TEST_P(ByteStringTest, MoveConstruct) {
360383
EXPECT_EQ(ByteString(large_byte_string()), large_byte_string());
361384
}
362385

386+
TEST_P(ByteStringTest, MoveConstructFromExternal) {
387+
const auto& small_byte_string = []() {
388+
return ByteString::FromExternal(GetSmallStringView());
389+
};
390+
const auto& medium_byte_string = []() {
391+
return ByteString::FromExternal(GetMediumStringView());
392+
};
393+
394+
EXPECT_EQ(ByteString(NewDeleteAllocator(), small_byte_string()),
395+
small_byte_string());
396+
EXPECT_EQ(ByteString(NewDeleteAllocator(), medium_byte_string()),
397+
medium_byte_string());
398+
399+
google::protobuf::Arena arena;
400+
EXPECT_EQ(ByteString(ArenaAllocator(&arena), small_byte_string()),
401+
small_byte_string());
402+
EXPECT_EQ(ByteString(ArenaAllocator(&arena), medium_byte_string()),
403+
medium_byte_string());
404+
405+
EXPECT_EQ(ByteString(GetAllocator(), small_byte_string()),
406+
small_byte_string());
407+
EXPECT_EQ(ByteString(GetAllocator(), medium_byte_string()),
408+
medium_byte_string());
409+
410+
EXPECT_EQ(ByteString(small_byte_string()), small_byte_string());
411+
EXPECT_EQ(ByteString(medium_byte_string()), medium_byte_string());
412+
}
413+
363414
TEST_P(ByteStringTest, CopyFromByteString) {
364415
ByteString small_byte_string =
365416
ByteString(GetAllocator(), GetSmallStringView());

‎common/value.cc‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1551,6 +1551,9 @@ Value WrapFieldImpl(
15511551
if (string.data() == scratch.data() &&
15521552
string.size() == scratch.size()) {
15531553
return StringValue(arena, std::move(scratch));
1554+
}
1555+
if constexpr (Unsafe::value) {
1556+
return StringValue::WrapUnsafe(string);
15541557
} else {
15551558
return StringValue(
15561559
Borrower::Arena(MessageArenaOr(message, arena)), string);
@@ -1586,6 +1589,9 @@ Value WrapFieldImpl(
15861589
if (string.data() == scratch.data() &&
15871590
string.size() == scratch.size()) {
15881591
return BytesValue(arena, std::move(scratch));
1592+
}
1593+
if constexpr (Unsafe::value) {
1594+
return BytesValue::WrapUnsafe(string);
15891595
} else {
15901596
return BytesValue(
15911597
Borrower::Arena(MessageArenaOr(message, arena)), string);
@@ -1674,6 +1680,9 @@ Value WrapRepeatedFieldImpl(
16741680
if (string.data() == scratch.data() &&
16751681
string.size() == scratch.size()) {
16761682
return StringValue(arena, std::move(scratch));
1683+
}
1684+
if constexpr (Unsafe::value) {
1685+
return StringValue::WrapUnsafe(string);
16771686
} else {
16781687
return StringValue(
16791688
Borrower::Arena(MessageArenaOr(message, arena)), string);
@@ -1705,6 +1714,9 @@ Value WrapRepeatedFieldImpl(
17051714
if (string.data() == scratch.data() &&
17061715
string.size() == scratch.size()) {
17071716
return BytesValue(arena, std::move(scratch));
1717+
}
1718+
if constexpr (Unsafe::value) {
1719+
return BytesValue::WrapUnsafe(string);
17081720
} else {
17091721
return BytesValue(
17101722
Borrower::Arena(MessageArenaOr(message, arena)), string);
@@ -1774,8 +1786,12 @@ Value WrapMapFieldValueImpl(
17741786
case google::protobuf::FieldDescriptor::TYPE_BOOL:
17751787
return BoolValue(value.GetBoolValue());
17761788
case google::protobuf::FieldDescriptor::TYPE_STRING:
1777-
return StringValue(Borrower::Arena(MessageArenaOr(message, arena)),
1778-
value.GetStringValue());
1789+
if constexpr (Unsafe::value) {
1790+
return StringValue::WrapUnsafe(value.GetStringValue());
1791+
} else {
1792+
return StringValue(Borrower::Arena(MessageArenaOr(message, arena)),
1793+
value.GetStringValue());
1794+
}
17791795
case google::protobuf::FieldDescriptor::TYPE_GROUP:
17801796
ABSL_FALLTHROUGH_INTENDED;
17811797
case google::protobuf::FieldDescriptor::TYPE_MESSAGE:
@@ -1787,8 +1803,12 @@ Value WrapMapFieldValueImpl(
17871803
message_factory, arena);
17881804
}
17891805
case google::protobuf::FieldDescriptor::TYPE_BYTES:
1790-
return BytesValue(Borrower::Arena(MessageArenaOr(message, arena)),
1791-
value.GetStringValue());
1806+
if constexpr (Unsafe::value) {
1807+
return BytesValue::WrapUnsafe(value.GetStringValue());
1808+
} else {
1809+
return BytesValue(Borrower::Arena(MessageArenaOr(message, arena)),
1810+
value.GetStringValue());
1811+
}
17921812
case google::protobuf::FieldDescriptor::TYPE_FIXED32:
17931813
ABSL_FALLTHROUGH_INTENDED;
17941814
case google::protobuf::FieldDescriptor::TYPE_UINT32:

‎common/values/bytes_value.h‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,13 +74,17 @@ class BytesValue final : private common_internal::ValueMixin<BytesValue> {
7474
static BytesValue Wrap(absl::string_view value,
7575
google::protobuf::Arena* absl_nullable arena
7676
ABSL_ATTRIBUTE_LIFETIME_BOUND);
77-
static BytesValue Wrap(absl::string_view value);
77+
static BytesValue Wrap(absl::string_view value) = delete;
7878
static BytesValue Wrap(const absl::Cord& value);
7979
static BytesValue Wrap(std::string&& value) = delete;
8080
static BytesValue Wrap(std::string&& value,
8181
google::protobuf::Arena* absl_nullable arena
8282
ABSL_ATTRIBUTE_LIFETIME_BOUND) = delete;
8383

84+
// Returns a BytesValue that aliases the provided string. Caller must ensure
85+
// the provided string outlives the use of the returned BytesValue.
86+
static BytesValue WrapUnsafe(absl::string_view value);
87+
8488
static BytesValue Concat(const BytesValue& lhs, const BytesValue& rhs,
8589
google::protobuf::Arena* absl_nonnull arena
8690
ABSL_ATTRIBUTE_LIFETIME_BOUND);
@@ -303,8 +307,8 @@ inline BytesValue BytesValue::Wrap(absl::string_view value,
303307
return BytesValue(Borrower::Arena(arena), value);
304308
}
305309

306-
inline BytesValue BytesValue::Wrap(absl::string_view value) {
307-
return Wrap(value, nullptr);
310+
inline BytesValue BytesValue::WrapUnsafe(absl::string_view value) {
311+
return BytesValue(common_internal::ByteString::FromExternal(value));
308312
}
309313

310314
inline BytesValue BytesValue::Wrap(const absl::Cord& value) {

‎common/values/string_value.h‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,13 +75,17 @@ class StringValue final : private common_internal::ValueMixin<StringValue> {
7575
static StringValue Wrap(absl::string_view value,
7676
google::protobuf::Arena* absl_nullable arena
7777
ABSL_ATTRIBUTE_LIFETIME_BOUND);
78-
static StringValue Wrap(absl::string_view value);
78+
static StringValue Wrap(absl::string_view value) = delete;
7979
static StringValue Wrap(const absl::Cord& value);
8080
static StringValue Wrap(std::string&& value) = delete;
8181
static StringValue Wrap(std::string&& value,
8282
google::protobuf::Arena* absl_nullable arena
8383
ABSL_ATTRIBUTE_LIFETIME_BOUND) = delete;
8484

85+
// Returns a StringValue that aliases the provided string. Caller must ensure
86+
// the provided string outlives the use of the returned StringValue.
87+
static StringValue WrapUnsafe(absl::string_view value);
88+
8589
static StringValue Concat(const StringValue& lhs, const StringValue& rhs,
8690
google::protobuf::Arena* absl_nonnull arena
8791
ABSL_ATTRIBUTE_LIFETIME_BOUND);
@@ -453,8 +457,8 @@ inline StringValue StringValue::Wrap(absl::string_view value,
453457
return StringValue(Borrower::Arena(arena), value);
454458
}
455459

456-
inline StringValue StringValue::Wrap(absl::string_view value) {
457-
return Wrap(value, nullptr);
460+
inline StringValue StringValue::WrapUnsafe(absl::string_view value) {
461+
return StringValue(common_internal::ByteString::FromExternal(value));
458462
}
459463

460464
inline StringValue StringValue::Wrap(const absl::Cord& value) {

‎runtime/BUILD‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -642,6 +642,7 @@ cc_test(
642642
"@com_google_absl//absl/memory",
643643
"@com_google_absl//absl/status:statusor",
644644
"@com_google_absl//absl/strings",
645+
"@com_google_absl//absl/strings:str_format",
645646
"@com_google_absl//absl/types:variant",
646647
"@com_google_cel_spec//proto/cel/expr/conformance/proto3:test_all_types_cc_proto",
647648
"@com_google_protobuf//:any_cc_proto",

0 commit comments

Comments
 (0)