diff options
| -rw-r--r-- | CMake/AbseilDll.cmake | 1 | ||||
| -rw-r--r-- | absl/numeric/int128.h | 10 | ||||
| -rw-r--r-- | absl/strings/BUILD.bazel | 29 | ||||
| -rw-r--r-- | absl/strings/CMakeLists.txt | 34 | ||||
| -rw-r--r-- | absl/strings/cord.cc | 49 | ||||
| -rw-r--r-- | absl/strings/cord.h | 28 | ||||
| -rw-r--r-- | absl/strings/internal/cord_internal.h | 4 | ||||
| -rw-r--r-- | absl/strings/internal/cordz_info.cc | 29 | ||||
| -rw-r--r-- | absl/strings/internal/cordz_info.h | 85 | ||||
| -rw-r--r-- | absl/strings/internal/cordz_info_test.cc | 109 | ||||
| -rw-r--r-- | absl/strings/internal/cordz_sample_token_test.cc | 74 | ||||
| -rw-r--r-- | absl/strings/internal/cordz_update_scope.h | 65 | ||||
| -rw-r--r-- | absl/strings/internal/cordz_update_scope_test.cc | 66 | ||||
| -rw-r--r-- | absl/strings/internal/cordz_update_tracker.h | 9 |
14 files changed, 427 insertions, 165 deletions
diff --git a/CMake/AbseilDll.cmake b/CMake/AbseilDll.cmake index 2f2fa13d..8ee4120f 100644 --- a/CMake/AbseilDll.cmake +++ b/CMake/AbseilDll.cmake @@ -216,6 +216,7 @@ set(ABSL_INTERNAL_DLL_FILES "strings/internal/cordz_sample_token.cc" "strings/internal/cordz_sample_token.h" "strings/internal/cordz_statistics.h" + "strings/internal/cordz_update_scope.h" "strings/internal/cordz_update_tracker.h" "strings/internal/stl_type_traits.h" "strings/internal/string_constant.h" diff --git a/absl/numeric/int128.h b/absl/numeric/int128.h index 0dd814a8..235361a8 100644 --- a/absl/numeric/int128.h +++ b/absl/numeric/int128.h @@ -817,27 +817,27 @@ inline uint128 operator-(uint128 val) { return MakeUint128(hi, lo); } -inline bool operator!(uint128 val) { +constexpr inline bool operator!(uint128 val) { return !Uint128High64(val) && !Uint128Low64(val); } // Logical operators. -inline uint128 operator~(uint128 val) { +constexpr inline uint128 operator~(uint128 val) { return MakeUint128(~Uint128High64(val), ~Uint128Low64(val)); } -inline uint128 operator|(uint128 lhs, uint128 rhs) { +constexpr inline uint128 operator|(uint128 lhs, uint128 rhs) { return MakeUint128(Uint128High64(lhs) | Uint128High64(rhs), Uint128Low64(lhs) | Uint128Low64(rhs)); } -inline uint128 operator&(uint128 lhs, uint128 rhs) { +constexpr inline uint128 operator&(uint128 lhs, uint128 rhs) { return MakeUint128(Uint128High64(lhs) & Uint128High64(rhs), Uint128Low64(lhs) & Uint128Low64(rhs)); } -inline uint128 operator^(uint128 lhs, uint128 rhs) { +constexpr inline uint128 operator^(uint128 lhs, uint128 rhs) { return MakeUint128(Uint128High64(lhs) ^ Uint128High64(rhs), Uint128Low64(lhs) ^ Uint128Low64(rhs)); } diff --git a/absl/strings/BUILD.bazel b/absl/strings/BUILD.bazel index 95ed0868..874dd390 100644 --- a/absl/strings/BUILD.bazel +++ b/absl/strings/BUILD.bazel @@ -330,6 +330,7 @@ cc_library( ":cordz_functions", ":cordz_info", ":cordz_statistics", + ":cordz_update_scope", ":cordz_update_tracker", ":internal", ":str_format", @@ -366,6 +367,7 @@ cc_library( copts = ABSL_DEFAULT_COPTS, deps = [ ":cord_internal", + ":cordz_functions", ":cordz_handle", ":cordz_statistics", ":cordz_update_tracker", @@ -378,6 +380,33 @@ cc_library( ) cc_library( + name = "cordz_update_scope", + hdrs = ["internal/cordz_update_scope.h"], + copts = ABSL_DEFAULT_COPTS, + deps = [ + ":cord_internal", + ":cordz_info", + ":cordz_update_tracker", + "//absl/base:config", + "//absl/base:core_headers", + ], +) + +cc_test( + name = "cordz_update_scope_test", + srcs = ["internal/cordz_update_scope_test.cc"], + copts = ABSL_DEFAULT_COPTS, + deps = [ + ":cord_internal", + ":cordz_info", + ":cordz_update_scope", + ":cordz_update_tracker", + "//absl/base:config", + "@com_google_googletest//:gtest_main", + ], +) + +cc_library( name = "cordz_sample_token", srcs = ["internal/cordz_sample_token.cc"], hdrs = ["internal/cordz_sample_token.h"], diff --git a/absl/strings/CMakeLists.txt b/absl/strings/CMakeLists.txt index 5a13831f..f48584e0 100644 --- a/absl/strings/CMakeLists.txt +++ b/absl/strings/CMakeLists.txt @@ -689,6 +689,7 @@ absl_cc_library( DEPS absl::config absl::cord_internal + absl::cordz_functions absl::cordz_handle absl::cordz_statistics absl::cordz_update_tracker @@ -756,6 +757,38 @@ absl_cc_test( absl_cc_library( NAME + cordz_update_scope + HDRS + "internal/cordz_update_scope.h" + COPTS + ${ABSL_TEST_COPTS} + DEPS + absl::config + absl::cord_internal + absl::cordz_info + absl::cordz_update_tracker + absl::core_headers +) + +absl_cc_test( + NAME + cordz_update_scope_test + SRCS + "internal/cordz_update_scope_test.cc" + COPTS + ${ABSL_TEST_COPTS} + DEPS + absl::config + absl::cord_internal + absl::cordz_info + absl::cordz_update_scope + absl::cordz_update_tracker + absl::core_headers + gmock_main +) + +absl_cc_library( + NAME cord HDRS "cord.h" @@ -769,6 +802,7 @@ absl_cc_library( absl::cord_internal absl::cordz_functions absl::cordz_info + absl::cordz_update_scope absl::cordz_update_tracker absl::core_headers absl::endian diff --git a/absl/strings/cord.cc b/absl/strings/cord.cc index 770c5a70..44e4d464 100644 --- a/absl/strings/cord.cc +++ b/absl/strings/cord.cc @@ -39,6 +39,7 @@ #include "absl/strings/internal/cord_rep_flat.h" #include "absl/strings/internal/cord_rep_ring.h" #include "absl/strings/internal/cordz_statistics.h" +#include "absl/strings/internal/cordz_update_scope.h" #include "absl/strings/internal/cordz_update_tracker.h" #include "absl/strings/internal/resize_uninitialized.h" #include "absl/strings/str_cat.h" @@ -55,8 +56,9 @@ using ::absl::cord_internal::CordRepExternal; using ::absl::cord_internal::CordRepFlat; using ::absl::cord_internal::CordRepRing; using ::absl::cord_internal::CordRepSubstring; -using ::absl::cord_internal::kMinFlatLength; +using ::absl::cord_internal::InlineData; using ::absl::cord_internal::kMaxFlatLength; +using ::absl::cord_internal::kMinFlatLength; using ::absl::cord_internal::CONCAT; using ::absl::cord_internal::EXTERNAL; @@ -301,16 +303,20 @@ inline char* Cord::InlineRep::set_data(size_t n) { return data_.as_chars(); } +static inline CordRepFlat* MakeFlat(const InlineData& data, size_t extra = 0) { + static_assert(kMinFlatLength >= sizeof(data), ""); + size_t len = data.inline_size(); + CordRepFlat* result = CordRepFlat::New(len + extra); + result->length = len; + memcpy(result->Data(), data.as_chars(), sizeof(data)); + return result; +} + inline CordRep* Cord::InlineRep::force_tree(size_t extra_hint) { if (data_.is_tree()) { return data_.as_tree(); } - - size_t len = inline_size(); - CordRepFlat* result = CordRepFlat::New(len + extra_hint); - result->length = len; - static_assert(kMinFlatLength >= sizeof(data_), ""); - memcpy(result->Data(), data_.as_chars(), sizeof(data_)); + CordRepFlat* result = MakeFlat(data_, extra_hint); set_tree(result); return result; } @@ -494,22 +500,6 @@ static bool RepMemoryUsageLeaf(const CordRep* rep, size_t* total_mem_usage) { return false; } -void Cord::InlineRep::StartProfiling() { - CordRep* tree = as_tree(); - auto* cordz_info = absl::cord_internal::CordzInfo::TrackCord(tree); - set_cordz_info(cordz_info); - cordz_info->RecordMetrics(size()); -} - -void Cord::InlineRep::StartProfiling(const Cord::InlineRep& src) { - CordRep* tree = as_tree(); - absl::cord_internal::CordzInfo* parent = - src.is_profiled() ? src.cordz_info() : nullptr; - auto* cordz_info = absl::cord_internal::CordzInfo::TrackCord(tree, parent); - set_cordz_info(cordz_info); - cordz_info->RecordMetrics(size()); -} - void Cord::InlineRep::UpdateCordzStatisticsSlow() { CordRep* tree = as_tree(); data_.cordz_info()->RecordMetrics(tree->length); @@ -522,9 +512,7 @@ void Cord::InlineRep::AssignSlow(const Cord::InlineRep& src) { if (is_tree()) { CordRep::Ref(tree()); clear_cordz_info(); - if (ABSL_PREDICT_FALSE(should_profile())) { - StartProfiling(src); - } + CordzInfo::MaybeTrackCord(data_, CordzUpdateTracker::kAssignCord); } } @@ -540,15 +528,14 @@ void Cord::InlineRep::UnrefTree() { // -------------------------------------------------------------------- // Constructors and destructors -Cord::Cord(absl::string_view src) { +Cord::Cord(absl::string_view src) : contents_(InlineData::kDefaultInit) { const size_t n = src.size(); if (n <= InlineRep::kMaxInline) { - contents_.set_data(src.data(), n, false); + contents_.set_data(src.data(), n, true); } else { contents_.data_.make_tree(NewTree(src.data(), n, 0)); - if (ABSL_PREDICT_FALSE(absl::cord_internal::cordz_should_profile())) { - contents_.StartProfiling(); - } + CordzInfo::MaybeTrackCord(contents_.data_, + CordzUpdateTracker::kConstructorString); } } diff --git a/absl/strings/cord.h b/absl/strings/cord.h index 1e6a73a5..bca20981 100644 --- a/absl/strings/cord.h +++ b/absl/strings/cord.h @@ -84,6 +84,7 @@ #include "absl/strings/internal/cordz_functions.h" #include "absl/strings/internal/cordz_info.h" #include "absl/strings/internal/cordz_statistics.h" +#include "absl/strings/internal/cordz_update_scope.h" #include "absl/strings/internal/cordz_update_tracker.h" #include "absl/strings/internal/resize_uninitialized.h" #include "absl/strings/internal/string_constant.h" @@ -669,7 +670,11 @@ class Cord { explicit constexpr Cord(strings_internal::StringConstant<T>); private: + using CordzInfo = cord_internal::CordzInfo; + using CordzUpdateScope = cord_internal::CordzUpdateScope; using CordzUpdateTracker = cord_internal::CordzUpdateTracker; + using InlineData = cord_internal::InlineData; + using MethodIdentifier = CordzUpdateTracker::MethodIdentifier; friend class CordTestPeer; friend bool operator==(const Cord& lhs, const Cord& rhs); @@ -694,6 +699,7 @@ class Cord { static_assert(kMaxInline >= sizeof(absl::cord_internal::CordRep*), ""); constexpr InlineRep() : data_() {} + explicit InlineRep(InlineData::DefaultInitType init) : data_(init) {} InlineRep(const InlineRep& src); InlineRep(InlineRep&& src); InlineRep& operator=(const InlineRep& src); @@ -779,15 +785,6 @@ class Cord { // Resets the current cordz_info to null / empty. void clear_cordz_info() { data_.clear_cordz_info(); } - // Starts profiling this cord. - void StartProfiling(); - - // Starts profiling this cord which has been copied from `src`. - void StartProfiling(const Cord::InlineRep& src); - - // Returns true if a Cord should be profiled and false otherwise. - static bool should_profile(); - // Updates the cordz statistics. info may be nullptr if the CordzInfo object // is unknown. void UpdateCordzStatistics(); @@ -964,9 +961,8 @@ inline Cord::InlineRep::InlineRep(const Cord::InlineRep& src) if (is_tree()) { data_.clear_cordz_info(); absl::cord_internal::CordRep::Ref(as_tree()); - if (ABSL_PREDICT_FALSE(should_profile())) { - StartProfiling(src); - } + CordzInfo::MaybeTrackCord(data_, src.data_, + CordzUpdateTracker::kConstructorCord); } } @@ -1040,9 +1036,7 @@ inline void Cord::InlineRep::set_tree(absl::cord_internal::CordRep* rep) { } else { // `data_` contains inlined data: initialize data_ to tree value `rep`. data_.make_tree(rep); - if (ABSL_PREDICT_FALSE(should_profile())) { - StartProfiling(); - } + CordzInfo::MaybeTrackCord(data_, CordzUpdateTracker::kUnknown); } UpdateCordzStatistics(); } @@ -1074,10 +1068,6 @@ inline void Cord::InlineRep::CopyToArray(char* dst) const { cord_internal::SmallMemmove(dst, data_.as_chars(), n); } -inline ABSL_ATTRIBUTE_ALWAYS_INLINE bool Cord::InlineRep::should_profile() { - return absl::cord_internal::cordz_should_profile(); -} - inline void Cord::InlineRep::UpdateCordzStatistics() { if (ABSL_PREDICT_TRUE(!is_profiled())) return; UpdateCordzStatisticsSlow(); diff --git a/absl/strings/internal/cord_internal.h b/absl/strings/internal/cord_internal.h index 01ce70ae..c7ae0104 100644 --- a/absl/strings/internal/cord_internal.h +++ b/absl/strings/internal/cord_internal.h @@ -329,6 +329,9 @@ static constexpr cordz_info_t BigEndianByte(unsigned char value) { class InlineData { public: + // DefaultInitType forces the use of the default initialization constructor. + enum DefaultInitType { kDefaultInit }; + // kNullCordzInfo holds the big endian representation of intptr_t(1) // This is the 'null' / initial value of 'cordz_info'. The null value // is specifically big endian 1 as with 64-bit pointers, the last @@ -336,6 +339,7 @@ class InlineData { static constexpr cordz_info_t kNullCordzInfo = BigEndianByte(1); constexpr InlineData() : as_chars_{0} {} + explicit InlineData(DefaultInitType) {} explicit constexpr InlineData(CordRep* rep) : as_tree_(rep) {} explicit constexpr InlineData(absl::string_view chars) : as_chars_{ diff --git a/absl/strings/internal/cordz_info.cc b/absl/strings/internal/cordz_info.cc index 6540d134..d2200680 100644 --- a/absl/strings/internal/cordz_info.cc +++ b/absl/strings/internal/cordz_info.cc @@ -45,15 +45,22 @@ CordzInfo* CordzInfo::Next(const CordzSnapshot& snapshot) const { return ci_next_unsafe(); } -CordzInfo* CordzInfo::TrackCord(CordRep* rep, const CordzInfo* src, - MethodIdentifier method) { - CordzInfo* ci = new CordzInfo(rep, src, method); - ci->Track(); - return ci; +void CordzInfo::TrackCord(InlineData& cord, MethodIdentifier method) { + assert(cord.is_tree()); + assert(!cord.is_profiled()); + CordzInfo* cordz_info = new CordzInfo(cord.as_tree(), nullptr, method); + cord.set_cordz_info(cordz_info); + cordz_info->Track(); } -CordzInfo* CordzInfo::TrackCord(CordRep* rep, MethodIdentifier method) { - return TrackCord(rep, nullptr, method); +void CordzInfo::TrackCord(InlineData& cord, const InlineData& src, + MethodIdentifier method) { + assert(cord.is_tree()); + assert(!cord.is_profiled()); + auto* info = src.is_tree() && src.is_profiled() ? src.cordz_info() : nullptr; + CordzInfo* cordz_info = new CordzInfo(cord.as_tree(), info, method); + cord.set_cordz_info(cordz_info); + cordz_info->Track(); } void CordzInfo::UntrackCord(CordzInfo* cordz_info) { @@ -160,14 +167,6 @@ void CordzInfo::Unlock() ABSL_UNLOCK_FUNCTION(mutex_) { } } -void CordzInfo::SetCordRep(CordRep* rep) { - mutex_.AssertHeld(); - rep_ = rep; - if (rep) { - size_.store(rep->length); - } -} - absl::Span<void* const> CordzInfo::GetStack() const { return absl::MakeConstSpan(stack_, stack_depth_); } diff --git a/absl/strings/internal/cordz_info.h b/absl/strings/internal/cordz_info.h index 5345be75..4cf57664 100644 --- a/absl/strings/internal/cordz_info.h +++ b/absl/strings/internal/cordz_info.h @@ -22,6 +22,7 @@ #include "absl/base/config.h" #include "absl/base/thread_annotations.h" #include "absl/strings/internal/cord_internal.h" +#include "absl/strings/internal/cordz_functions.h" #include "absl/strings/internal/cordz_handle.h" #include "absl/strings/internal/cordz_statistics.h" #include "absl/strings/internal/cordz_update_tracker.h" @@ -41,22 +42,37 @@ namespace cord_internal { // and will either be deleted or appended to the global_delete_queue. If it is // placed on the global_delete_queue, the CordzInfo object will be cleaned in // the destructor of a CordzSampleToken object. -class CordzInfo : public CordzHandle { +class ABSL_LOCKABLE CordzInfo : public CordzHandle { public: using MethodIdentifier = CordzUpdateTracker::MethodIdentifier; - // All profiled Cords should be accompanied by a call to TrackCord. // TrackCord creates a CordzInfo instance which tracks important metrics of - // the sampled cord. CordzInfo instances are placed in a global list which is - // used to discover and snapshot all actively tracked cords. - // Callers are responsible for calling UntrackCord() before the tracked Cord - // instance is deleted, or to stop tracking the sampled Cord. - // Callers are also responsible for guarding changes to `rep` through the - // Lock() and Unlock() calls, and calling SetCordRep() if the root of the - // sampled cord changes before the old root has been unreffed and/or deleted. - // `method` identifies the Cord method which initiated the cord to be sampled. - static CordzInfo* TrackCord( - CordRep* rep, MethodIdentifier method = MethodIdentifier::kUnknown); + // a sampled cord, and stores the created CordzInfo instance into `cord'. All + // CordzInfo instances are placed in a global list which is used to discover + // and snapshot all actively tracked cords. Callers are responsible for + // calling UntrackCord() before the tracked Cord instance is deleted, or to + // stop tracking the sampled Cord. Callers are also responsible for guarding + // changes to the 'tree' value of a Cord (InlineData.tree) through the Lock() + // and Unlock() calls. Any change resulting in a new tree value for the cord + // requires a call to SetCordRep() before the old tree has been unreffed + // and/or deleted. `method` identifies the Cord public API method initiating + // the cord to be sampled. + // Requires `cord` to hold a tree, and `cord.cordz_info()` to be null. + static void TrackCord(InlineData& cord, MethodIdentifier method); + + // Identical to TrackCord(), except that this function fills the + // `parent_stack` and `parent_method` properties of the returned CordzInfo + // instance from the provided `src` instance if `src` is sampled. + // This function should be used for sampling 'copy constructed' cords. + static void TrackCord(InlineData& cord, const InlineData& src, + MethodIdentifier method); + + // Maybe sample the cord identified by 'cord' for method 'method'. + // Uses `cordz_should_profile` to randomly pick cords to be sampled, and if + // so, invokes `TrackCord` to start sampling `cord`. + static void MaybeTrackCord(InlineData& cord, MethodIdentifier method); + static void MaybeTrackCord(InlineData& cord, const InlineData& src, + MethodIdentifier method); // Stops tracking changes for a sampled cord, and deletes the provided info. // This function must be called before the sampled cord instance is deleted, @@ -65,14 +81,6 @@ class CordzInfo : public CordzHandle { // CordInfo instance is being held by a concurrent collection thread. static void UntrackCord(CordzInfo* cordz_info); - // Identical to TrackCord(), except that this function fills the - // `parent_stack` and `parent_method` properties of the returned CordzInfo - // instance from the provided `src` instance if `src` is not null. - // This function should be used for sampling 'copy constructed' cords. - static CordzInfo* TrackCord( - CordRep* rep, const CordzInfo* src, - MethodIdentifier method = MethodIdentifier::kUnknown); - CordzInfo() = delete; CordzInfo(const CordzInfo&) = delete; CordzInfo& operator=(const CordzInfo&) = delete; @@ -92,6 +100,9 @@ class CordzInfo : public CordzHandle { // then this method will delete this CordzInfo instance. void Unlock() ABSL_UNLOCK_FUNCTION(mutex_); + // 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 // 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 @@ -136,6 +147,9 @@ class CordzInfo : public CordzHandle { MethodIdentifier method); ~CordzInfo() override; + void Track(); + void Untrack(); + // Returns the parent method from `src`, which is either `parent_method_` or // `method_` depending on `parent_method_` being kUnknown. // Returns kUnknown if `src` is null. @@ -147,9 +161,6 @@ class CordzInfo : public CordzHandle { // Returns 0 if `src` is null. static int FillParentStack(const CordzInfo* src, void** stack); - void Track(); - void Untrack(); - // 'Unsafe' head/next/prev accessors not requiring the lock being held. // These are used exclusively for iterations (Head / Next) where we enforce // a token being held, so reading an 'old' / deleted pointer is fine. @@ -184,6 +195,34 @@ class CordzInfo : public CordzHandle { std::atomic<int64_t> size_{0}; }; +inline ABSL_ATTRIBUTE_ALWAYS_INLINE void CordzInfo::MaybeTrackCord( + InlineData& cord, MethodIdentifier method) { + if (ABSL_PREDICT_FALSE(cordz_should_profile())) { + TrackCord(cord, method); + } +} + +inline ABSL_ATTRIBUTE_ALWAYS_INLINE void CordzInfo::MaybeTrackCord( + InlineData& cord, const InlineData& src, MethodIdentifier method) { + if (ABSL_PREDICT_FALSE(cordz_should_profile())) { + TrackCord(cord, src, method); + } +} + +inline void CordzInfo::AssertHeld() ABSL_ASSERT_EXCLUSIVE_LOCK(mutex_) { +#ifndef NDEBUG + mutex_.AssertHeld(); +#endif +} + +inline void CordzInfo::SetCordRep(CordRep* rep) { + AssertHeld(); + rep_ = rep; + if (rep) { + size_.store(rep->length); + } +} + } // 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 66acf9b6..89e5fae5 100644 --- a/absl/strings/internal/cordz_info_test.cc +++ b/absl/strings/internal/cordz_info_test.cc @@ -1,4 +1,4 @@ -// Copyright 2019 The Abseil Authors. +// Copyright 2019 The Abseil Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -48,6 +48,13 @@ struct TestCordRep { ~TestCordRep() { CordRepFlat::Delete(rep); } }; +struct TestCordData { + TestCordRep rep; + InlineData data; + + TestCordData() { data.make_tree(rep.rep); } +}; + // Used test values auto constexpr kUnknownMethod = CordzUpdateTracker::kUnknown; auto constexpr kTrackCordMethod = CordzUpdateTracker::kConstructorString; @@ -72,18 +79,20 @@ std::string FormatStack(absl::Span<void* const> raw_stack) { } TEST(CordzInfoTest, TrackCord) { - TestCordRep rep; - CordzInfo* info = CordzInfo::TrackCord(rep.rep); + TestCordData data; + CordzInfo::TrackCord(data.data, kTrackCordMethod); + CordzInfo* info = data.data.cordz_info(); ASSERT_THAT(info, Ne(nullptr)); EXPECT_FALSE(info->is_snapshot()); EXPECT_THAT(CordzInfo::Head(CordzSnapshot()), Eq(info)); - EXPECT_THAT(info->GetCordRepForTesting(), Eq(rep.rep)); + EXPECT_THAT(info->GetCordRepForTesting(), Eq(data.rep.rep)); CordzInfo::UntrackCord(info); } TEST(CordzInfoTest, UntrackCord) { - TestCordRep rep; - CordzInfo* info = CordzInfo::TrackCord(rep.rep); + TestCordData data; + CordzInfo::TrackCord(data.data, kTrackCordMethod); + CordzInfo* info = data.data.cordz_info(); CordzSnapshot snapshot; CordzInfo::UntrackCord(info); @@ -93,21 +102,23 @@ TEST(CordzInfoTest, UntrackCord) { } TEST(CordzInfoTest, SetCordRep) { - TestCordRep rep; - CordzInfo* info = CordzInfo::TrackCord(rep.rep); + TestCordData data; + CordzInfo::TrackCord(data.data, kTrackCordMethod); + CordzInfo* info = data.data.cordz_info(); - TestCordRep rep2; + TestCordRep rep; info->Lock(CordzUpdateTracker::kAppendCord); - info->SetCordRep(rep2.rep); + info->SetCordRep(rep.rep); info->Unlock(); - EXPECT_THAT(info->GetCordRepForTesting(), Eq(rep2.rep)); + EXPECT_THAT(info->GetCordRepForTesting(), Eq(rep.rep)); CordzInfo::UntrackCord(info); } TEST(CordzInfoTest, SetCordRepNullUntracksCordOnUnlock) { - TestCordRep rep; - CordzInfo* info = CordzInfo::TrackCord(rep.rep); + TestCordData data; + CordzInfo::TrackCord(data.data, kTrackCordMethod); + CordzInfo* info = data.data.cordz_info(); info->Lock(CordzUpdateTracker::kAppendString); info->SetCordRep(nullptr); @@ -121,25 +132,29 @@ TEST(CordzInfoTest, SetCordRepNullUntracksCordOnUnlock) { #if GTEST_HAS_DEATH_TEST TEST(CordzInfoTest, SetCordRepRequiresMutex) { + TestCordData data; + CordzInfo::TrackCord(data.data, kTrackCordMethod); + CordzInfo* info = data.data.cordz_info(); TestCordRep rep; - CordzInfo* info = CordzInfo::TrackCord(rep.rep); - TestCordRep rep2; - EXPECT_DEATH(info->SetCordRep(rep2.rep), ".*"); + EXPECT_DEBUG_DEATH(info->SetCordRep(rep.rep), ".*"); CordzInfo::UntrackCord(info); } #endif // GTEST_HAS_DEATH_TEST TEST(CordzInfoTest, TrackUntrackHeadFirstV2) { - TestCordRep rep; CordzSnapshot snapshot; EXPECT_THAT(CordzInfo::Head(snapshot), Eq(nullptr)); - CordzInfo* info1 = CordzInfo::TrackCord(rep.rep); + TestCordData data; + CordzInfo::TrackCord(data.data, kTrackCordMethod); + CordzInfo* info1 = data.data.cordz_info(); ASSERT_THAT(CordzInfo::Head(snapshot), Eq(info1)); EXPECT_THAT(info1->Next(snapshot), Eq(nullptr)); - CordzInfo* info2 = CordzInfo::TrackCord(rep.rep); + TestCordData data2; + CordzInfo::TrackCord(data2.data, kTrackCordMethod); + CordzInfo* info2 = data2.data.cordz_info(); ASSERT_THAT(CordzInfo::Head(snapshot), Eq(info2)); EXPECT_THAT(info2->Next(snapshot), Eq(info1)); EXPECT_THAT(info1->Next(snapshot), Eq(nullptr)); @@ -153,15 +168,18 @@ TEST(CordzInfoTest, TrackUntrackHeadFirstV2) { } TEST(CordzInfoTest, TrackUntrackTailFirstV2) { - TestCordRep rep; CordzSnapshot snapshot; EXPECT_THAT(CordzInfo::Head(snapshot), Eq(nullptr)); - CordzInfo* info1 = CordzInfo::TrackCord(rep.rep); + TestCordData data; + CordzInfo::TrackCord(data.data, kTrackCordMethod); + CordzInfo* info1 = data.data.cordz_info(); ASSERT_THAT(CordzInfo::Head(snapshot), Eq(info1)); EXPECT_THAT(info1->Next(snapshot), Eq(nullptr)); - CordzInfo* info2 = CordzInfo::TrackCord(rep.rep); + TestCordData data2; + CordzInfo::TrackCord(data2.data, kTrackCordMethod); + CordzInfo* info2 = data2.data.cordz_info(); ASSERT_THAT(CordzInfo::Head(snapshot), Eq(info2)); EXPECT_THAT(info2->Next(snapshot), Eq(info1)); EXPECT_THAT(info1->Next(snapshot), Eq(nullptr)); @@ -175,7 +193,7 @@ TEST(CordzInfoTest, TrackUntrackTailFirstV2) { } TEST(CordzInfoTest, StackV2) { - TestCordRep rep; + TestCordData data; // kMaxStackDepth is intentionally less than 64 (which is the max depth that // Cordz will record) because if the actual stack depth is over 64 // (which it is on Apple platforms) then the expected_stack will end up @@ -186,7 +204,8 @@ TEST(CordzInfoTest, StackV2) { // makes small modifications to its testing stack. 50 is sufficient to prove // that we got a decent stack. static constexpr int kMaxStackDepth = 50; - CordzInfo* info = CordzInfo::TrackCord(rep.rep); + CordzInfo::TrackCord(data.data, kTrackCordMethod); + CordzInfo* info = data.data.cordz_info(); std::vector<void*> local_stack; local_stack.resize(kMaxStackDepth); // In some environments we don't get stack traces. For example in Android @@ -209,19 +228,21 @@ TEST(CordzInfoTest, StackV2) { } // Local helper functions to get different stacks for child and parent. -CordzInfo* TrackChildCord(CordRep* rep, const CordzInfo* parent) { - return CordzInfo::TrackCord(rep, parent, kChildMethod); +CordzInfo* TrackChildCord(InlineData& data, const InlineData& parent) { + CordzInfo::TrackCord(data, parent, kChildMethod); + return data.cordz_info(); } -CordzInfo* TrackParentCord(CordRep* rep) { - return CordzInfo::TrackCord(rep, kTrackCordMethod); +CordzInfo* TrackParentCord(InlineData& data) { + CordzInfo::TrackCord(data, kTrackCordMethod); + return data.cordz_info(); } TEST(CordzInfoTest, GetStatistics) { - TestCordRep rep; - CordzInfo* info = TrackParentCord(rep.rep); + TestCordData data; + CordzInfo* info = TrackParentCord(data.data); CordzStatistics statistics = info->GetCordzStatistics(); - EXPECT_THAT(statistics.size, Eq(rep.rep->length)); + EXPECT_THAT(statistics.size, Eq(data.rep.rep->length)); EXPECT_THAT(statistics.method, Eq(kTrackCordMethod)); EXPECT_THAT(statistics.parent_method, Eq(kUnknownMethod)); EXPECT_THAT(statistics.update_tracker.Value(kTrackCordMethod), Eq(1)); @@ -230,8 +251,8 @@ TEST(CordzInfoTest, GetStatistics) { } TEST(CordzInfoTest, LockCountsMethod) { - TestCordRep rep; - CordzInfo* info = TrackParentCord(rep.rep); + TestCordData data; + CordzInfo* info = TrackParentCord(data.data); info->Lock(kUpdateMethod); info->Unlock(); @@ -245,16 +266,17 @@ TEST(CordzInfoTest, LockCountsMethod) { } TEST(CordzInfoTest, FromParent) { - TestCordRep rep; - CordzInfo* info_parent = TrackParentCord(rep.rep); - CordzInfo* info_child = TrackChildCord(rep.rep, info_parent); + TestCordData parent; + TestCordData child; + CordzInfo* info_parent = TrackParentCord(parent.data); + CordzInfo* info_child = TrackChildCord(child.data, parent.data); std::string stack = FormatStack(info_parent->GetStack()); std::string parent_stack = FormatStack(info_child->GetParentStack()); EXPECT_THAT(stack, Eq(parent_stack)); CordzStatistics statistics = info_child->GetCordzStatistics(); - EXPECT_THAT(statistics.size, Eq(rep.rep->length)); + EXPECT_THAT(statistics.size, Eq(child.rep.rep->length)); EXPECT_THAT(statistics.method, Eq(kChildMethod)); EXPECT_THAT(statistics.parent_method, Eq(kTrackCordMethod)); EXPECT_THAT(statistics.update_tracker.Value(kChildMethod), Eq(1)); @@ -263,12 +285,13 @@ TEST(CordzInfoTest, FromParent) { CordzInfo::UntrackCord(info_child); } -TEST(CordzInfoTest, FromParentNullptr) { - TestCordRep rep; - CordzInfo* info = TrackChildCord(rep.rep, nullptr); +TEST(CordzInfoTest, FromParentInlined) { + InlineData parent; + TestCordData child; + CordzInfo* info = TrackChildCord(child.data, parent); EXPECT_TRUE(info->GetParentStack().empty()); CordzStatistics statistics = info->GetCordzStatistics(); - EXPECT_THAT(statistics.size, Eq(rep.rep->length)); + EXPECT_THAT(statistics.size, Eq(child.rep.rep->length)); EXPECT_THAT(statistics.method, Eq(kChildMethod)); EXPECT_THAT(statistics.parent_method, Eq(kUnknownMethod)); EXPECT_THAT(statistics.update_tracker.Value(kChildMethod), Eq(1)); @@ -276,8 +299,8 @@ TEST(CordzInfoTest, FromParentNullptr) { } TEST(CordzInfoTest, RecordMetrics) { - TestCordRep rep; - CordzInfo* info = TrackParentCord(rep.rep); + TestCordData data; + CordzInfo* info = TrackParentCord(data.data); CordzStatistics expected; expected.size = 100; diff --git a/absl/strings/internal/cordz_sample_token_test.cc b/absl/strings/internal/cordz_sample_token_test.cc index 8c052642..5fbfd01c 100644 --- a/absl/strings/internal/cordz_sample_token_test.cc +++ b/absl/strings/internal/cordz_sample_token_test.cc @@ -14,6 +14,10 @@ #include "absl/strings/internal/cordz_sample_token.h" +#include <memory> +#include <type_traits> +#include <vector> + #include "gmock/gmock.h" #include "gtest/gtest.h" #include "absl/memory/memory.h" @@ -46,6 +50,16 @@ struct TestCordRep { ~TestCordRep() { CordRepFlat::Delete(rep); } }; +struct TestCord { + TestCordRep rep; + InlineData data; + + TestCord() { data.make_tree(rep.rep); } +}; + +// Used test values +auto constexpr kTrackCordMethod = CordzUpdateTracker::kConstructorString; + TEST(CordzSampleTokenTest, IteratorTraits) { static_assert(std::is_copy_constructible<CordzSampleToken::Iterator>::value, ""); @@ -83,12 +97,13 @@ TEST(CordzSampleTokenTest, IteratorEmpty) { } TEST(CordzSampleTokenTest, Iterator) { - TestCordRep rep1; - TestCordRep rep2; - TestCordRep rep3; - CordzInfo* info1 = CordzInfo::TrackCord(rep1.rep); - CordzInfo* info2 = CordzInfo::TrackCord(rep2.rep); - CordzInfo* info3 = CordzInfo::TrackCord(rep3.rep); + TestCord cord1, cord2, cord3; + CordzInfo::TrackCord(cord1.data, kTrackCordMethod); + CordzInfo* info1 = cord1.data.cordz_info(); + CordzInfo::TrackCord(cord2.data, kTrackCordMethod); + CordzInfo* info2 = cord2.data.cordz_info(); + CordzInfo::TrackCord(cord3.data, kTrackCordMethod); + CordzInfo* info3 = cord3.data.cordz_info(); CordzSampleToken token; std::vector<const CordzInfo*> found; @@ -104,22 +119,25 @@ TEST(CordzSampleTokenTest, Iterator) { } TEST(CordzSampleTokenTest, IteratorEquality) { - TestCordRep rep1; - TestCordRep rep2; - TestCordRep rep3; - CordzInfo* info1 = CordzInfo::TrackCord(rep1.rep); + TestCord cord1; + TestCord cord2; + TestCord cord3; + CordzInfo::TrackCord(cord1.data, kTrackCordMethod); + CordzInfo* info1 = cord1.data.cordz_info(); CordzSampleToken token1; // lhs starts with the CordzInfo corresponding to cord1 at the head. CordzSampleToken::Iterator lhs = token1.begin(); - CordzInfo* info2 = CordzInfo::TrackCord(rep2.rep); + CordzInfo::TrackCord(cord2.data, kTrackCordMethod); + CordzInfo* info2 = cord2.data.cordz_info(); CordzSampleToken token2; // rhs starts with the CordzInfo corresponding to cord2 at the head. CordzSampleToken::Iterator rhs = token2.begin(); - CordzInfo* info3 = CordzInfo::TrackCord(rep3.rep); + CordzInfo::TrackCord(cord3.data, kTrackCordMethod); + CordzInfo* info3 = cord3.data.cordz_info(); // lhs is on cord1 while rhs is on cord2. EXPECT_THAT(lhs, Ne(rhs)); @@ -149,10 +167,8 @@ TEST(CordzSampleTokenTest, MultiThreaded) { for (int i = 0; i < kNumThreads; ++i) { pool.Schedule([&stop]() { absl::BitGen gen; - TestCordRep reps[kNumCords]; - CordzInfo* infos[kNumCords] = {nullptr}; - std::vector<std::unique_ptr<CordzSampleToken>> tokens; - tokens.resize(kNumTokens); + TestCord cords[kNumCords]; + std::unique_ptr<CordzSampleToken> tokens[kNumTokens]; while (!stop.HasBeenNotified()) { // Randomly perform one of five actions: @@ -163,36 +179,38 @@ TEST(CordzSampleTokenTest, MultiThreaded) { // 5) Sample int index = absl::Uniform(gen, 0, kNumCords); if (absl::Bernoulli(gen, 0.5)) { + TestCord& cord = cords[index]; // Track/untrack. - if (infos[index]) { + if (cord.data.is_profiled()) { // 1) Untrack - CordzInfo::UntrackCord(infos[index]); - infos[index] = nullptr; + CordzInfo::UntrackCord(cord.data.cordz_info()); + cord.data.clear_cordz_info();; } else { // 2) Track - infos[index] = CordzInfo::TrackCord(reps[index].rep); + CordzInfo::TrackCord(cord.data, kTrackCordMethod); } } else { - if (tokens[index]) { + std::unique_ptr<CordzSampleToken>& token = tokens[index]; + if (token) { if (absl::Bernoulli(gen, 0.5)) { // 3) Iterate over Cords visible to a token. - for (const CordzInfo& info : *tokens[index]) { + for (const CordzInfo& info : *token) { // This is trivial work to allow us to compile the loop. - EXPECT_THAT(info.Next(*tokens[index]), Ne(&info)); + EXPECT_THAT(info.Next(*token), Ne(&info)); } } else { // 4) Unsample - tokens[index].reset(); + token = nullptr; } } else { // 5) Sample - tokens[index] = absl::make_unique<CordzSampleToken>(); + token = absl::make_unique<CordzSampleToken>(); } } } - for (CordzInfo* info : infos) { - if (info != nullptr) { - CordzInfo::UntrackCord(info); + for (TestCord& cord : cords) { + if (cord.data.is_profiled()) { + CordzInfo::UntrackCord(cord.data.cordz_info()); } } }); diff --git a/absl/strings/internal/cordz_update_scope.h b/absl/strings/internal/cordz_update_scope.h new file mode 100644 index 00000000..018359a8 --- /dev/null +++ b/absl/strings/internal/cordz_update_scope.h @@ -0,0 +1,65 @@ +// Copyright 2021 The Abseil Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef ABSL_STRINGS_INTERNAL_CORDZ_UPDATE_SCOPE_H_ +#define ABSL_STRINGS_INTERNAL_CORDZ_UPDATE_SCOPE_H_ + +#include "absl/base/config.h" +#include "absl/base/optimization.h" +#include "absl/base/thread_annotations.h" +#include "absl/strings/internal/cord_internal.h" +#include "absl/strings/internal/cordz_info.h" +#include "absl/strings/internal/cordz_update_tracker.h" + +namespace absl { +ABSL_NAMESPACE_BEGIN +namespace cord_internal { + +// CordzUpdateScope scopes an update to the provided CordzInfo. +// The class invokes `info->Lock(method)` and `info->Unlock()` to guard +// cordrep updates. This class does nothing if `info` is null. +// See also the 'Lock`, `Unlock` and `SetCordRep` methods in `CordzInfo`. +class ABSL_SCOPED_LOCKABLE CordzUpdateScope { + public: + CordzUpdateScope(CordzInfo* info, CordzUpdateTracker::MethodIdentifier method) + ABSL_EXCLUSIVE_LOCK_FUNCTION(info) + : info_(info) { + if (ABSL_PREDICT_FALSE(info_)) { + info->Lock(method); + } + } + + ~CordzUpdateScope() ABSL_UNLOCK_FUNCTION() { + if (ABSL_PREDICT_FALSE(info_)) { + info_->Unlock(); + } + } + + void SetCordRep(CordRep* rep) const { + if (ABSL_PREDICT_FALSE(info_)) { + info_->SetCordRep(rep); + } + } + + CordzInfo* info() const { return info_; } + + private: + CordzInfo* const info_; +}; + +} // namespace cord_internal +ABSL_NAMESPACE_END +} // namespace absl + +#endif // ABSL_STRINGS_INTERNAL_CORDZ_UPDATE_SCOPE_H_ diff --git a/absl/strings/internal/cordz_update_scope_test.cc b/absl/strings/internal/cordz_update_scope_test.cc new file mode 100644 index 00000000..4bdc78d3 --- /dev/null +++ b/absl/strings/internal/cordz_update_scope_test.cc @@ -0,0 +1,66 @@ +// Copyright 2021 The Abseil Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "absl/strings/internal/cordz_update_scope.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" +#include "absl/base/config.h" +#include "absl/strings/internal/cord_rep_flat.h" +#include "absl/strings/internal/cordz_info.h" +#include "absl/strings/internal/cordz_update_tracker.h" + +namespace absl { +ABSL_NAMESPACE_BEGIN +namespace cord_internal { + +namespace { + +struct TestCordRep { + CordRepFlat* rep; + + TestCordRep() { + rep = CordRepFlat::New(100); + rep->length = 100; + memset(rep->Data(), 1, 100); + } + ~TestCordRep() { CordRepFlat::Delete(rep); } +}; + +struct TestCord { + TestCordRep rep; + InlineData data; + + TestCord() { data.make_tree(rep.rep); } +}; + +// Used test values +auto constexpr kTrackCordMethod = CordzUpdateTracker::kConstructorString; + +TEST(CordzUpdateScopeTest, ScopeNullptr) { + CordzUpdateScope scope(nullptr, kTrackCordMethod); +} + +TEST(CordzUpdateScopeTest, ScopeSampledCord) { + TestCord cord; + CordzInfo::TrackCord(cord.data, kTrackCordMethod); + CordzUpdateScope scope(cord.data.cordz_info(), kTrackCordMethod); + cord.data.cordz_info()->SetCordRep(nullptr); +} + +} // namespace +ABSL_NAMESPACE_END +} // namespace cord_internal + +} // namespace absl diff --git a/absl/strings/internal/cordz_update_tracker.h b/absl/strings/internal/cordz_update_tracker.h index a090676d..741950b2 100644 --- a/absl/strings/internal/cordz_update_tracker.h +++ b/absl/strings/internal/cordz_update_tracker.h @@ -90,7 +90,14 @@ class CordzUpdateTracker { } private: - std::atomic<int64_t> values_[kNumMethods]; + // Until C++20 std::atomic is not constexpr default-constructible, so we need + // a wrapper for this class to be constexpr constructible. + class Counter : public std::atomic<int64_t> { + public: + constexpr Counter() noexcept : std::atomic<int64_t>(0) {} + }; + + Counter values_[kNumMethods]; }; } // namespace cord_internal |