Skip to content

A
abseil-cpp
Commit 5dd24072 by Abseil Team Committed by Derek Mauro
Options

Export of internal Abseil changes 

--
60b8e77be4bab1bbd3b4c3b70054879229634511 by Derek Mauro <dmauro@google.com>:

Use _MSVC_LANG for some C++ dialect checks since MSVC doesn't
set __cplusplus accurately by default.

https://devblogs.microsoft.com/cppblog/msvc-now-correctly-reports-__cplusplus/

See GitHub #722.

PiperOrigin-RevId: 371362181

--
5d736accdff04db0e722f377c0d79f2d3ed53263 by Martijn Vels <mvels@google.com>:

Fix the estimated memory size for CordRepExternal

PiperOrigin-RevId: 371350380

--
eaaa1d8a167aeca67a2aa3a098a2b61a9d72172f by Martijn Vels <mvels@google.com>:

Remove flakes by not enforcing re-allocated pointers do never match original

Tests that do multiple updates could end up with the original allocated pointer on a 2nd resize, so the 'EqIfPrivate' should not assume that if we do 'not' have the capacity that all following relocations will never match the original. We only care about 'pointer unchanged if private and there is capacity', trying to establish 'pointer changed at some point due to re-allocation; is pointless.

PiperOrigin-RevId: 371338965

--
d1837bee6bade1902b095c1cbf64231668bb84c5 by Martijn Vels <mvels@google.com>:

Undo inline of small data copy in cord

This leads to a performance regression as the code is not inlined (absent hard FDO inputs), and there are no suitable tail call options.

PiperOrigin-RevId: 371332332

--
06dc64b833069efc7d18b11df607c8c22be690da by Martijn Vels <mvels@google.com>:

Add final instrumentation for Cordz and remove 'old' cordz logic.

This change instruments the last cord function for cordz. It removes the 'old' functions: set_tree, replace_tree, UpdateCordzStatistics and RecordMetrics.

PiperOrigin-RevId: 371219909

--
a5e0be538579c603052feec03e6d9910c43ea787 by Martijn Vels <mvels@google.com>:

Extend the life of CordRep* if inside a snapshot

If a snapshot (potentially) includes the current CordzInfo, we need to extent the lifetime of the CordRep*, as the snapshot 'point in time' observation of the cord should ideally be preserved.

PiperOrigin-RevId: 371146151

--
74d77a89774cd6c8ecdeebee0193b294a39383d6 by Martijn Vels <mvels@google.com>:

Instrument std::string consuming methods: ctor, operator=, Append and Prepend

This change moves the 'steal into CordRep' logic into a separate function so we can use it directly in the ctor, operator assign and append and prepend, allowing Cordz instrumentation with the proper method attributes.

The assign operator is implemented in AssignLargeString leaving the dispatch inlined in cord.h (which as a side effects also allows clean tail calls in the AssignLargeString method)

PiperOrigin-RevId: 371094756

--
b39effc45266b7ce2e7f96caa3b16cb6e3acc2dd by Martijn Vels <mvels@google.com>:

Add Cordz instrumentation to CordReader

