summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--absl/memory/memory.h2
-rw-r--r--absl/meta/type_traits.h2
-rw-r--r--absl/strings/BUILD.bazel1
-rw-r--r--absl/strings/CMakeLists.txt1
-rw-r--r--absl/strings/cord.cc154
-rw-r--r--absl/strings/cord.h77
-rw-r--r--absl/strings/cord_ring_test.cc47
-rw-r--r--absl/strings/cord_test.cc9
-rw-r--r--absl/strings/cord_test_helpers.h23
-rw-r--r--absl/strings/cordz_test.cc133
-rw-r--r--absl/strings/internal/cordz_handle.cc7
-rw-r--r--absl/strings/internal/cordz_handle.h13
-rw-r--r--absl/strings/internal/cordz_handle_test.cc13
-rw-r--r--absl/strings/internal/cordz_info.cc21
-rw-r--r--absl/strings/internal/cordz_info.h22
-rw-r--r--absl/strings/internal/cordz_info_test.cc24
-rw-r--r--absl/strings/internal/cordz_update_tracker.h2
-rw-r--r--absl/strings/internal/cordz_update_tracker_test.cc2
18 files changed, 394 insertions, 159 deletions
diff --git a/absl/memory/memory.h b/absl/memory/memory.h
index 2b5ff623..d6332606 100644
--- a/absl/memory/memory.h
+++ b/absl/memory/memory.h
@@ -420,7 +420,7 @@ struct pointer_traits<T*> {
//
// A C++11 compatible implementation of C++17's std::allocator_traits.
//
-#if __cplusplus >= 201703L
+#if __cplusplus >= 201703L || (defined(_MSVC_LANG) && _MSVC_LANG >= 201703L)
using std::allocator_traits;
#else // __cplusplus >= 201703L
template <typename Alloc>
diff --git a/absl/meta/type_traits.h b/absl/meta/type_traits.h
index b5427a43..e7c12393 100644
--- a/absl/meta/type_traits.h
+++ b/absl/meta/type_traits.h
@@ -634,7 +634,7 @@ using underlying_type_t = typename std::underlying_type<T>::type;
namespace type_traits_internal {
-#if __cplusplus >= 201703L
+#if __cplusplus >= 201703L || (defined(_MSVC_LANG) && _MSVC_LANG >= 201703L)
// std::result_of is deprecated (C++17) or removed (C++20)
template<typename> struct result_of;
template<typename F, typename... Args>
diff --git a/absl/strings/BUILD.bazel b/absl/strings/BUILD.bazel
index ae88f7bb..15277de3 100644
--- a/absl/strings/BUILD.bazel
+++ b/absl/strings/BUILD.bazel
@@ -529,6 +529,7 @@ cc_library(
":cord",
":cord_internal",
":strings",
+ "//absl/base:config",
],
)
diff --git a/absl/strings/CMakeLists.txt b/absl/strings/CMakeLists.txt
index 8ebe91e6..1750f7af 100644
--- a/absl/strings/CMakeLists.txt
+++ b/absl/strings/CMakeLists.txt
@@ -830,6 +830,7 @@ absl_cc_library(
COPTS
${ABSL_TEST_COPTS}
DEPS
+ absl::config
absl::cord
absl::cord_internal
absl::strings
diff --git a/absl/strings/cord.cc b/absl/strings/cord.cc
index 9262aee6..1c2ff9f2 100644
--- a/absl/strings/cord.cc
+++ b/absl/strings/cord.cc
@@ -56,6 +56,7 @@ using ::absl::cord_internal::CordRepExternal;
using ::absl::cord_internal::CordRepFlat;
using ::absl::cord_internal::CordRepRing;
using ::absl::cord_internal::CordRepSubstring;
+using ::absl::cord_internal::CordzUpdateTracker;
using ::absl::cord_internal::InlineData;
using ::absl::cord_internal::kMaxFlatLength;
using ::absl::cord_internal::kMinFlatLength;
@@ -281,6 +282,35 @@ static CordRep* NewSubstring(CordRep* child, size_t offset, size_t length) {
}
}
+// Creates a CordRep from the provided string. If the string is large enough,
+// and not wasteful, we move the string into an external cord rep, preserving
+// the already allocated string contents.
+// Requires the provided string length to be larger than `kMaxInline`.
+static CordRep* CordRepFromString(std::string&& src) {
+ assert(src.length() > cord_internal::kMaxInline);
+ if (
+ // String is short: copy data to avoid external block overhead.
+ src.size() <= kMaxBytesToCopy ||
+ // String is wasteful: copy data to avoid pinning too much unused memory.
+ src.size() < src.capacity() / 2
+ ) {
+ return NewTree(src.data(), src.size(), 0);
+ }
+
+ struct StringReleaser {
+ void operator()(absl::string_view /* data */) {}
+ std::string data;
+ };
+ const absl::string_view original_data = src;
+ auto* rep =
+ static_cast<::absl::cord_internal::CordRepExternalImpl<StringReleaser>*>(
+ absl::cord_internal::NewExternalRep(original_data,
+ StringReleaser{std::move(src)}));
+ // Moving src may have invalidated its data pointer, so adjust it.
+ rep->base = rep->template get<0>().data.data();
+ return rep;
+}
+
// --------------------------------------------------------------------
// Cord::InlineRep functions
@@ -486,17 +516,17 @@ static bool RepMemoryUsageLeaf(const CordRep* rep, size_t* total_mem_usage) {
return true;
}
if (rep->tag == EXTERNAL) {
- *total_mem_usage += sizeof(CordRepConcat) + rep->length;
+ // We don't know anything about the embedded / bound data, but we can safely
+ // assume it is 'at least' a word / pointer to data. In the future we may
+ // choose to use the 'data' byte as a tag to identify the types of some
+ // well-known externals, such as a std::string instance.
+ *total_mem_usage +=
+ sizeof(cord_internal::CordRepExternalImpl<intptr_t>) + rep->length;
return true;
}
return false;
}
-void Cord::InlineRep::UpdateCordzStatisticsSlow() {
- CordRep* tree = as_tree();
- data_.cordz_info()->RecordMetrics(tree->length);
-}
-
void Cord::InlineRep::AssignSlow(const Cord::InlineRep& src) {
assert(&src != this);
assert(is_tree() || src.is_tree());
@@ -525,42 +555,24 @@ void Cord::InlineRep::UnrefTree() {
// --------------------------------------------------------------------
// Constructors and destructors
-Cord::Cord(absl::string_view src) : contents_(InlineData::kDefaultInit) {
+Cord::Cord(absl::string_view src, MethodIdentifier method)
+ : contents_(InlineData::kDefaultInit) {
const size_t n = src.size();
if (n <= InlineRep::kMaxInline) {
contents_.set_data(src.data(), n, true);
} else {
CordRep* rep = NewTree(src.data(), n, 0);
- contents_.EmplaceTree(rep, CordzUpdateTracker::kConstructorString);
+ contents_.EmplaceTree(rep, method);
}
}
template <typename T, Cord::EnableIfString<T>>
-Cord::Cord(T&& src) {
- if (
- // String is short: copy data to avoid external block overhead.
- src.size() <= kMaxBytesToCopy ||
- // String is wasteful: copy data to avoid pinning too much unused memory.
- src.size() < src.capacity() / 2
- ) {
- if (src.size() <= InlineRep::kMaxInline) {
- contents_.set_data(src.data(), src.size(), false);
- } else {
- contents_.set_tree(NewTree(src.data(), src.size(), 0));
- }
+Cord::Cord(T&& src) : contents_(InlineData::kDefaultInit) {
+ if (src.size() <= InlineRep::kMaxInline) {
+ contents_.set_data(src.data(), src.size(), true);
} else {
- struct StringReleaser {
- void operator()(absl::string_view /* data */) {}
- std::string data;
- };
- const absl::string_view original_data = src;
- auto* rep = static_cast<
- ::absl::cord_internal::CordRepExternalImpl<StringReleaser>*>(
- absl::cord_internal::NewExternalRep(
- original_data, StringReleaser{std::forward<T>(src)}));
- // Moving src may have invalidated its data pointer, so adjust it.
- rep->base = rep->template get<0>().data.data();
- contents_.set_tree(rep);
+ CordRep* rep = CordRepFromString(std::forward<T>(src));
+ contents_.EmplaceTree(rep, CordzUpdateTracker::kConstructorString);
}
}
@@ -583,6 +595,20 @@ void Cord::Clear() {
}
}
+Cord& Cord::AssignLargeString(std::string&& src) {
+ auto constexpr method = CordzUpdateTracker::kAssignString;
+ assert(src.size() > kMaxBytesToCopy);
+ CordRep* rep = CordRepFromString(std::move(src));
+ if (CordRep* tree = contents_.tree()) {
+ CordzUpdateScope scope(contents_.cordz_info(), method);
+ contents_.SetTree(rep, scope);
+ CordRep::Unref(tree);
+ } else {
+ contents_.EmplaceTree(rep, method);
+ }
+ return *this;
+}
+
Cord& Cord::operator=(absl::string_view src) {
auto constexpr method = CordzUpdateTracker::kAssignString;
const char* data = src.data();
@@ -616,18 +642,6 @@ Cord& Cord::operator=(absl::string_view src) {
return *this;
}
-template <typename T, Cord::EnableIfString<T>>
-Cord& Cord::operator=(T&& src) {
- if (src.size() <= kMaxBytesToCopy) {
- *this = absl::string_view(src);
- } else {
- *this = Cord(std::forward<T>(src));
- }
- return *this;
-}
-
-template Cord& Cord::operator=(std::string&& src);
-
// TODO(sanjay): Move to Cord::InlineRep section of file. For now,
// we keep it here to make diffs easier.
void Cord::InlineRep::AppendArray(absl::string_view src,
@@ -653,10 +667,8 @@ void Cord::InlineRep::AppendArray(absl::string_view src,
return;
}
- // It is possible that src.data() == data_, but when we transition from an
- // InlineRep to a tree we need to assign data_ = root via set_tree. To
- // avoid corrupting the source data before we copy it, delay calling
- // set_tree until after we've copied data.
+ // Note: we don't concern ourselves if src aliases data stored in the
+ // inlined data of 'this', as we update the InlineData only at the end.
// We are going from an inline size to beyond inline size. Make the new size
// either double the inlined size, or the added size + 10%.
const size_t size1 = inline_length * 2 + src.size();
@@ -762,7 +774,8 @@ void Cord::Append(T&& src) {
if (src.size() <= kMaxBytesToCopy) {
Append(absl::string_view(src));
} else {
- Append(Cord(std::forward<T>(src)));
+ CordRep* rep = CordRepFromString(std::forward<T>(src));
+ contents_.AppendTree(rep, CordzUpdateTracker::kAppendString);
}
}
@@ -804,7 +817,8 @@ inline void Cord::Prepend(T&& src) {
if (src.size() <= kMaxBytesToCopy) {
Prepend(absl::string_view(src));
} else {
- Prepend(Cord(std::forward<T>(src)));
+ CordRep* rep = CordRepFromString(std::forward<T>(src));
+ contents_.PrependTree(rep, CordzUpdateTracker::kPrependString);
}
}
@@ -988,22 +1002,6 @@ static CordRep* NewSubRange(CordRep* node, size_t pos, size_t n) {
return results[0];
}
-void Cord::CopyDataAtPosition(size_t pos, size_t new_size, char* dest) const {
- assert(new_size <= cord_internal::kMaxInline);
- assert(pos <= size());
- assert(new_size <= size() - pos);
- Cord::ChunkIterator it = chunk_begin();
- it.AdvanceBytes(pos);
- size_t remaining_size = new_size;
- while (remaining_size > it->size()) {
- cord_internal::SmallMemmove(dest, it->data(), it->size());
- remaining_size -= it->size();
- dest += it->size();
- ++it;
- }
- cord_internal::SmallMemmove(dest, it->data(), remaining_size);
-}
-
Cord Cord::Subcord(size_t pos, size_t new_size) const {
Cord sub_cord;
size_t length = size();
@@ -1020,7 +1018,17 @@ Cord Cord::Subcord(size_t pos, size_t new_size) const {
}
if (new_size <= InlineRep::kMaxInline) {
- CopyDataAtPosition(pos, new_size, sub_cord.contents_.data_.as_chars());
+ char* dest = sub_cord.contents_.data_.as_chars();
+ Cord::ChunkIterator it = chunk_begin();
+ it.AdvanceBytes(pos);
+ size_t remaining_size = new_size;
+ while (remaining_size > it->size()) {
+ cord_internal::SmallMemmove(dest, it->data(), it->size());
+ remaining_size -= it->size();
+ dest += it->size();
+ ++it;
+ }
+ cord_internal::SmallMemmove(dest, it->data(), remaining_size);
sub_cord.contents_.set_inline_size(new_size);
return sub_cord;
}
@@ -1474,6 +1482,7 @@ Cord Cord::ChunkIterator::AdvanceAndReadBytes(size_t n) {
ABSL_HARDENING_ASSERT(bytes_remaining_ >= n &&
"Attempted to iterate past `end()`");
Cord subcord;
+ auto constexpr method = CordzUpdateTracker::kCordReader;
if (n <= InlineRep::kMaxInline) {
// Range to read fits in inline data. Flatten it.
@@ -1496,11 +1505,12 @@ Cord Cord::ChunkIterator::AdvanceAndReadBytes(size_t n) {
if (ring_reader_) {
size_t chunk_size = current_chunk_.size();
if (n <= chunk_size && n <= kMaxBytesToCopy) {
- subcord = Cord(current_chunk_.substr(0, n));
+ subcord = Cord(current_chunk_.substr(0, n), method);
} else {
auto* ring = CordRep::Ref(ring_reader_.ring())->ring();
size_t offset = ring_reader_.length() - bytes_remaining_;
- subcord.contents_.set_tree(CordRepRing::SubRing(ring, offset, n));
+ CordRep* rep = CordRepRing::SubRing(ring, offset, n);
+ subcord.contents_.EmplaceTree(rep, method);
}
if (n < chunk_size) {
bytes_remaining_ -= n;
@@ -1519,7 +1529,7 @@ Cord Cord::ChunkIterator::AdvanceAndReadBytes(size_t n) {
const char* data = subnode->tag == EXTERNAL ? subnode->external()->base
: subnode->flat()->Data();
subnode = NewSubstring(subnode, current_chunk_.data() - data, n);
- subcord.contents_.set_tree(VerifyTree(subnode));
+ subcord.contents_.EmplaceTree(VerifyTree(subnode), method);
RemoveChunkPrefix(n);
return subcord;
}
@@ -1562,7 +1572,7 @@ Cord Cord::ChunkIterator::AdvanceAndReadBytes(size_t n) {
if (node == nullptr) {
// We have reached the end of the Cord.
assert(bytes_remaining_ == 0);
- subcord.contents_.set_tree(VerifyTree(subnode));
+ subcord.contents_.EmplaceTree(VerifyTree(subnode), method);
return subcord;
}
@@ -1602,7 +1612,7 @@ Cord Cord::ChunkIterator::AdvanceAndReadBytes(size_t n) {
current_chunk_ = absl::string_view(data + offset + n, length - n);
current_leaf_ = node;
bytes_remaining_ -= n;
- subcord.contents_.set_tree(VerifyTree(subnode));
+ subcord.contents_.EmplaceTree(VerifyTree(subnode), method);
return subcord;
}
diff --git a/absl/strings/cord.h b/absl/strings/cord.h
index d7b43247..8abc4742 100644
--- a/absl/strings/cord.h
+++ b/absl/strings/cord.h
@@ -678,6 +678,10 @@ class Cord {
using InlineData = cord_internal::InlineData;
using MethodIdentifier = CordzUpdateTracker::MethodIdentifier;
+ // Creates a cord instance with `method` representing the originating
+ // public API call causing the cord to be created.
+ explicit Cord(absl::string_view src, MethodIdentifier method);
+
friend class CordTestPeer;
friend bool operator==(const Cord& lhs, const Cord& rhs);
friend bool operator==(const Cord& lhs, absl::string_view rhs);
@@ -692,10 +696,6 @@ class Cord {
// called by Flatten() when the cord was not already flat.
absl::string_view FlattenSlowPath();
- // Copies `new_size` bytes starting at `pos` into `dest`. Requires at least
- // `new_size` bytes to be available, and `new_size` to be <= kMaxInline.
- void CopyDataAtPosition(size_t pos, size_t new_size, char* dest) const;
-
// Actual cord contents are hidden inside the following simple
// class so that we can isolate the bulk of cord.cc from changes
// to the representation.
@@ -725,11 +725,6 @@ class Cord {
// Returns nullptr if holding bytes
absl::cord_internal::CordRep* tree() const;
absl::cord_internal::CordRep* as_tree() const;
- // Discards old pointer, if any
- void set_tree(absl::cord_internal::CordRep* rep);
- // Replaces a tree with a new root. This is faster than set_tree, but it
- // should only be used when it's clear that the old rep was a tree.
- void replace_tree(absl::cord_internal::CordRep* rep);
// Returns non-null iff was holding a pointer
absl::cord_internal::CordRep* clear();
// Converts to pointer if necessary.
@@ -831,11 +826,6 @@ class Cord {
// Resets the current cordz_info to null / empty.
void clear_cordz_info() { data_.clear_cordz_info(); }
- // Updates the cordz statistics. info may be nullptr if the CordzInfo object
- // is unknown.
- void UpdateCordzStatistics();
- void UpdateCordzStatisticsSlow();
-
private:
friend class Cord;
@@ -892,6 +882,10 @@ class Cord {
template <typename C>
void AppendImpl(C&& src);
+ // Assigns the value in 'src' to this instance, 'stealing' its contents.
+ // Requires src.length() > kMaxBytesToCopy.
+ Cord& AssignLargeString(std::string&& src);
+
// Helper for AbslHashValue().
template <typename H>
H HashFragmented(H hash_state) const {
@@ -994,8 +988,11 @@ inline CordRep* NewExternalRep(absl::string_view data,
template <typename Releaser>
Cord MakeCordFromExternal(absl::string_view data, Releaser&& releaser) {
Cord cord;
- cord.contents_.set_tree(::absl::cord_internal::NewExternalRep(
- data, std::forward<Releaser>(releaser)));
+ if (auto* rep = ::absl::cord_internal::NewExternalRep(
+ data, std::forward<Releaser>(releaser))) {
+ cord.contents_.EmplaceTree(rep,
+ Cord::MethodIdentifier::kMakeCordFromExternal);
+ }
return cord;
}
@@ -1119,36 +1116,6 @@ inline void Cord::InlineRep::CommitTree(const CordRep* old_rep, CordRep* rep,
}
}
-inline void Cord::InlineRep::set_tree(absl::cord_internal::CordRep* rep) {
- if (rep == nullptr) {
- if (data_.is_tree()) {
- CordzInfo::MaybeUntrackCord(data_.cordz_info());
- }
- ResetToEmpty();
- } else {
- if (data_.is_tree()) {
- // `data_` already holds a 'tree' value and an optional cordz_info value.
- // Replace the tree value only, leaving the cordz_info value unchanged.
- data_.set_tree(rep);
- } else {
- // `data_` contains inlined data: initialize data_ to tree value `rep`.
- data_.make_tree(rep);
- CordzInfo::MaybeTrackCord(data_, CordzUpdateTracker::kUnknown);
- }
- UpdateCordzStatistics();
- }
-}
-
-inline void Cord::InlineRep::replace_tree(absl::cord_internal::CordRep* rep) {
- ABSL_ASSERT(is_tree());
- if (ABSL_PREDICT_FALSE(rep == nullptr)) {
- set_tree(rep);
- return;
- }
- data_.set_tree(rep);
- UpdateCordzStatistics();
-}
-
inline absl::cord_internal::CordRep* Cord::InlineRep::clear() {
if (is_tree()) {
CordzInfo::MaybeUntrackCord(cordz_info());
@@ -1165,13 +1132,11 @@ inline void Cord::InlineRep::CopyToArray(char* dst) const {
cord_internal::SmallMemmove(dst, data_.as_chars(), n);
}
-inline void Cord::InlineRep::UpdateCordzStatistics() {
- if (ABSL_PREDICT_TRUE(!is_profiled())) return;
- UpdateCordzStatisticsSlow();
-}
-
constexpr inline Cord::Cord() noexcept {}
+inline Cord::Cord(absl::string_view src)
+ : Cord(src, CordzUpdateTracker::kConstructorString) {}
+
template <typename T>
constexpr Cord::Cord(strings_internal::StringConstant<T>)
: contents_(strings_internal::StringConstant<T>::value.size() <=
@@ -1187,6 +1152,15 @@ inline Cord& Cord::operator=(const Cord& x) {
return *this;
}
+template <typename T, Cord::EnableIfString<T>>
+Cord& Cord::operator=(T&& src) {
+ if (src.size() <= cord_internal::kMaxBytesToCopy) {
+ return operator=(absl::string_view(src));
+ } else {
+ return AssignLargeString(std::forward<T>(src));
+ }
+}
+
inline Cord::Cord(const Cord& src) : contents_(src.contents_) {}
inline Cord::Cord(Cord&& src) noexcept : contents_(std::move(src.contents_)) {}
@@ -1201,7 +1175,6 @@ inline Cord& Cord::operator=(Cord&& x) noexcept {
}
extern template Cord::Cord(std::string&& src);
-extern template Cord& Cord::operator=(std::string&& src);
inline size_t Cord::size() const {
// Length is 1st field in str.rep_
diff --git a/absl/strings/cord_ring_test.cc b/absl/strings/cord_ring_test.cc
index 2943cf38..cc8fbaf9 100644
--- a/absl/strings/cord_ring_test.cc
+++ b/absl/strings/cord_ring_test.cc
@@ -98,15 +98,22 @@ using TestParams = std::vector<TestParam>;
// Matcher validating when mutable copies are required / performed.
MATCHER_P2(EqIfPrivate, param, rep,
absl::StrCat("Equal 0x", absl::Hex(rep), " if private")) {
- return param.refcount_is_one ? arg == rep : arg != rep;
+ return param.refcount_is_one ? arg == rep : true;
}
// Matcher validating when mutable copies are required / performed.
MATCHER_P2(EqIfPrivateAndCapacity, param, rep,
absl::StrCat("Equal 0x", absl::Hex(rep),
" if private and capacity")) {
- return (param.refcount_is_one && param.with_capacity) ? arg == rep
- : arg != rep;
+ return (param.refcount_is_one && param.with_capacity) ? arg == rep : true;
+}
+
+// Matcher validating a shared ring was re-allocated. Should only be used for
+// tests doing exactly one update as subsequent updates could return the
+// original (freed and re-used) pointer.
+MATCHER_P2(NeIfShared, param, rep,
+ absl::StrCat("Not equal 0x", absl::Hex(rep), " if shared")) {
+ return param.refcount_is_one ? true : arg != rep;
}
MATCHER_P2(EqIfInputPrivate, param, rep, "Equal if input is private") {
@@ -518,6 +525,7 @@ TEST_P(CordRingCreateTest, CreateFromRing) {
CordRepRing* result = NeedsUnref(CordRepRing::Create(ring));
ASSERT_THAT(result, IsValidRingBuffer());
EXPECT_THAT(result, EqIfPrivate(GetParam(), ring));
+ EXPECT_THAT(result, NeIfShared(GetParam(), ring));
EXPECT_THAT(ToFlats(result), ElementsAreArray(kFoxFlats));
}
@@ -655,6 +663,7 @@ TEST_P(CordRingBuildTest, AppendFlat) {
CordRepRing* result = NeedsUnref(CordRepRing::Append(ring, MakeFlat(str2)));
ASSERT_THAT(result, IsValidRingBuffer());
EXPECT_THAT(result, EqIfPrivateAndCapacity(GetParam(), ring));
+ EXPECT_THAT(result, NeIfShared(GetParam(), ring));
EXPECT_THAT(result->length, Eq(str1.size() + str2.size()));
EXPECT_THAT(ToFlats(result), ElementsAre(str1, str2));
}
@@ -666,6 +675,7 @@ TEST_P(CordRingBuildTest, PrependFlat) {
CordRepRing* result = NeedsUnref(CordRepRing::Prepend(ring, MakeFlat(str2)));
ASSERT_THAT(result, IsValidRingBuffer());
EXPECT_THAT(result, EqIfPrivateAndCapacity(GetParam(), ring));
+ EXPECT_THAT(result, NeIfShared(GetParam(), ring));
EXPECT_THAT(result->length, Eq(str1.size() + str2.size()));
EXPECT_THAT(ToFlats(result), ElementsAre(str2, str1));
}
@@ -677,6 +687,7 @@ TEST_P(CordRingBuildTest, AppendString) {
CordRepRing* result = NeedsUnref(CordRepRing::Append(ring, str2));
ASSERT_THAT(result, IsValidRingBuffer());
EXPECT_THAT(result, EqIfPrivateAndCapacity(GetParam(), ring));
+ EXPECT_THAT(result, NeIfShared(GetParam(), ring));
EXPECT_THAT(result->length, Eq(str1.size() + str2.size()));
EXPECT_THAT(ToFlats(result), ElementsAre(str1, str2));
}
@@ -689,6 +700,7 @@ TEST_P(CordRingBuildTest, AppendStringHavingExtra) {
ASSERT_THAT(result, IsValidRingBuffer());
EXPECT_THAT(result->length, Eq(str1.size() + str2.size()));
EXPECT_THAT(result, EqIfPrivate(GetParam(), ring));
+ EXPECT_THAT(result, NeIfShared(GetParam(), ring));
}
TEST_P(CordRingBuildTest, AppendStringHavingPartialExtra) {
@@ -710,6 +722,7 @@ TEST_P(CordRingBuildTest, AppendStringHavingPartialExtra) {
ASSERT_THAT(result, IsValidRingBuffer());
EXPECT_THAT(result->length, Eq(str1.size() + str2.size()));
EXPECT_THAT(result, EqIfPrivateAndCapacity(GetParam(), ring));
+ EXPECT_THAT(result, NeIfShared(GetParam(), ring));
if (GetParam().refcount_is_one) {
EXPECT_THAT(ToFlats(result), ElementsAre(StrCat(str1, str1a), str2a));
} else {
@@ -725,6 +738,7 @@ TEST_P(CordRingBuildTest, AppendStringHavingExtraInSubstring) {
CordRepRing* result = NeedsUnref(CordRepRing::Append(ring, str2));
ASSERT_THAT(result, IsValidRingBuffer());
EXPECT_THAT(result, EqIfPrivate(GetParam(), ring));
+ EXPECT_THAT(result, NeIfShared(GetParam(), ring));
EXPECT_THAT(result->length, Eq(4 + str2.size()));
if (GetParam().refcount_is_one) {
EXPECT_THAT(ToFlats(result), ElementsAre(StrCat("1234", str2)));
@@ -758,6 +772,7 @@ TEST_P(CordRingBuildTest, AppendStringHavingSharedExtra) {
CordRepRing* result = NeedsUnref(CordRepRing::Append(ring, str2));
ASSERT_THAT(result, IsValidRingBuffer());
EXPECT_THAT(result, EqIfPrivateAndCapacity(GetParam(), ring));
+ EXPECT_THAT(result, NeIfShared(GetParam(), ring));
EXPECT_THAT(result->length, Eq(4 + str2.size()));
EXPECT_THAT(ToFlats(result), ElementsAre("1234", str2));
@@ -802,6 +817,7 @@ TEST_P(CordRingBuildTest, PrependStringHavingExtra) {
CordRepRing* result = NeedsUnref(CordRepRing::Prepend(ring, str2));
ASSERT_THAT(result, IsValidRingBuffer());
EXPECT_THAT(result, EqIfPrivate(GetParam(), ring));
+ EXPECT_THAT(result, NeIfShared(GetParam(), ring));
EXPECT_THAT(result->length, Eq(4 + str2.size()));
if (GetParam().refcount_is_one) {
EXPECT_THAT(ToFlats(result), ElementsAre(StrCat(str2, "1234")));
@@ -833,6 +849,7 @@ TEST_P(CordRingBuildTest, PrependStringHavingSharedExtra) {
ASSERT_THAT(result, IsValidRingBuffer());
EXPECT_THAT(result->length, Eq(str1a.size() + str2.size()));
EXPECT_THAT(result, EqIfPrivateAndCapacity(GetParam(), ring));
+ EXPECT_THAT(result, NeIfShared(GetParam(), ring));
EXPECT_THAT(ToFlats(result), ElementsAre(str2, str1a));
CordRep::Unref(shared_type == 1 ? flat1 : flat);
}
@@ -920,6 +937,7 @@ TEST_P(CordRingSubTest, SubRing) {
result = NeedsUnref(CordRepRing::SubRing(ring, offset, len));
ASSERT_THAT(result, IsValidRingBuffer());
ASSERT_THAT(result, EqIfPrivate(GetParam(), ring));
+ ASSERT_THAT(result, NeIfShared(GetParam(), ring));
ASSERT_THAT(ToString(result), Eq(all.substr(offset, len)));
}
}
@@ -945,6 +963,7 @@ TEST_P(CordRingSubTest, SubRingFromLargeExternal) {
result = NeedsUnref(CordRepRing::SubRing(ring, offset, len));
ASSERT_THAT(result, IsValidRingBuffer());
ASSERT_THAT(result, EqIfPrivate(GetParam(), ring));
+ ASSERT_THAT(result, NeIfShared(GetParam(), ring));
auto str = ToString(result);
ASSERT_THAT(str, SizeIs(len));
ASSERT_THAT(str, Eq(all.substr(offset, len)));
@@ -966,6 +985,7 @@ TEST_P(CordRingSubTest, RemovePrefix) {
result = NeedsUnref(CordRepRing::RemovePrefix(ring, len));
ASSERT_THAT(result, IsValidRingBuffer());
EXPECT_THAT(result, EqIfPrivate(GetParam(), ring));
+ ASSERT_THAT(result, NeIfShared(GetParam(), ring));
EXPECT_THAT(ToString(result), Eq(all.substr(len)));
}
}
@@ -996,8 +1016,9 @@ TEST_P(CordRingSubTest, RemoveSuffix) {
ring = RefIfShared(FromFlats(flats, composition));
result = NeedsUnref(CordRepRing::RemoveSuffix(ring, len));
ASSERT_THAT(result, IsValidRingBuffer());
- EXPECT_THAT(result, EqIfPrivate(GetParam(), ring));
- EXPECT_THAT(ToString(result), Eq(all.substr(0, all.size() - len)));
+ ASSERT_THAT(result, EqIfPrivate(GetParam(), ring));
+ ASSERT_THAT(result, NeIfShared(GetParam(), ring));
+ ASSERT_THAT(ToString(result), Eq(all.substr(0, all.size() - len)));
}
}
@@ -1010,6 +1031,7 @@ TEST_P(CordRingSubTest, AppendRing) {
CordRepRing* result = NeedsUnref(CordRepRing::Append(ring, child));
ASSERT_THAT(result, IsValidRingBuffer());
EXPECT_THAT(result, EqIfPrivate(GetParam(), ring));
+ EXPECT_THAT(result, NeIfShared(GetParam(), ring));
EXPECT_THAT(ToFlats(result), ElementsAreArray(kFoxFlats));
}
@@ -1023,6 +1045,7 @@ TEST_P(CordRingBuildInputTest, AppendRingWithFlatOffset) {
CordRepRing* result = NeedsUnref(CordRepRing::Append(ring, stripped));
ASSERT_THAT(result, IsValidRingBuffer());
EXPECT_THAT(result, EqIfPrivateAndCapacity(GetParam(), ring));
+ EXPECT_THAT(result, NeIfShared(GetParam(), ring));
EXPECT_THAT(ToFlats(result), ElementsAre("Head", "brown ", "fox ", "jumps ",
"over ", "the ", "lazy ", "dog"));
}
@@ -1037,6 +1060,7 @@ TEST_P(CordRingBuildInputTest, AppendRingWithBrokenOffset) {
CordRepRing* result = NeedsUnref(CordRepRing::Append(ring, stripped));
ASSERT_THAT(result, IsValidRingBuffer());
EXPECT_THAT(result, EqIfPrivateAndCapacity(GetParam(), ring));
+ EXPECT_THAT(result, NeIfShared(GetParam(), ring));
EXPECT_THAT(ToFlats(result),
ElementsAre("Head", "umps ", "over ", "the ", "lazy ", "dog"));
}
@@ -1051,6 +1075,7 @@ TEST_P(CordRingBuildInputTest, AppendRingWithFlatLength) {
CordRepRing* result = NeedsUnref(CordRepRing::Append(ring, stripped));
ASSERT_THAT(result, IsValidRingBuffer());
EXPECT_THAT(result, EqIfPrivateAndCapacity(GetParam(), ring));
+ EXPECT_THAT(result, NeIfShared(GetParam(), ring));
EXPECT_THAT(ToFlats(result), ElementsAre("Head", "The ", "quick ", "brown ",
"fox ", "jumps ", "over ", "the "));
}
@@ -1065,6 +1090,7 @@ TEST_P(CordRingBuildTest, AppendRingWithBrokenFlatLength) {
CordRepRing* result = NeedsUnref(CordRepRing::Append(ring, stripped));
ASSERT_THAT(result, IsValidRingBuffer());
EXPECT_THAT(result, EqIfPrivateAndCapacity(GetParam(), ring));
+ EXPECT_THAT(result, NeIfShared(GetParam(), ring));
EXPECT_THAT(ToFlats(result), ElementsAre("Head", "The ", "quick ", "brown ",
"fox ", "jumps ", "ov"));
}
@@ -1079,6 +1105,7 @@ TEST_P(CordRingBuildTest, AppendRingMiddlePiece) {
CordRepRing* result = NeedsUnref(CordRepRing::Append(ring, stripped));
ASSERT_THAT(result, IsValidRingBuffer());
EXPECT_THAT(result, EqIfPrivateAndCapacity(GetParam(), ring));
+ EXPECT_THAT(result, NeIfShared(GetParam(), ring));
EXPECT_THAT(ToFlats(result),
ElementsAre("Head", "ck ", "brown ", "fox ", "jum"));
}
@@ -1093,6 +1120,7 @@ TEST_P(CordRingBuildTest, AppendRingSinglePiece) {
CordRepRing* result = NeedsUnref(CordRepRing::Append(ring, stripped));
ASSERT_THAT(result, IsValidRingBuffer());
EXPECT_THAT(result, EqIfPrivateAndCapacity(GetParam(), ring));
+ EXPECT_THAT(result, NeIfShared(GetParam(), ring));
EXPECT_THAT(ToFlats(result), ElementsAre("Head", "row"));
}
@@ -1110,6 +1138,7 @@ TEST_P(CordRingBuildInputTest, AppendRingSinglePieceWithPrefix) {
CordRepRing* result = NeedsUnref(CordRepRing::Append(ring, stripped));
ASSERT_THAT(result, IsValidRingBuffer());
EXPECT_THAT(result, EqIfPrivateAndCapacity(GetParam(), ring));
+ EXPECT_THAT(result, NeIfShared(GetParam(), ring));
EXPECT_THAT(ToFlats(result), ElementsAre("Prepend", "Head", "row"));
}
@@ -1123,6 +1152,7 @@ TEST_P(CordRingBuildInputTest, PrependRing) {
CordRepRing* result = NeedsUnref(CordRepRing::Prepend(ring, child));
ASSERT_THAT(result, IsValidRingBuffer());
EXPECT_THAT(result, EqIfPrivateAndCapacity(GetParam(), ring));
+ EXPECT_THAT(result, NeIfShared(GetParam(), ring));
EXPECT_THAT(ToFlats(result), ElementsAreArray(kFoxFlats));
}
@@ -1136,6 +1166,7 @@ TEST_P(CordRingBuildInputTest, PrependRingWithFlatOffset) {
CordRepRing* result = NeedsUnref(CordRepRing::Prepend(ring, stripped));
ASSERT_THAT(result, IsValidRingBuffer());
EXPECT_THAT(result, EqIfPrivateAndCapacity(GetParam(), ring));
+ EXPECT_THAT(result, NeIfShared(GetParam(), ring));
EXPECT_THAT(ToFlats(result), ElementsAre("brown ", "fox ", "jumps ", "over ",
"the ", "lazy ", "dog", "Tail"));
}
@@ -1149,6 +1180,7 @@ TEST_P(CordRingBuildInputTest, PrependRingWithBrokenOffset) {
CordRep* stripped = RefIfInputSharedIndirect(RemovePrefix(21, child));
CordRepRing* result = NeedsUnref(CordRepRing::Prepend(ring, stripped));
EXPECT_THAT(result, EqIfPrivateAndCapacity(GetParam(), ring));
+ EXPECT_THAT(result, NeIfShared(GetParam(), ring));
EXPECT_THAT(ToFlats(result),
ElementsAre("umps ", "over ", "the ", "lazy ", "dog", "Tail"));
}
@@ -1163,6 +1195,7 @@ TEST_P(CordRingBuildInputTest, PrependRingWithFlatLength) {
CordRepRing* result = NeedsUnref(CordRepRing::Prepend(ring, stripped));
ASSERT_THAT(result, IsValidRingBuffer());
EXPECT_THAT(result, EqIfPrivateAndCapacity(GetParam(), ring));
+ EXPECT_THAT(result, NeIfShared(GetParam(), ring));
EXPECT_THAT(ToFlats(result), ElementsAre("The ", "quick ", "brown ", "fox ",
"jumps ", "over ", "the ", "Tail"));
}
@@ -1177,6 +1210,7 @@ TEST_P(CordRingBuildInputTest, PrependRingWithBrokenFlatLength) {
CordRepRing* result = NeedsUnref(CordRepRing::Prepend(ring, stripped));
ASSERT_THAT(result, IsValidRingBuffer());
EXPECT_THAT(result, EqIfPrivateAndCapacity(GetParam(), ring));
+ EXPECT_THAT(result, NeIfShared(GetParam(), ring));
EXPECT_THAT(ToFlats(result), ElementsAre("The ", "quick ", "brown ", "fox ",
"jumps ", "ov", "Tail"));
}
@@ -1192,6 +1226,7 @@ TEST_P(CordRingBuildInputTest, PrependRingMiddlePiece) {
CordRepRing* result = NeedsUnref(CordRepRing::Prepend(ring, stripped));
ASSERT_THAT(result, IsValidRingBuffer());
EXPECT_THAT(result, EqIfPrivateAndCapacity(GetParam(), ring));
+ EXPECT_THAT(result, NeIfShared(GetParam(), ring));
EXPECT_THAT(ToFlats(result),
ElementsAre("ck ", "brown ", "fox ", "jum", "Tail"));
}
@@ -1206,6 +1241,7 @@ TEST_P(CordRingBuildInputTest, PrependRingSinglePiece) {
CordRepRing* result = NeedsUnref(CordRepRing::Prepend(ring, stripped));
ASSERT_THAT(result, IsValidRingBuffer());
EXPECT_THAT(result, EqIfPrivateAndCapacity(GetParam(), ring));
+ EXPECT_THAT(result, NeIfShared(GetParam(), ring));
EXPECT_THAT(ToFlats(result), ElementsAre("row", "Tail"));
}
@@ -1222,6 +1258,7 @@ TEST_P(CordRingBuildInputTest, PrependRingSinglePieceWithPrefix) {
CordRepRing* result = NeedsUnref(CordRepRing::Prepend(ring, stripped));
ASSERT_THAT(result, IsValidRingBuffer());
EXPECT_THAT(result, EqIfPrivateAndCapacity(GetParam(), ring));
+ EXPECT_THAT(result, NeIfShared(GetParam(), ring));
EXPECT_THAT(ToFlats(result), ElementsAre("row", "Prepend", "Tail"));
}
diff --git a/absl/strings/cord_test.cc b/absl/strings/cord_test.cc
index 74a90863..14eca155 100644
--- a/absl/strings/cord_test.cc
+++ b/absl/strings/cord_test.cc
@@ -190,14 +190,15 @@ class CordTestPeer {
}
static Cord MakeSubstring(Cord src, size_t offset, size_t length) {
- Cord cord = src;
- ABSL_RAW_CHECK(cord.contents_.is_tree(), "Can not be inlined");
+ ABSL_RAW_CHECK(src.contents_.is_tree(), "Can not be inlined");
+ Cord cord;
auto* rep = new cord_internal::CordRepSubstring;
rep->tag = cord_internal::SUBSTRING;
- rep->child = cord.contents_.tree();
+ rep->child = cord_internal::CordRep::Ref(src.contents_.tree());
rep->start = offset;
rep->length = length;
- cord.contents_.replace_tree(rep);
+ cord.contents_.EmplaceTree(rep,
+ cord_internal::CordzUpdateTracker::kSubCord);
return cord;
}
};
diff --git a/absl/strings/cord_test_helpers.h b/absl/strings/cord_test_helpers.h
index 6ccccc51..31a1dc89 100644
--- a/absl/strings/cord_test_helpers.h
+++ b/absl/strings/cord_test_helpers.h
@@ -19,7 +19,9 @@
#include <cstdint>
#include <iostream>
+#include <string>
+#include "absl/base/config.h"
#include "absl/strings/cord.h"
#include "absl/strings/internal/cord_internal.h"
#include "absl/strings/string_view.h"
@@ -29,10 +31,27 @@ ABSL_NAMESPACE_BEGIN
// Cord sizes relevant for testing
enum class TestCordSize {
+ // An empty value
kEmpty = 0,
+
+ // An inlined string value
kInlined = cord_internal::kMaxInline / 2 + 1,
+
+ // 'Well known' SSO lengths (excluding terminating zero).
+ // libstdcxx has a maximum SSO of 15, libc++ has a maximum SSO of 22.
+ kStringSso1 = 15,
+ kStringSso2 = 22,
+
+ // A string value which is too large to fit in inlined data, but small enough
+ // such that Cord prefers copying the value if possible, i.e.: not stealing
+ // std::string inputs, or referencing existing CordReps on Append, etc.
kSmall = cord_internal::kMaxBytesToCopy / 2 + 1,
+
+ // A string value large enough that Cord prefers to reference or steal from
+ // existing inputs rather than copying contents of the input.
kMedium = cord_internal::kMaxFlatLength / 2 + 1,
+
+ // A string value large enough to cause it to be stored in mutliple flats.
kLarge = cord_internal::kMaxFlatLength * 4
};
@@ -45,6 +64,10 @@ inline absl::string_view ToString(TestCordSize size) {
return "Inlined";
case TestCordSize::kSmall:
return "Small";
+ case TestCordSize::kStringSso1:
+ return "StringSso1";
+ case TestCordSize::kStringSso2:
+ return "StringSso2";
case TestCordSize::kMedium:
return "Medium";
case TestCordSize::kLarge:
diff --git a/absl/strings/cordz_test.cc b/absl/strings/cordz_test.cc
index 84947cfd..aeb3d13f 100644
--- a/absl/strings/cordz_test.cc
+++ b/absl/strings/cordz_test.cc
@@ -34,6 +34,7 @@
#ifdef ABSL_INTERNAL_CORDZ_ENABLED
using testing::Eq;
+using testing::AnyOf;
namespace absl {
ABSL_NAMESPACE_BEGIN
@@ -84,24 +85,50 @@ class CordzUpdateTest : public testing::TestWithParam<TestCordSize> {
Cord cord_{MakeString(GetParam())};
};
+template <typename T>
+std::string ParamToString(::testing::TestParamInfo<T> param) {
+ return std::string(ToString(param.param));
+}
+
INSTANTIATE_TEST_SUITE_P(WithParam, CordzUpdateTest,
testing::Values(TestCordSize::kEmpty,
TestCordSize::kInlined,
TestCordSize::kLarge),
TestParamToString);
-TEST(CordzTest, ConstructSmallString) {
+class CordzStringTest : public testing::TestWithParam<TestCordSize> {
+ private:
+ CordzSamplingIntervalHelper sample_every_{1};
+};
+
+INSTANTIATE_TEST_SUITE_P(WithParam, CordzStringTest,
+ testing::Values(TestCordSize::kInlined,
+ TestCordSize::kStringSso1,
+ TestCordSize::kStringSso2,
+ TestCordSize::kSmall,
+ TestCordSize::kLarge),
+ ParamToString<TestCordSize>);
+
+TEST(CordzTest, ConstructSmallArray) {
CordzSamplingIntervalHelper sample_every{1};
Cord cord(MakeString(TestCordSize::kSmall));
EXPECT_THAT(cord, HasValidCordzInfoOf(Method::kConstructorString));
}
-TEST(CordzTest, ConstructLargeString) {
+TEST(CordzTest, ConstructLargeArray) {
CordzSamplingIntervalHelper sample_every{1};
Cord cord(MakeString(TestCordSize::kLarge));
EXPECT_THAT(cord, HasValidCordzInfoOf(Method::kConstructorString));
}
+TEST_P(CordzStringTest, ConstructString) {
+ CordzSamplingIntervalHelper sample_every{1};
+ Cord cord(std::string(Length(GetParam()), '.'));
+ if (Length(GetParam()) > kMaxInline) {
+ EXPECT_THAT(cord, HasValidCordzInfoOf(Method::kConstructorString));
+ }
+}
+
TEST(CordzTest, CopyConstruct) {
CordzSamplingIntervalHelper sample_every{1};
Cord src = UnsampledCord(MakeString(TestCordSize::kLarge));
@@ -151,6 +178,28 @@ TEST_P(CordzUpdateTest, AssignInlinedArray) {
EXPECT_THAT(GetCordzInfoForTesting(cord()), Eq(nullptr));
}
+TEST_P(CordzStringTest, AssignStringToInlined) {
+ Cord cord;
+ cord = std::string(Length(GetParam()), '.');
+ if (Length(GetParam()) > kMaxInline) {
+ EXPECT_THAT(cord, HasValidCordzInfoOf(Method::kAssignString));
+ }
+}
+
+TEST_P(CordzStringTest, AssignStringToCord) {
+ Cord cord(MakeString(TestCordSize::kLarge));
+ cord = std::string(Length(GetParam()), '.');
+ if (Length(GetParam()) > kMaxInline) {
+ EXPECT_THAT(cord, HasValidCordzInfoOf(Method::kConstructorString));
+ EXPECT_THAT(cord, CordzMethodCountEq(Method::kAssignString, 1));
+ }
+}
+
+TEST_P(CordzUpdateTest, AssignInlinedString) {
+ cord() = std::string(Length(TestCordSize::kInlined), '.');
+ EXPECT_THAT(GetCordzInfoForTesting(cord()), Eq(nullptr));
+}
+
TEST_P(CordzUpdateTest, AppendCord) {
Cord src = UnsampledCord(MakeString(TestCordSize::kLarge));
cord().Append(src);
@@ -162,12 +211,6 @@ TEST_P(CordzUpdateTest, MoveAppendCord) {
EXPECT_THAT(cord(), HasValidCordzInfoOf(InitialOr(Method::kAppendCord)));
}
-TEST_P(CordzUpdateTest, PrependCord) {
- Cord src = UnsampledCord(MakeString(TestCordSize::kLarge));
- cord().Prepend(src);
- EXPECT_THAT(cord(), HasValidCordzInfoOf(InitialOr(Method::kPrependCord)));
-}
-
TEST_P(CordzUpdateTest, AppendSmallArray) {
cord().Append(MakeString(TestCordSize::kSmall));
EXPECT_THAT(cord(), HasValidCordzInfoOf(InitialOr(Method::kAppendString)));
@@ -178,6 +221,80 @@ TEST_P(CordzUpdateTest, AppendLargeArray) {
EXPECT_THAT(cord(), HasValidCordzInfoOf(InitialOr(Method::kAppendString)));
}
+TEST_P(CordzStringTest, AppendStringToEmpty) {
+ Cord cord;
+ cord.Append(std::string(Length(GetParam()), '.'));
+ if (Length(GetParam()) > kMaxInline) {
+ EXPECT_THAT(cord, HasValidCordzInfoOf(Method::kAppendString));
+ }
+}
+
+TEST_P(CordzStringTest, AppendStringToInlined) {
+ Cord cord(MakeString(TestCordSize::kInlined));
+ cord.Append(std::string(Length(GetParam()), '.'));
+ if (Length(TestCordSize::kInlined) + Length(GetParam()) > kMaxInline) {
+ EXPECT_THAT(cord, HasValidCordzInfoOf(Method::kAppendString));
+ }
+}
+
+TEST_P(CordzStringTest, AppendStringToCord) {
+ Cord cord(MakeString(TestCordSize::kLarge));
+ cord.Append(std::string(Length(GetParam()), '.'));
+ EXPECT_THAT(cord, HasValidCordzInfoOf(Method::kConstructorString));
+ EXPECT_THAT(cord, CordzMethodCountEq(Method::kAppendString, 1));
+}
+
+TEST(CordzTest, MakeCordFromExternal) {
+ CordzSamplingIntervalHelper sample_every{1};
+ Cord cord = MakeCordFromExternal("Hello world", [](absl::string_view) {});
+ EXPECT_THAT(cord, HasValidCordzInfoOf(Method::kMakeCordFromExternal));
+}
+
+TEST(CordzTest, MakeCordFromEmptyExternal) {
+ CordzSamplingIntervalHelper sample_every{1};
+ Cord cord = MakeCordFromExternal({}, [](absl::string_view) {});
+ EXPECT_THAT(GetCordzInfoForTesting(cord), Eq(nullptr));
+}
+
+TEST_P(CordzUpdateTest, PrependCord) {
+ Cord src = UnsampledCord(MakeString(TestCordSize::kLarge));
+ cord().Prepend(src);
+ EXPECT_THAT(cord(), HasValidCordzInfoOf(InitialOr(Method::kPrependCord)));
+}
+
+TEST_P(CordzUpdateTest, PrependSmallArray) {
+ cord().Prepend(MakeString(TestCordSize::kSmall));
+ EXPECT_THAT(cord(), HasValidCordzInfoOf(InitialOr(Method::kPrependString)));
+}
+
+TEST_P(CordzUpdateTest, PrependLargeArray) {
+ cord().Prepend(MakeString(TestCordSize::kLarge));
+ EXPECT_THAT(cord(), HasValidCordzInfoOf(InitialOr(Method::kPrependString)));
+}
+
+TEST_P(CordzStringTest, PrependStringToEmpty) {
+ Cord cord;
+ cord.Prepend(std::string(Length(GetParam()), '.'));
+ if (Length(GetParam()) > kMaxInline) {
+ EXPECT_THAT(cord, HasValidCordzInfoOf(Method::kPrependString));
+ }
+}
+
+TEST_P(CordzStringTest, PrependStringToInlined) {
+ Cord cord(MakeString(TestCordSize::kInlined));
+ cord.Prepend(std::string(Length(GetParam()), '.'));
+ if (Length(TestCordSize::kInlined) + Length(GetParam()) > kMaxInline) {
+ EXPECT_THAT(cord, HasValidCordzInfoOf(Method::kPrependString));
+ }
+}
+
+TEST_P(CordzStringTest, PrependStringToCord) {
+ Cord cord(MakeString(TestCordSize::kLarge));
+ cord.Prepend(std::string(Length(GetParam()), '.'));
+ EXPECT_THAT(cord, HasValidCordzInfoOf(Method::kConstructorString));
+ EXPECT_THAT(cord, CordzMethodCountEq(Method::kPrependString, 1));
+}
+
TEST(CordzTest, RemovePrefix) {
CordzSamplingIntervalHelper sample_every(1);
Cord cord(MakeString(TestCordSize::kLarge));
diff --git a/absl/strings/internal/cordz_handle.cc b/absl/strings/internal/cordz_handle.cc
index 5297ec8e..a73fefed 100644
--- a/absl/strings/internal/cordz_handle.cc
+++ b/absl/strings/internal/cordz_handle.cc
@@ -68,11 +68,16 @@ CordzHandle::~CordzHandle() {
}
}
+bool CordzHandle::SafeToDelete() const {
+ return is_snapshot_ || queue_->IsEmpty();
+}
+
void CordzHandle::Delete(CordzHandle* handle) {
+ assert(handle);
if (handle) {
handle->ODRCheck();
Queue* const queue = handle->queue_;
- if (!handle->is_snapshot_ && !queue->IsEmpty()) {
+ if (!handle->SafeToDelete()) {
SpinLockHolder lock(&queue->mutex);
CordzHandle* dq_tail = queue->dq_tail.load(std::memory_order_acquire);
if (dq_tail != nullptr) {
diff --git a/absl/strings/internal/cordz_handle.h b/absl/strings/internal/cordz_handle.h
index 93076cfd..5df53c78 100644
--- a/absl/strings/internal/cordz_handle.h
+++ b/absl/strings/internal/cordz_handle.h
@@ -40,9 +40,20 @@ class CordzHandle {
bool is_snapshot() const { return is_snapshot_; }
+ // Returns true if this instance is safe to be deleted because it is either a
+ // snapshot, which is always safe to delete, or not included in the global
+ // delete queue and thus not included in any snapshot.
+ // Callers are responsible for making sure this instance can not be newly
+ // discovered by other threads. For example, CordzInfo instances first de-list
+ // themselves from the global CordzInfo list before determining if they are
+ // safe to be deleted directly.
+ // If SafeToDelete returns false, callers MUST use the Delete() method to
+ // safely queue CordzHandle instances for deletion.
+ bool SafeToDelete() const;
+
// Deletes the provided instance, or puts it on the delete queue to be deleted
// once there are no more sample tokens (snapshot) instances potentially
- // referencing the instance. `handle` may be null.
+ // referencing the instance. `handle` should not be null.
static void Delete(CordzHandle* handle);
// Returns the current entries in the delete queue in LIFO order.
diff --git a/absl/strings/internal/cordz_handle_test.cc b/absl/strings/internal/cordz_handle_test.cc
index c04240e8..4eefd72c 100644
--- a/absl/strings/internal/cordz_handle_test.cc
+++ b/absl/strings/internal/cordz_handle_test.cc
@@ -52,6 +52,7 @@ TEST(CordzHandleTest, CordzHandleCreateDelete) {
bool deleted = false;
auto* handle = new CordzHandleDeleteTracker(&deleted);
EXPECT_FALSE(handle->is_snapshot());
+ EXPECT_TRUE(handle->SafeToDelete());
EXPECT_THAT(DeleteQueue(), SizeIs(0));
CordzHandle::Delete(handle);
@@ -62,6 +63,7 @@ TEST(CordzHandleTest, CordzHandleCreateDelete) {
TEST(CordzHandleTest, CordzSnapshotCreateDelete) {
auto* snapshot = new CordzSnapshot();
EXPECT_TRUE(snapshot->is_snapshot());
+ EXPECT_TRUE(snapshot->SafeToDelete());
EXPECT_THAT(DeleteQueue(), ElementsAre(snapshot));
delete snapshot;
EXPECT_THAT(DeleteQueue(), SizeIs(0));
@@ -71,10 +73,12 @@ TEST(CordzHandleTest, CordzHandleCreateDeleteWithSnapshot) {
bool deleted = false;
auto* snapshot = new CordzSnapshot();
auto* handle = new CordzHandleDeleteTracker(&deleted);
+ EXPECT_FALSE(handle->SafeToDelete());
CordzHandle::Delete(handle);
EXPECT_THAT(DeleteQueue(), ElementsAre(handle, snapshot));
EXPECT_FALSE(deleted);
+ EXPECT_FALSE(handle->SafeToDelete());
delete snapshot;
EXPECT_THAT(DeleteQueue(), SizeIs(0));
@@ -219,8 +223,8 @@ TEST(CordzHandleTest, MultiThreaded) {
if (safe_to_inspect.size() > max_safe_to_inspect) {
max_safe_to_inspect = safe_to_inspect.size();
}
+ CordzHandle::Delete(old_handle);
}
- CordzHandle::Delete(old_handle);
}
// Confirm that the test did *something*. This check will be satisfied as
@@ -234,9 +238,10 @@ TEST(CordzHandleTest, MultiThreaded) {
// Have each thread attempt to clean up everything. Some thread will be
// the last to reach this cleanup code, and it will be guaranteed to clean
// up everything because nothing remains to create new handles.
- for (size_t i = 0; i < handles.size(); i++) {
- CordzHandle* handle = handles[i].exchange(nullptr);
- CordzHandle::Delete(handle);
+ for (auto& h : handles) {
+ if (CordzHandle* handle = h.exchange(nullptr)) {
+ CordzHandle::Delete(handle);
+ }
}
});
}
diff --git a/absl/strings/internal/cordz_info.cc b/absl/strings/internal/cordz_info.cc
index 0461ec46..0f657aaf 100644
--- a/absl/strings/internal/cordz_info.cc
+++ b/absl/strings/internal/cordz_info.cc
@@ -126,13 +126,6 @@ void CordzInfo::Track() {
}
void CordzInfo::Untrack() {
- {
- // TODO(b/117940323): change this to assuming ownership instead once all
- // Cord logic is properly keeping `rep_` in sync with the Cord root rep.
- absl::MutexLock lock(&mutex_);
- rep_ = nullptr;
- }
-
ODRCheck();
{
SpinLockHolder l(&list_->mutex);
@@ -154,6 +147,20 @@ void CordzInfo::Untrack() {
list_->head.store(next, std::memory_order_release);
}
}
+
+ // We can no longer be discovered: perform a fast path check if we are not
+ // listed on any delete queue, so we can directly delete this instance.
+ if (SafeToDelete()) {
+ UnsafeSetCordRep(nullptr);
+ delete this;
+ return;
+ }
+
+ // We are likely part of a snapshot, extend the life of the CordRep
+ {
+ absl::MutexLock lock(&mutex_);
+ if (rep_) CordRep::Ref(rep_);
+ }
CordzHandle::Delete(this);
}
diff --git a/absl/strings/internal/cordz_info.h b/absl/strings/internal/cordz_info.h
index f7682cb3..1fe046ec 100644
--- a/absl/strings/internal/cordz_info.h
+++ b/absl/strings/internal/cordz_info.h
@@ -110,7 +110,7 @@ class ABSL_LOCKABLE CordzInfo : public CordzHandle {
// Asserts that this CordzInfo instance is locked.
void AssertHeld() ABSL_ASSERT_EXCLUSIVE_LOCK(mutex_);
- // Updates the `rep' property of this instance. This methods is invoked by
+ // Updates the `rep` property of this instance. This methods is invoked by
// Cord logic each time the root node of a sampled Cord changes, and before
// the old root reference count is deleted. This guarantees that collection
// code can always safely take a reference on the tracked cord.
@@ -119,6 +119,11 @@ class ABSL_LOCKABLE CordzInfo : public CordzHandle {
// Cord code is in a state where this can be proven true by the compiler.
void SetCordRep(CordRep* rep);
+ // Returns the current `rep` property of this instance with a reference
+ // added, or null if this instance represents a cord that has since been
+ // deleted or untracked.
+ CordRep* RefCordRep() const ABSL_LOCKS_EXCLUDED(mutex_);
+
// Returns the current value of `rep_` for testing purposes only.
CordRep* GetCordRepForTesting() const ABSL_NO_THREAD_SAFETY_ANALYSIS {
return rep_;
@@ -148,6 +153,9 @@ class ABSL_LOCKABLE CordzInfo : public CordzHandle {
}
private:
+ using SpinLock = absl::base_internal::SpinLock;
+ using SpinLockHolder = ::absl::base_internal::SpinLockHolder;
+
// Global cordz info list. CordzInfo stores a pointer to the global list
// instance to harden against ODR violations.
struct List {
@@ -155,7 +163,7 @@ class ABSL_LOCKABLE CordzInfo : public CordzHandle {
: mutex(absl::kConstInit,
absl::base_internal::SCHEDULE_COOPERATIVE_AND_KERNEL) {}
- absl::base_internal::SpinLock mutex;
+ SpinLock mutex;
std::atomic<CordzInfo*> head ABSL_GUARDED_BY(mutex){nullptr};
};
@@ -165,6 +173,9 @@ class ABSL_LOCKABLE CordzInfo : public CordzHandle {
MethodIdentifier method);
~CordzInfo() override;
+ // Sets `rep_` without holding a lock.
+ void UnsafeSetCordRep(CordRep* rep) ABSL_NO_THREAD_SAFETY_ANALYSIS;
+
void Track();
// Returns the parent method from `src`, which is either `parent_method_` or
@@ -244,6 +255,13 @@ inline void CordzInfo::SetCordRep(CordRep* rep) {
}
}
+inline void CordzInfo::UnsafeSetCordRep(CordRep* rep) { rep_ = rep; }
+
+inline CordRep* CordzInfo::RefCordRep() const ABSL_LOCKS_EXCLUDED(mutex_) {
+ MutexLock lock(&mutex_);
+ return rep_ ? CordRep::Ref(rep_) : nullptr;
+}
+
} // namespace cord_internal
ABSL_NAMESPACE_END
} // namespace absl
diff --git a/absl/strings/internal/cordz_info_test.cc b/absl/strings/internal/cordz_info_test.cc
index 5eaf4b9c..5bb23e47 100644
--- a/absl/strings/internal/cordz_info_test.cc
+++ b/absl/strings/internal/cordz_info_test.cc
@@ -38,6 +38,7 @@ using ::testing::ElementsAre;
using ::testing::Eq;
using ::testing::HasSubstr;
using ::testing::Ne;
+using ::testing::SizeIs;
// Used test values
auto constexpr kUnknownMethod = CordzUpdateTracker::kUnknown;
@@ -78,10 +79,19 @@ TEST(CordzInfoTest, UntrackCord) {
CordzInfo::TrackCord(data.data, kTrackCordMethod);
CordzInfo* info = data.data.cordz_info();
+ info->Untrack();
+ EXPECT_THAT(DeleteQueue(), SizeIs(0));
+}
+
+TEST(CordzInfoTest, UntrackCordWithSnapshot) {
+ TestCordData data;
+ CordzInfo::TrackCord(data.data, kTrackCordMethod);
+ CordzInfo* info = data.data.cordz_info();
+
CordzSnapshot snapshot;
info->Untrack();
EXPECT_THAT(CordzInfo::Head(CordzSnapshot()), Eq(nullptr));
- EXPECT_THAT(info->GetCordRepForTesting(), Eq(nullptr));
+ EXPECT_THAT(info->GetCordRepForTesting(), Eq(data.rep.rep));
EXPECT_THAT(DeleteQueue(), ElementsAre(info, &snapshot));
}
@@ -113,6 +123,18 @@ TEST(CordzInfoTest, SetCordRepNullUntracksCordOnUnlock) {
EXPECT_THAT(CordzInfo::Head(CordzSnapshot()), Eq(nullptr));
}
+TEST(CordzInfoTest, RefCordRep) {
+ TestCordData data;
+ CordzInfo::TrackCord(data.data, kTrackCordMethod);
+ CordzInfo* info = data.data.cordz_info();
+
+ size_t refcount = data.rep.rep->refcount.Get();
+ EXPECT_THAT(info->RefCordRep(), Eq(data.rep.rep));
+ EXPECT_THAT(data.rep.rep->refcount.Get(), Eq(refcount + 1));
+ CordRep::Unref(data.rep.rep);
+ info->Untrack();
+}
+
#if GTEST_HAS_DEATH_TEST
TEST(CordzInfoTest, SetCordRepRequiresMutex) {
diff --git a/absl/strings/internal/cordz_update_tracker.h b/absl/strings/internal/cordz_update_tracker.h
index 741950b2..d5b14067 100644
--- a/absl/strings/internal/cordz_update_tracker.h
+++ b/absl/strings/internal/cordz_update_tracker.h
@@ -47,8 +47,10 @@ class CordzUpdateTracker {
kClear,
kConstructorCord,
kConstructorString,
+ kCordReader,
kFlatten,
kGetAppendRegion,
+ kMakeCordFromExternal,
kMoveAppendCord,
kMoveAssignCord,
kMovePrependCord,
diff --git a/absl/strings/internal/cordz_update_tracker_test.cc b/absl/strings/internal/cordz_update_tracker_test.cc
index eda662f2..fcd17df7 100644
--- a/absl/strings/internal/cordz_update_tracker_test.cc
+++ b/absl/strings/internal/cordz_update_tracker_test.cc
@@ -45,8 +45,10 @@ Methods AllMethods() {
Method::kClear,
Method::kConstructorCord,
Method::kConstructorString,
+ Method::kCordReader,
Method::kFlatten,
Method::kGetAppendRegion,
+ Method::kMakeCordFromExternal,
Method::kMoveAppendCord,
Method::kMoveAssignCord,
Method::kMovePrependCord,
generated by cgit v1.2.3 (git 2.39.1) at 2024-06-08 09:49:25 +0000