diff options
author | Abseil Team <absl-team@google.com> | 2021-04-22 04:30:35 -0700 |
---|---|---|
committer | Dino Radaković <dinor@google.com> | 2021-04-22 09:26:52 -0700 |
commit | e38e1aae3866a4ae3a41ccb38d3f05618ea30ca4 (patch) | |
tree | 72a8ed083e3153a908a1c5fe1b3b506212c4bb14 /absl | |
parent | 1ae9b71c474628d60eb251a3f62967fe64151bb2 (diff) |
Export of internal Abseil changes
--
9e0df3dd23da17cd0ff75c93c1493a858032639c by Martijn Vels <mvels@google.com>:
Prep work for changing Cordz instrumentation
Create CordzInfo::TrackCord() and CordzInfo::MaybeTrackCord() methods on InlineData
This follows a suggestion from kfm@ to move where possible Cordz logic out of Cord and Cord::InlineRep, which we can now that InlineData provides us the abstraction of Cords internal data.
PiperOrigin-RevId: 369844310
--
45f39709033bd3bc09fa1a7880a5b3c9eaa617c7 by Abseil Team <absl-team@google.com>:
Fix dependence on C++20 constexpr default construction of std::atomic.
PiperOrigin-RevId: 369745251
--
195ae230963c95068406ab0e73b4e711b5f3cd62 by Martijn Vels <mvels@google.com>:
Reduce the cost of 'SetCordRep()`
This change inlines SetCordRep(), which is currently two stores and a branch, going forward a naked store. AssertHeld is only enforced for debug builds.
The intention here is not to reduce the 'actual' (or 'self') cost of SetCordRep() as it should only be called for sampled cords, but to reduce the cost of a branch and out of line call at the call site. The compiler can now 'perfectly inline' the single branch / store.
PiperOrigin-RevId: 369696265
--
d722199ed69d413994740624159ac7bd001a9219 by Martijn Vels <mvels@google.com>:
Add kDefaultInit initialization
This avoids double store on init
PiperOrigin-RevId: 369655217
--
3499aed79e6cc12ce36277063ec37991bba0cccd by Martijn Vels <mvels@google.com>:
Introduce CordzUpdateScope
PiperOrigin-RevId: 369491326
--
325c1bc99c7d1aeca7bd1273e51a0900b6faf731 by Abseil Team <absl-team@google.com>:
make unary and logical operators constexpr
PiperOrigin-RevId: 369476773
--
ad3bed3dea5e5d7d281ff36ee786f802630c3728 by Martijn Vels <mvels@google.com>:
Add LOCKABLE attribute to CordzInfo
PiperOrigin-RevId: 369476772
GitOrigin-RevId: 9e0df3dd23da17cd0ff75c93c1493a858032639c
Change-Id: I00e2859328fe8da46d2e04d3f07dfe70ec6cb1f5
Diffstat (limited to 'absl')
-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 |
13 files changed, 426 insertions, 165 deletions
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 |