PiperOrigin-RevId: 370990181
GitOrigin-RevId: 60b8e77be4bab1bbd3b4c3b70054879229634511
Change-Id: I96af62e6f1a643e8b1228ae01e6c84e33706bb05
parent f14d5f4a
Showing with 352 additions and 154 deletions
absl/memory/memory.h
...@@ -420,7 +420,7 @@ struct pointer_traits<T*> { ...@@ -420,7 +420,7 @@ struct pointer_traits<T*> {
// //
// A C++11 compatible implementation of C++17's std::allocator_traits. // 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; using std::allocator_traits;
#else // __cplusplus >= 201703L #else // __cplusplus >= 201703L
template <typename Alloc> template <typename Alloc>
......
absl/meta/type_traits.h
...@@ -634,7 +634,7 @@ using underlying_type_t = typename std::underlying_type<T>::type; ...@@ -634,7 +634,7 @@ using underlying_type_t = typename std::underlying_type<T>::type;
namespace type_traits_internal { 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) // std::result_of is deprecated (C++17) or removed (C++20)
template<typename> struct result_of; template<typename> struct result_of;
template<typename F, typename... Args> template<typename F, typename... Args>
......
absl/strings/BUILD.bazel
...@@ -529,6 +529,7 @@ cc_library( ...@@ -529,6 +529,7 @@ cc_library(
":cord", ":cord",
":cord_internal", ":cord_internal",
":strings", ":strings",
"//absl/base:config",
], ],
) )
......
absl/strings/CMakeLists.txt
...@@ -830,6 +830,7 @@ absl_cc_library( ...@@ -830,6 +830,7 @@ absl_cc_library(
COPTS COPTS
${ABSL_TEST_COPTS} ${ABSL_TEST_COPTS}
DEPS DEPS
absl::config
absl::cord absl::cord
absl::cord_internal absl::cord_internal
absl::strings absl::strings
......
absl/strings/cord.h
...@@ -678,6 +678,10 @@ class Cord { ...@@ -678,6 +678,10 @@ class Cord {
using InlineData = cord_internal::InlineData; using InlineData = cord_internal::InlineData;
using MethodIdentifier = CordzUpdateTracker::MethodIdentifier; 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 class CordTestPeer;
friend bool operator==(const Cord& lhs, const Cord& rhs); friend bool operator==(const Cord& lhs, const Cord& rhs);
friend bool operator==(const Cord& lhs, absl::string_view rhs); friend bool operator==(const Cord& lhs, absl::string_view rhs);
...@@ -692,10 +696,6 @@ class Cord { ...@@ -692,10 +696,6 @@ class Cord {
// called by Flatten() when the cord was not already flat. // called by Flatten() when the cord was not already flat.
absl::string_view FlattenSlowPath(); 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 // Actual cord contents are hidden inside the following simple
// class so that we can isolate the bulk of cord.cc from changes // class so that we can isolate the bulk of cord.cc from changes
// to the representation. // to the representation.
...@@ -725,11 +725,6 @@ class Cord { ...@@ -725,11 +725,6 @@ class Cord {
// Returns nullptr if holding bytes // Returns nullptr if holding bytes
absl::cord_internal::CordRep* tree() const; absl::cord_internal::CordRep* tree() const;
absl::cord_internal::CordRep* as_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 // Returns non-null iff was holding a pointer
absl::cord_internal::CordRep* clear(); absl::cord_internal::CordRep* clear();
// Converts to pointer if necessary. // Converts to pointer if necessary.
...@@ -831,11 +826,6 @@ class Cord { ...@@ -831,11 +826,6 @@ class Cord {
// Resets the current cordz_info to null / empty. // Resets the current cordz_info to null / empty.
void clear_cordz_info() { data_.clear_cordz_info(); } 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: private:
friend class Cord; friend class Cord;
...@@ -892,6 +882,10 @@ class Cord { ...@@ -892,6 +882,10 @@ class Cord {
template <typename C> template <typename C>
void AppendImpl(C&& src); 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(). // Helper for AbslHashValue().
template <typename H> template <typename H>
H HashFragmented(H hash_state) const { H HashFragmented(H hash_state) const {
...@@ -994,8 +988,11 @@ inline CordRep* NewExternalRep(absl::string_view data, ...@@ -994,8 +988,11 @@ inline CordRep* NewExternalRep(absl::string_view data,
template <typename Releaser> template <typename Releaser>
Cord MakeCordFromExternal(absl::string_view data, Releaser&& releaser) { Cord MakeCordFromExternal(absl::string_view data, Releaser&& releaser) {
Cord cord; Cord cord;
cord.contents_.set_tree(::absl::cord_internal::NewExternalRep( if (auto* rep = ::absl::cord_internal::NewExternalRep(
data, std::forward<Releaser>(releaser))); data, std::forward<Releaser>(releaser))) {
cord.contents_.EmplaceTree(rep,
Cord::MethodIdentifier::kMakeCordFromExternal);
}
return cord; return cord;
} }
...@@ -1119,36 +1116,6 @@ inline void Cord::InlineRep::CommitTree(const CordRep* old_rep, CordRep* rep, ...@@ -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() { inline absl::cord_internal::CordRep* Cord::InlineRep::clear() {
if (is_tree()) { if (is_tree()) {
CordzInfo::MaybeUntrackCord(cordz_info()); CordzInfo::MaybeUntrackCord(cordz_info());
...@@ -1165,13 +1132,11 @@ inline void Cord::InlineRep::CopyToArray(char* dst) const { ...@@ -1165,13 +1132,11 @@ inline void Cord::InlineRep::CopyToArray(char* dst) const {
cord_internal::SmallMemmove(dst, data_.as_chars(), n); 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 {} constexpr inline Cord::Cord() noexcept {}
inline Cord::Cord(absl::string_view src)
: Cord(src, CordzUpdateTracker::kConstructorString) {}
template <typename T> template <typename T>
constexpr Cord::Cord(strings_internal::StringConstant<T>) constexpr Cord::Cord(strings_internal::StringConstant<T>)
: contents_(strings_internal::StringConstant<T>::value.size() <= : contents_(strings_internal::StringConstant<T>::value.size() <=
...@@ -1187,6 +1152,15 @@ inline Cord& Cord::operator=(const Cord& x) { ...@@ -1187,6 +1152,15 @@ inline Cord& Cord::operator=(const Cord& x) {
return *this; 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(const Cord& src) : contents_(src.contents_) {}
inline Cord::Cord(Cord&& src) noexcept : contents_(std::move(src.contents_)) {} inline Cord::Cord(Cord&& src) noexcept : contents_(std::move(src.contents_)) {}
...@@ -1201,7 +1175,6 @@ inline Cord& Cord::operator=(Cord&& x) noexcept { ...@@ -1201,7 +1175,6 @@ inline Cord& Cord::operator=(Cord&& x) noexcept {
} }
extern template Cord::Cord(std::string&& src); extern template Cord::Cord(std::string&& src);
extern template Cord& Cord::operator=(std::string&& src);
inline size_t Cord::size() const { inline size_t Cord::size() const {
// Length is 1st field in str.rep_ // Length is 1st field in str.rep_
......
absl/strings/cord_test.cc
...@@ -190,14 +190,15 @@ class CordTestPeer { ...@@ -190,14 +190,15 @@ class CordTestPeer {
} }
static Cord MakeSubstring(Cord src, size_t offset, size_t length) { static Cord MakeSubstring(Cord src, size_t offset, size_t length) {
Cord cord = src; ABSL_RAW_CHECK(src.contents_.is_tree(), "Can not be inlined");
ABSL_RAW_CHECK(cord.contents_.is_tree(), "Can not be inlined"); Cord cord;
auto* rep = new cord_internal::CordRepSubstring; auto* rep = new cord_internal::CordRepSubstring;
rep->tag = cord_internal::SUBSTRING; rep->tag = cord_internal::SUBSTRING;
rep->child = cord.contents_.tree(); rep->child = cord_internal::CordRep::Ref(src.contents_.tree());
rep->start = offset; rep->start = offset;
rep->length = length; rep->length = length;
cord.contents_.replace_tree(rep); cord.contents_.EmplaceTree(rep,
cord_internal::CordzUpdateTracker::kSubCord);
return cord; return cord;
} }
}; };
......
absl/strings/cord_test_helpers.h
...@@ -19,7 +19,9 @@ ...@@ -19,7 +19,9 @@
#include <cstdint> #include <cstdint>
#include <iostream> #include <iostream>
#include <string>
#include "absl/base/config.h"
#include "absl/strings/cord.h" #include "absl/strings/cord.h"
#include "absl/strings/internal/cord_internal.h" #include "absl/strings/internal/cord_internal.h"
#include "absl/strings/string_view.h" #include "absl/strings/string_view.h"
...@@ -29,10 +31,27 @@ ABSL_NAMESPACE_BEGIN ...@@ -29,10 +31,27 @@ ABSL_NAMESPACE_BEGIN
// Cord sizes relevant for testing // Cord sizes relevant for testing
enum class TestCordSize { enum class TestCordSize {
// An empty value
kEmpty = 0, kEmpty = 0,
// An inlined string value
kInlined = cord_internal::kMaxInline / 2 + 1, 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, 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, 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 kLarge = cord_internal::kMaxFlatLength * 4
}; };
...@@ -45,6 +64,10 @@ inline absl::string_view ToString(TestCordSize size) { ...@@ -45,6 +64,10 @@ inline absl::string_view ToString(TestCordSize size) {
return "Inlined"; return "Inlined";
case TestCordSize::kSmall: case TestCordSize::kSmall:
return "Small"; return "Small";
case TestCordSize::kStringSso1:
return "StringSso1";
case TestCordSize::kStringSso2:
return "StringSso2";
case TestCordSize::kMedium: case TestCordSize::kMedium:
return "Medium"; return "Medium";
case TestCordSize::kLarge: case TestCordSize::kLarge:
......
absl/strings/cordz_test.cc
...@@ -34,6 +34,7 @@ ...@@ -34,6 +34,7 @@
#ifdef ABSL_INTERNAL_CORDZ_ENABLED #ifdef ABSL_INTERNAL_CORDZ_ENABLED
using testing::Eq; using testing::Eq;
using testing::AnyOf;
namespace absl { namespace absl {
ABSL_NAMESPACE_BEGIN ABSL_NAMESPACE_BEGIN
...@@ -84,24 +85,50 @@ class CordzUpdateTest : public testing::TestWithParam<TestCordSize> { ...@@ -84,24 +85,50 @@ class CordzUpdateTest : public testing::TestWithParam<TestCordSize> {
Cord cord_{MakeString(GetParam())}; 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, INSTANTIATE_TEST_SUITE_P(WithParam, CordzUpdateTest,
testing::Values(TestCordSize::kEmpty, testing::Values(TestCordSize::kEmpty,
TestCordSize::kInlined, TestCordSize::kInlined,
TestCordSize::kLarge), TestCordSize::kLarge),
TestParamToString); 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}; CordzSamplingIntervalHelper sample_every{1};
Cord cord(MakeString(TestCordSize::kSmall)); Cord cord(MakeString(TestCordSize::kSmall));
EXPECT_THAT(cord, HasValidCordzInfoOf(Method::kConstructorString)); EXPECT_THAT(cord, HasValidCordzInfoOf(Method::kConstructorString));
} }
TEST(CordzTest, ConstructLargeString) { TEST(CordzTest, ConstructLargeArray) {
CordzSamplingIntervalHelper sample_every{1}; CordzSamplingIntervalHelper sample_every{1};
Cord cord(MakeString(TestCordSize::kLarge)); Cord cord(MakeString(TestCordSize::kLarge));
EXPECT_THAT(cord, HasValidCordzInfoOf(Method::kConstructorString)); 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) { TEST(CordzTest, CopyConstruct) {
CordzSamplingIntervalHelper sample_every{1}; CordzSamplingIntervalHelper sample_every{1};
Cord src = UnsampledCord(MakeString(TestCordSize::kLarge)); Cord src = UnsampledCord(MakeString(TestCordSize::kLarge));
...@@ -151,6 +178,28 @@ TEST_P(CordzUpdateTest, AssignInlinedArray) { ...@@ -151,6 +178,28 @@ TEST_P(CordzUpdateTest, AssignInlinedArray) {
EXPECT_THAT(GetCordzInfoForTesting(cord()), Eq(nullptr)); 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) { TEST_P(CordzUpdateTest, AppendCord) {
Cord src = UnsampledCord(MakeString(TestCordSize::kLarge)); Cord src = UnsampledCord(MakeString(TestCordSize::kLarge));
cord().Append(src); cord().Append(src);
...@@ -162,12 +211,6 @@ TEST_P(CordzUpdateTest, MoveAppendCord) { ...@@ -162,12 +211,6 @@ TEST_P(CordzUpdateTest, MoveAppendCord) {
EXPECT_THAT(cord(), HasValidCordzInfoOf(InitialOr(Method::kAppendCord))); 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) { TEST_P(CordzUpdateTest, AppendSmallArray) {
cord().Append(MakeString(TestCordSize::kSmall)); cord().Append(MakeString(TestCordSize::kSmall));
EXPECT_THAT(cord(), HasValidCordzInfoOf(InitialOr(Method::kAppendString))); EXPECT_THAT(cord(), HasValidCordzInfoOf(InitialOr(Method::kAppendString)));
...@@ -178,6 +221,80 @@ TEST_P(CordzUpdateTest, AppendLargeArray) { ...@@ -178,6 +221,80 @@ TEST_P(CordzUpdateTest, AppendLargeArray) {
EXPECT_THAT(cord(), HasValidCordzInfoOf(InitialOr(Method::kAppendString))); 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) { TEST(CordzTest, RemovePrefix) {
CordzSamplingIntervalHelper sample_every(1); CordzSamplingIntervalHelper sample_every(1);
Cord cord(MakeString(TestCordSize::kLarge)); Cord cord(MakeString(TestCordSize::kLarge));
......
absl/strings/internal/cordz_handle.cc
...@@ -68,11 +68,16 @@ CordzHandle::~CordzHandle() { ...@@ -68,11 +68,16 @@ CordzHandle::~CordzHandle() {
} }
} }
bool CordzHandle::SafeToDelete() const {
return is_snapshot_ || queue_->IsEmpty();
}
void CordzHandle::Delete(CordzHandle* handle) { void CordzHandle::Delete(CordzHandle* handle) {
assert(handle);
if (handle) { if (handle) {
handle->ODRCheck(); handle->ODRCheck();
Queue* const queue = handle->queue_; Queue* const queue = handle->queue_;
if (!handle->is_snapshot_ && !queue->IsEmpty()) { if (!handle->SafeToDelete()) {
SpinLockHolder lock(&queue->mutex); SpinLockHolder lock(&queue->mutex);
CordzHandle* dq_tail = queue->dq_tail.load(std::memory_order_acquire); CordzHandle* dq_tail = queue->dq_tail.load(std::memory_order_acquire);
if (dq_tail != nullptr) { if (dq_tail != nullptr) {
......
absl/strings/internal/cordz_handle.h
...@@ -40,9 +40,20 @@ class CordzHandle { ...@@ -40,9 +40,20 @@ class CordzHandle {
bool is_snapshot() const { return is_snapshot_; } 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 // Deletes the provided instance, or puts it on the delete queue to be deleted
// once there are no more sample tokens (snapshot) instances potentially // 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); static void Delete(CordzHandle* handle);
// Returns the current entries in the delete queue in LIFO order. // Returns the current entries in the delete queue in LIFO order.
......
absl/strings/internal/cordz_handle_test.cc
...@@ -52,6 +52,7 @@ TEST(CordzHandleTest, CordzHandleCreateDelete) { ...@@ -52,6 +52,7 @@ TEST(CordzHandleTest, CordzHandleCreateDelete) {
bool deleted = false; bool deleted = false;
auto* handle = new CordzHandleDeleteTracker(&deleted); auto* handle = new CordzHandleDeleteTracker(&deleted);
EXPECT_FALSE(handle->is_snapshot()); EXPECT_FALSE(handle->is_snapshot());
EXPECT_TRUE(handle->SafeToDelete());
EXPECT_THAT(DeleteQueue(), SizeIs(0)); EXPECT_THAT(DeleteQueue(), SizeIs(0));
CordzHandle::Delete(handle); CordzHandle::Delete(handle);
...@@ -62,6 +63,7 @@ TEST(CordzHandleTest, CordzHandleCreateDelete) { ...@@ -62,6 +63,7 @@ TEST(CordzHandleTest, CordzHandleCreateDelete) {
TEST(CordzHandleTest, CordzSnapshotCreateDelete) { TEST(CordzHandleTest, CordzSnapshotCreateDelete) {
auto* snapshot = new CordzSnapshot(); auto* snapshot = new CordzSnapshot();
EXPECT_TRUE(snapshot->is_snapshot()); EXPECT_TRUE(snapshot->is_snapshot());
EXPECT_TRUE(snapshot->SafeToDelete());
EXPECT_THAT(DeleteQueue(), ElementsAre(snapshot)); EXPECT_THAT(DeleteQueue(), ElementsAre(snapshot));
delete snapshot; delete snapshot;
EXPECT_THAT(DeleteQueue(), SizeIs(0)); EXPECT_THAT(DeleteQueue(), SizeIs(0));
...@@ -71,10 +73,12 @@ TEST(CordzHandleTest, CordzHandleCreateDeleteWithSnapshot) { ...@@ -71,10 +73,12 @@ TEST(CordzHandleTest, CordzHandleCreateDeleteWithSnapshot) {
bool deleted = false; bool deleted = false;
auto* snapshot = new CordzSnapshot(); auto* snapshot = new CordzSnapshot();
auto* handle = new CordzHandleDeleteTracker(&deleted); auto* handle = new CordzHandleDeleteTracker(&deleted);
EXPECT_FALSE(handle->SafeToDelete());
CordzHandle::Delete(handle); CordzHandle::Delete(handle);
EXPECT_THAT(DeleteQueue(), ElementsAre(handle, snapshot)); EXPECT_THAT(DeleteQueue(), ElementsAre(handle, snapshot));
EXPECT_FALSE(deleted); EXPECT_FALSE(deleted);
EXPECT_FALSE(handle->SafeToDelete());
delete snapshot; delete snapshot;
EXPECT_THAT(DeleteQueue(), SizeIs(0)); EXPECT_THAT(DeleteQueue(), SizeIs(0));
...@@ -219,8 +223,8 @@ TEST(CordzHandleTest, MultiThreaded) { ...@@ -219,8 +223,8 @@ TEST(CordzHandleTest, MultiThreaded) {
if (safe_to_inspect.size() > max_safe_to_inspect) { if (safe_to_inspect.size() > max_safe_to_inspect) {
max_safe_to_inspect = safe_to_inspect.size(); 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 // Confirm that the test did *something*. This check will be satisfied as
...@@ -234,9 +238,10 @@ TEST(CordzHandleTest, MultiThreaded) { ...@@ -234,9 +238,10 @@ TEST(CordzHandleTest, MultiThreaded) {
// Have each thread attempt to clean up everything. Some thread will be // 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 // the last to reach this cleanup code, and it will be guaranteed to clean
// up everything because nothing remains to create new handles. // up everything because nothing remains to create new handles.
for (size_t i = 0; i < handles.size(); i++) { for (auto& h : handles) {
CordzHandle* handle = handles[i].exchange(nullptr); if (CordzHandle* handle = h.exchange(nullptr)) {
CordzHandle::Delete(handle); CordzHandle::Delete(handle);
}
} }
}); });
} }
......
absl/strings/internal/cordz_info.cc
...@@ -126,13 +126,6 @@ void CordzInfo::Track() { ...@@ -126,13 +126,6 @@ void CordzInfo::Track() {
} }
void CordzInfo::Untrack() { 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(); ODRCheck();
{ {
SpinLockHolder l(&list_->mutex); SpinLockHolder l(&list_->mutex);
...@@ -154,6 +147,20 @@ void CordzInfo::Untrack() { ...@@ -154,6 +147,20 @@ void CordzInfo::Untrack() {
list_->head.store(next, std::memory_order_release); 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); CordzHandle::Delete(this);
} }
......
absl/strings/internal/cordz_info.h
...@@ -110,7 +110,7 @@ class ABSL_LOCKABLE CordzInfo : public CordzHandle { ...@@ -110,7 +110,7 @@ class ABSL_LOCKABLE CordzInfo : public CordzHandle {
// Asserts that this CordzInfo instance is locked. // Asserts that this CordzInfo instance is locked.
void AssertHeld() ABSL_ASSERT_EXCLUSIVE_LOCK(mutex_); 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 // 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 // the old root reference count is deleted. This guarantees that collection
// code can always safely take a reference on the tracked cord. // code can always safely take a reference on the tracked cord.
...@@ -119,6 +119,11 @@ class ABSL_LOCKABLE CordzInfo : public CordzHandle { ...@@ -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. // Cord code is in a state where this can be proven true by the compiler.
void SetCordRep(CordRep* rep); 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. // Returns the current value of `rep_` for testing purposes only.
CordRep* GetCordRepForTesting() const ABSL_NO_THREAD_SAFETY_ANALYSIS { CordRep* GetCordRepForTesting() const ABSL_NO_THREAD_SAFETY_ANALYSIS {
return rep_; return rep_;
...@@ -148,6 +153,9 @@ class ABSL_LOCKABLE CordzInfo : public CordzHandle { ...@@ -148,6 +153,9 @@ class ABSL_LOCKABLE CordzInfo : public CordzHandle {
} }
private: 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 // Global cordz info list. CordzInfo stores a pointer to the global list
// instance to harden against ODR violations. // instance to harden against ODR violations.
struct List { struct List {
...@@ -155,7 +163,7 @@ class ABSL_LOCKABLE CordzInfo : public CordzHandle { ...@@ -155,7 +163,7 @@ class ABSL_LOCKABLE CordzInfo : public CordzHandle {
: mutex(absl::kConstInit, : mutex(absl::kConstInit,
absl::base_internal::SCHEDULE_COOPERATIVE_AND_KERNEL) {} absl::base_internal::SCHEDULE_COOPERATIVE_AND_KERNEL) {}
absl::base_internal::SpinLock mutex; SpinLock mutex;
std::atomic<CordzInfo*> head ABSL_GUARDED_BY(mutex){nullptr}; std::atomic<CordzInfo*> head ABSL_GUARDED_BY(mutex){nullptr};
}; };
...@@ -165,6 +173,9 @@ class ABSL_LOCKABLE CordzInfo : public CordzHandle { ...@@ -165,6 +173,9 @@ class ABSL_LOCKABLE CordzInfo : public CordzHandle {
MethodIdentifier method); MethodIdentifier method);
~CordzInfo() override; ~CordzInfo() override;
// Sets `rep_` without holding a lock.
void UnsafeSetCordRep(CordRep* rep) ABSL_NO_THREAD_SAFETY_ANALYSIS;
void Track(); void Track();
// Returns the parent method from `src`, which is either `parent_method_` or // Returns the parent method from `src`, which is either `parent_method_` or
...@@ -244,6 +255,13 @@ inline void CordzInfo::SetCordRep(CordRep* rep) { ...@@ -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 } // namespace cord_internal
ABSL_NAMESPACE_END ABSL_NAMESPACE_END
} // namespace absl } // namespace absl
......
absl/strings/internal/cordz_info_test.cc
...@@ -38,6 +38,7 @@ using ::testing::ElementsAre; ...@@ -38,6 +38,7 @@ using ::testing::ElementsAre;
using ::testing::Eq; using ::testing::Eq;
using ::testing::HasSubstr; using ::testing::HasSubstr;
using ::testing::Ne; using ::testing::Ne;
using ::testing::SizeIs;
// Used test values // Used test values
auto constexpr kUnknownMethod = CordzUpdateTracker::kUnknown; auto constexpr kUnknownMethod = CordzUpdateTracker::kUnknown;
...@@ -78,10 +79,19 @@ TEST(CordzInfoTest, UntrackCord) { ...@@ -78,10 +79,19 @@ TEST(CordzInfoTest, UntrackCord) {
CordzInfo::TrackCord(data.data, kTrackCordMethod); CordzInfo::TrackCord(data.data, kTrackCordMethod);
CordzInfo* info = data.data.cordz_info(); 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; CordzSnapshot snapshot;
info->Untrack(); info->Untrack();
EXPECT_THAT(CordzInfo::Head(CordzSnapshot()), Eq(nullptr)); 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)); EXPECT_THAT(DeleteQueue(), ElementsAre(info, &snapshot));
} }
...@@ -113,6 +123,18 @@ TEST(CordzInfoTest, SetCordRepNullUntracksCordOnUnlock) { ...@@ -113,6 +123,18 @@ TEST(CordzInfoTest, SetCordRepNullUntracksCordOnUnlock) {
EXPECT_THAT(CordzInfo::Head(CordzSnapshot()), Eq(nullptr)); 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 #if GTEST_HAS_DEATH_TEST
TEST(CordzInfoTest, SetCordRepRequiresMutex) { TEST(CordzInfoTest, SetCordRepRequiresMutex) {
......
absl/strings/internal/cordz_update_tracker.h
...@@ -47,8 +47,10 @@ class CordzUpdateTracker { ...@@ -47,8 +47,10 @@ class CordzUpdateTracker {
kClear, kClear,
kConstructorCord, kConstructorCord,
kConstructorString, kConstructorString,
kCordReader,
kFlatten, kFlatten,
kGetAppendRegion, kGetAppendRegion,
kMakeCordFromExternal,
kMoveAppendCord, kMoveAppendCord,
kMoveAssignCord, kMoveAssignCord,
kMovePrependCord, kMovePrependCord,
......
absl/strings/internal/cordz_update_tracker_test.cc
...@@ -45,8 +45,10 @@ Methods AllMethods() { ...@@ -45,8 +45,10 @@ Methods AllMethods() {
Method::kClear, Method::kClear,
Method::kConstructorCord, Method::kConstructorCord,
Method::kConstructorString, Method::kConstructorString,
Method::kCordReader,
Method::kFlatten, Method::kFlatten,
Method::kGetAppendRegion, Method::kGetAppendRegion,
Method::kMakeCordFromExternal,
Method::kMoveAppendCord, Method::kMoveAppendCord,
Method::kMoveAssignCord, Method::kMoveAssignCord,
Method::kMovePrependCord, Method::kMovePrependCord,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment