diff --git a/absl/algorithm/container.h b/absl/algorithm/container.h index 6398438f08c..1652e7b055b 100644 --- a/absl/algorithm/container.h +++ b/absl/algorithm/container.h @@ -905,11 +905,11 @@ void c_sort(C& c) { // Overload of c_sort() for performing a `comp` comparison other than the // default `operator<`. -template -void c_sort(C& c, Compare&& comp) { +template +void c_sort(C& c, LessThan&& comp) { std::sort(container_algorithm_internal::c_begin(c), container_algorithm_internal::c_end(c), - std::forward(comp)); + std::forward(comp)); } // c_stable_sort() @@ -925,11 +925,11 @@ void c_stable_sort(C& c) { // Overload of c_stable_sort() for performing a `comp` comparison other than the // default `operator<`. -template -void c_stable_sort(C& c, Compare&& comp) { +template +void c_stable_sort(C& c, LessThan&& comp) { std::stable_sort(container_algorithm_internal::c_begin(c), container_algorithm_internal::c_end(c), - std::forward(comp)); + std::forward(comp)); } // c_is_sorted() @@ -944,11 +944,11 @@ bool c_is_sorted(const C& c) { // c_is_sorted() overload for performing a `comp` comparison other than the // default `operator<`. -template -bool c_is_sorted(const C& c, Compare&& comp) { +template +bool c_is_sorted(const C& c, LessThan&& comp) { return std::is_sorted(container_algorithm_internal::c_begin(c), container_algorithm_internal::c_end(c), - std::forward(comp)); + std::forward(comp)); } // c_partial_sort() @@ -966,14 +966,14 @@ void c_partial_sort( // Overload of c_partial_sort() for performing a `comp` comparison other than // the default `operator<`. -template +template void c_partial_sort( RandomAccessContainer& sequence, container_algorithm_internal::ContainerIter middle, - Compare&& comp) { + LessThan&& comp) { std::partial_sort(container_algorithm_internal::c_begin(sequence), middle, container_algorithm_internal::c_end(sequence), - std::forward(comp)); + std::forward(comp)); } // c_partial_sort_copy() @@ -994,15 +994,15 @@ c_partial_sort_copy(const C& sequence, RandomAccessContainer& result) { // Overload of c_partial_sort_copy() for performing a `comp` comparison other // than the default `operator<`. -template +template container_algorithm_internal::ContainerIter c_partial_sort_copy(const C& sequence, RandomAccessContainer& result, - Compare&& comp) { + LessThan&& comp) { return std::partial_sort_copy(container_algorithm_internal::c_begin(sequence), container_algorithm_internal::c_end(sequence), container_algorithm_internal::c_begin(result), container_algorithm_internal::c_end(result), - std::forward(comp)); + std::forward(comp)); } // c_is_sorted_until() @@ -1018,12 +1018,12 @@ container_algorithm_internal::ContainerIter c_is_sorted_until(C& c) { // Overload of c_is_sorted_until() for performing a `comp` comparison other than // the default `operator<`. -template +template container_algorithm_internal::ContainerIter c_is_sorted_until( - C& c, Compare&& comp) { + C& c, LessThan&& comp) { return std::is_sorted_until(container_algorithm_internal::c_begin(c), container_algorithm_internal::c_end(c), - std::forward(comp)); + std::forward(comp)); } // c_nth_element() @@ -1043,14 +1043,14 @@ void c_nth_element( // Overload of c_nth_element() for performing a `comp` comparison other than // the default `operator<`. -template +template void c_nth_element( RandomAccessContainer& sequence, container_algorithm_internal::ContainerIter nth, - Compare&& comp) { + LessThan&& comp) { std::nth_element(container_algorithm_internal::c_begin(sequence), nth, container_algorithm_internal::c_end(sequence), - std::forward(comp)); + std::forward(comp)); } //------------------------------------------------------------------------------ @@ -1072,12 +1072,12 @@ container_algorithm_internal::ContainerIter c_lower_bound( // Overload of c_lower_bound() for performing a `comp` comparison other than // the default `operator<`. -template +template container_algorithm_internal::ContainerIter c_lower_bound( - Sequence& sequence, T&& value, Compare&& comp) { + Sequence& sequence, T&& value, LessThan&& comp) { return std::lower_bound(container_algorithm_internal::c_begin(sequence), container_algorithm_internal::c_end(sequence), - std::forward(value), std::forward(comp)); + std::forward(value), std::forward(comp)); } // c_upper_bound() @@ -1095,12 +1095,12 @@ container_algorithm_internal::ContainerIter c_upper_bound( // Overload of c_upper_bound() for performing a `comp` comparison other than // the default `operator<`. -template +template container_algorithm_internal::ContainerIter c_upper_bound( - Sequence& sequence, T&& value, Compare&& comp) { + Sequence& sequence, T&& value, LessThan&& comp) { return std::upper_bound(container_algorithm_internal::c_begin(sequence), container_algorithm_internal::c_end(sequence), - std::forward(value), std::forward(comp)); + std::forward(value), std::forward(comp)); } // c_equal_range() @@ -1118,12 +1118,12 @@ c_equal_range(Sequence& sequence, T&& value) { // Overload of c_equal_range() for performing a `comp` comparison other than // the default `operator<`. -template +template container_algorithm_internal::ContainerIterPairType -c_equal_range(Sequence& sequence, T&& value, Compare&& comp) { +c_equal_range(Sequence& sequence, T&& value, LessThan&& comp) { return std::equal_range(container_algorithm_internal::c_begin(sequence), container_algorithm_internal::c_end(sequence), - std::forward(value), std::forward(comp)); + std::forward(value), std::forward(comp)); } // c_binary_search() @@ -1140,12 +1140,12 @@ bool c_binary_search(Sequence&& sequence, T&& value) { // Overload of c_binary_search() for performing a `comp` comparison other than // the default `operator<`. -template -bool c_binary_search(Sequence&& sequence, T&& value, Compare&& comp) { +template +bool c_binary_search(Sequence&& sequence, T&& value, LessThan&& comp) { return std::binary_search(container_algorithm_internal::c_begin(sequence), container_algorithm_internal::c_end(sequence), std::forward(value), - std::forward(comp)); + std::forward(comp)); } //------------------------------------------------------------------------------ @@ -1166,14 +1166,14 @@ OutputIterator c_merge(const C1& c1, const C2& c2, OutputIterator result) { // Overload of c_merge() for performing a `comp` comparison other than // the default `operator<`. -template +template OutputIterator c_merge(const C1& c1, const C2& c2, OutputIterator result, - Compare&& comp) { + LessThan&& comp) { return std::merge(container_algorithm_internal::c_begin(c1), container_algorithm_internal::c_end(c1), container_algorithm_internal::c_begin(c2), container_algorithm_internal::c_end(c2), result, - std::forward(comp)); + std::forward(comp)); } // c_inplace_merge() @@ -1189,13 +1189,13 @@ void c_inplace_merge(C& c, // Overload of c_inplace_merge() for performing a merge using a `comp` other // than `operator<`. -template +template void c_inplace_merge(C& c, container_algorithm_internal::ContainerIter middle, - Compare&& comp) { + LessThan&& comp) { std::inplace_merge(container_algorithm_internal::c_begin(c), middle, container_algorithm_internal::c_end(c), - std::forward(comp)); + std::forward(comp)); } // c_includes() @@ -1213,13 +1213,13 @@ bool c_includes(const C1& c1, const C2& c2) { // Overload of c_includes() for performing a merge using a `comp` other than // `operator<`. -template -bool c_includes(const C1& c1, const C2& c2, Compare&& comp) { +template +bool c_includes(const C1& c1, const C2& c2, LessThan&& comp) { return std::includes(container_algorithm_internal::c_begin(c1), container_algorithm_internal::c_end(c1), container_algorithm_internal::c_begin(c2), container_algorithm_internal::c_end(c2), - std::forward(comp)); + std::forward(comp)); } // c_set_union() @@ -1243,7 +1243,7 @@ OutputIterator c_set_union(const C1& c1, const C2& c2, OutputIterator output) { // Overload of c_set_union() for performing a merge using a `comp` other than // `operator<`. -template ::value, void>::type, @@ -1251,12 +1251,12 @@ template ::value, void>::type> OutputIterator c_set_union(const C1& c1, const C2& c2, OutputIterator output, - Compare&& comp) { + LessThan&& comp) { return std::set_union(container_algorithm_internal::c_begin(c1), container_algorithm_internal::c_end(c1), container_algorithm_internal::c_begin(c2), container_algorithm_internal::c_end(c2), output, - std::forward(comp)); + std::forward(comp)); } // c_set_intersection() @@ -1280,7 +1280,7 @@ OutputIterator c_set_intersection(const C1& c1, const C2& c2, // Overload of c_set_intersection() for performing a merge using a `comp` other // than `operator<`. -template ::value, void>::type, @@ -1288,12 +1288,12 @@ template ::value, void>::type> OutputIterator c_set_intersection(const C1& c1, const C2& c2, - OutputIterator output, Compare&& comp) { + OutputIterator output, LessThan&& comp) { return std::set_intersection(container_algorithm_internal::c_begin(c1), container_algorithm_internal::c_end(c1), container_algorithm_internal::c_begin(c2), container_algorithm_internal::c_end(c2), output, - std::forward(comp)); + std::forward(comp)); } // c_set_difference() @@ -1318,7 +1318,7 @@ OutputIterator c_set_difference(const C1& c1, const C2& c2, // Overload of c_set_difference() for performing a merge using a `comp` other // than `operator<`. -template ::value, void>::type, @@ -1326,12 +1326,12 @@ template ::value, void>::type> OutputIterator c_set_difference(const C1& c1, const C2& c2, - OutputIterator output, Compare&& comp) { + OutputIterator output, LessThan&& comp) { return std::set_difference(container_algorithm_internal::c_begin(c1), container_algorithm_internal::c_end(c1), container_algorithm_internal::c_begin(c2), container_algorithm_internal::c_end(c2), output, - std::forward(comp)); + std::forward(comp)); } // c_set_symmetric_difference() @@ -1357,7 +1357,7 @@ OutputIterator c_set_symmetric_difference(const C1& c1, const C2& c2, // Overload of c_set_symmetric_difference() for performing a merge using a // `comp` other than `operator<`. -template ::value, void>::type, @@ -1366,13 +1366,13 @@ template ::type> OutputIterator c_set_symmetric_difference(const C1& c1, const C2& c2, OutputIterator output, - Compare&& comp) { + LessThan&& comp) { return std::set_symmetric_difference( container_algorithm_internal::c_begin(c1), container_algorithm_internal::c_end(c1), container_algorithm_internal::c_begin(c2), container_algorithm_internal::c_end(c2), output, - std::forward(comp)); + std::forward(comp)); } //------------------------------------------------------------------------------ @@ -1391,11 +1391,11 @@ void c_push_heap(RandomAccessContainer& sequence) { // Overload of c_push_heap() for performing a push operation on a heap using a // `comp` other than `operator<`. -template -void c_push_heap(RandomAccessContainer& sequence, Compare&& comp) { +template +void c_push_heap(RandomAccessContainer& sequence, LessThan&& comp) { std::push_heap(container_algorithm_internal::c_begin(sequence), container_algorithm_internal::c_end(sequence), - std::forward(comp)); + std::forward(comp)); } // c_pop_heap() @@ -1410,11 +1410,11 @@ void c_pop_heap(RandomAccessContainer& sequence) { // Overload of c_pop_heap() for performing a pop operation on a heap using a // `comp` other than `operator<`. -template -void c_pop_heap(RandomAccessContainer& sequence, Compare&& comp) { +template +void c_pop_heap(RandomAccessContainer& sequence, LessThan&& comp) { std::pop_heap(container_algorithm_internal::c_begin(sequence), container_algorithm_internal::c_end(sequence), - std::forward(comp)); + std::forward(comp)); } // c_make_heap() @@ -1429,11 +1429,11 @@ void c_make_heap(RandomAccessContainer& sequence) { // Overload of c_make_heap() for performing heap comparisons using a // `comp` other than `operator<` -template -void c_make_heap(RandomAccessContainer& sequence, Compare&& comp) { +template +void c_make_heap(RandomAccessContainer& sequence, LessThan&& comp) { std::make_heap(container_algorithm_internal::c_begin(sequence), container_algorithm_internal::c_end(sequence), - std::forward(comp)); + std::forward(comp)); } // c_sort_heap() @@ -1448,11 +1448,11 @@ void c_sort_heap(RandomAccessContainer& sequence) { // Overload of c_sort_heap() for performing heap comparisons using a // `comp` other than `operator<` -template -void c_sort_heap(RandomAccessContainer& sequence, Compare&& comp) { +template +void c_sort_heap(RandomAccessContainer& sequence, LessThan&& comp) { std::sort_heap(container_algorithm_internal::c_begin(sequence), container_algorithm_internal::c_end(sequence), - std::forward(comp)); + std::forward(comp)); } // c_is_heap() @@ -1467,11 +1467,11 @@ bool c_is_heap(const RandomAccessContainer& sequence) { // Overload of c_is_heap() for performing heap comparisons using a // `comp` other than `operator<` -template -bool c_is_heap(const RandomAccessContainer& sequence, Compare&& comp) { +template +bool c_is_heap(const RandomAccessContainer& sequence, LessThan&& comp) { return std::is_heap(container_algorithm_internal::c_begin(sequence), container_algorithm_internal::c_end(sequence), - std::forward(comp)); + std::forward(comp)); } // c_is_heap_until() @@ -1487,12 +1487,12 @@ c_is_heap_until(RandomAccessContainer& sequence) { // Overload of c_is_heap_until() for performing heap comparisons using a // `comp` other than `operator<` -template +template container_algorithm_internal::ContainerIter -c_is_heap_until(RandomAccessContainer& sequence, Compare&& comp) { +c_is_heap_until(RandomAccessContainer& sequence, LessThan&& comp) { return std::is_heap_until(container_algorithm_internal::c_begin(sequence), container_algorithm_internal::c_end(sequence), - std::forward(comp)); + std::forward(comp)); } //------------------------------------------------------------------------------ @@ -1513,12 +1513,12 @@ container_algorithm_internal::ContainerIter c_min_element( // Overload of c_min_element() for performing a `comp` comparison other than // `operator<`. -template +template container_algorithm_internal::ContainerIter c_min_element( - Sequence& sequence, Compare&& comp) { + Sequence& sequence, LessThan&& comp) { return std::min_element(container_algorithm_internal::c_begin(sequence), container_algorithm_internal::c_end(sequence), - std::forward(comp)); + std::forward(comp)); } // c_max_element() @@ -1535,12 +1535,12 @@ container_algorithm_internal::ContainerIter c_max_element( // Overload of c_max_element() for performing a `comp` comparison other than // `operator<`. -template +template container_algorithm_internal::ContainerIter c_max_element( - Sequence& sequence, Compare&& comp) { + Sequence& sequence, LessThan&& comp) { return std::max_element(container_algorithm_internal::c_begin(sequence), container_algorithm_internal::c_end(sequence), - std::forward(comp)); + std::forward(comp)); } // c_minmax_element() @@ -1558,12 +1558,12 @@ c_minmax_element(C& c) { // Overload of c_minmax_element() for performing `comp` comparisons other than // `operator<`. -template +template container_algorithm_internal::ContainerIterPairType -c_minmax_element(C& c, Compare&& comp) { +c_minmax_element(C& c, LessThan&& comp) { return std::minmax_element(container_algorithm_internal::c_begin(c), container_algorithm_internal::c_end(c), - std::forward(comp)); + std::forward(comp)); } //------------------------------------------------------------------------------ @@ -1588,15 +1588,15 @@ bool c_lexicographical_compare(Sequence1&& sequence1, Sequence2&& sequence2) { // Overload of c_lexicographical_compare() for performing a lexicographical // comparison using a `comp` operator instead of `operator<`. -template +template bool c_lexicographical_compare(Sequence1&& sequence1, Sequence2&& sequence2, - Compare&& comp) { + LessThan&& comp) { return std::lexicographical_compare( container_algorithm_internal::c_begin(sequence1), container_algorithm_internal::c_end(sequence1), container_algorithm_internal::c_begin(sequence2), container_algorithm_internal::c_end(sequence2), - std::forward(comp)); + std::forward(comp)); } // c_next_permutation() @@ -1612,11 +1612,11 @@ bool c_next_permutation(C& c) { // Overload of c_next_permutation() for performing a lexicographical // comparison using a `comp` operator instead of `operator<`. -template -bool c_next_permutation(C& c, Compare&& comp) { +template +bool c_next_permutation(C& c, LessThan&& comp) { return std::next_permutation(container_algorithm_internal::c_begin(c), container_algorithm_internal::c_end(c), - std::forward(comp)); + std::forward(comp)); } // c_prev_permutation() @@ -1632,11 +1632,11 @@ bool c_prev_permutation(C& c) { // Overload of c_prev_permutation() for performing a lexicographical // comparison using a `comp` operator instead of `operator<`. -template -bool c_prev_permutation(C& c, Compare&& comp) { +template +bool c_prev_permutation(C& c, LessThan&& comp) { return std::prev_permutation(container_algorithm_internal::c_begin(c), container_algorithm_internal::c_end(c), - std::forward(comp)); + std::forward(comp)); } //------------------------------------------------------------------------------ diff --git a/absl/debugging/symbolize_elf.inc b/absl/debugging/symbolize_elf.inc index f4d5727bdeb..87dbd078b9c 100644 --- a/absl/debugging/symbolize_elf.inc +++ b/absl/debugging/symbolize_elf.inc @@ -701,6 +701,16 @@ static ABSL_ATTRIBUTE_NOINLINE FindSymbolResult FindSymbol( const char *start_address = ComputeOffset(original_start_address, relocation); +#ifdef __arm__ + // ARM functions are always aligned to multiples of two bytes; the + // lowest-order bit in start_address is ignored by the CPU and indicates + // whether the function contains ARM (0) or Thumb (1) code. We don't care + // about what encoding is being used; we just want the real start address + // of the function. + start_address = reinterpret_cast( + reinterpret_cast(start_address) & ~1); +#endif + if (deref_function_descriptor_pointer && InSection(original_start_address, opd)) { // The opd section is mapped into memory. Just dereference diff --git a/absl/debugging/symbolize_test.cc b/absl/debugging/symbolize_test.cc index a2dd4956c49..35de02e24b4 100644 --- a/absl/debugging/symbolize_test.cc +++ b/absl/debugging/symbolize_test.cc @@ -477,6 +477,46 @@ void ABSL_ATTRIBUTE_NOINLINE TestWithReturnAddress() { #endif } +#if defined(__arm__) && ABSL_HAVE_ATTRIBUTE(target) +// Test that we correctly identify bounds of Thumb functions on ARM. +// +// Thumb functions have the lowest-order bit set in their addresses in the ELF +// symbol table. This requires some extra logic to properly compute function +// bounds. To test this logic, nudge a Thumb function right up against an ARM +// function and try to symbolize the ARM function. +// +// A naive implementation will simply use the Thumb function's entry point as +// written in the symbol table and will therefore treat the Thumb function as +// extending one byte further in the instruction stream than it actually does. +// When asked to symbolize the start of the ARM function, it will identify an +// overlap between the Thumb and ARM functions, and it will return the name of +// the Thumb function. +// +// A correct implementation, on the other hand, will null out the lowest-order +// bit in the Thumb function's entry point. It will correctly compute the end of +// the Thumb function, it will find no overlap between the Thumb and ARM +// functions, and it will return the name of the ARM function. + +__attribute__((target("thumb"))) int ArmThumbOverlapThumb(int x) { + return x * x * x; +} + +__attribute__((target("arm"))) int ArmThumbOverlapArm(int x) { + return x * x * x; +} + +void ABSL_ATTRIBUTE_NOINLINE TestArmThumbOverlap() { +#if defined(ABSL_HAVE_ATTRIBUTE_NOINLINE) + const char *symbol = TrySymbolize((void *)&ArmThumbOverlapArm); + ABSL_RAW_CHECK(symbol != nullptr, "TestArmThumbOverlap failed"); + ABSL_RAW_CHECK(strcmp("ArmThumbOverlapArm()", symbol) == 0, + "TestArmThumbOverlap failed"); + std::cout << "TestArmThumbOverlap passed" << std::endl; +#endif +} + +#endif // defined(__arm__) && ABSL_HAVE_ATTRIBUTE(target) + #elif defined(_WIN32) #if !defined(ABSL_CONSUME_DLL) @@ -551,6 +591,9 @@ int main(int argc, char **argv) { TestWithPCInsideInlineFunction(); TestWithPCInsideNonInlineFunction(); TestWithReturnAddress(); +#if defined(__arm__) && ABSL_HAVE_ATTRIBUTE(target) + TestArmThumbOverlap(); +#endif #endif return RUN_ALL_TESTS(); diff --git a/absl/random/internal/pool_urbg.cc b/absl/random/internal/pool_urbg.cc index 5bee530770e..725100a4150 100644 --- a/absl/random/internal/pool_urbg.cc +++ b/absl/random/internal/pool_urbg.cc @@ -194,11 +194,10 @@ RandenPoolEntry* PoolAlignedAlloc() { // Not all the platforms that we build for have std::aligned_alloc, however // since we never free these objects, we can over allocate and munge the // pointers to the correct alignment. - void* memory = std::malloc(sizeof(RandenPoolEntry) + kAlignment); - auto x = reinterpret_cast(memory); + intptr_t x = reinterpret_cast( + new char[sizeof(RandenPoolEntry) + kAlignment]); auto y = x % kAlignment; - void* aligned = - (y == 0) ? memory : reinterpret_cast(x + kAlignment - y); + void* aligned = reinterpret_cast(y == 0 ? x : (x + kAlignment - y)); return new (aligned) RandenPoolEntry(); } diff --git a/absl/strings/BUILD.bazel b/absl/strings/BUILD.bazel index 1306ea65a9e..95ed08689b6 100644 --- a/absl/strings/BUILD.bazel +++ b/absl/strings/BUILD.bazel @@ -368,6 +368,7 @@ cc_library( ":cord_internal", ":cordz_handle", ":cordz_statistics", + ":cordz_update_tracker", "//absl/base:config", "//absl/base:core_headers", "//absl/debugging:stacktrace", @@ -406,6 +407,7 @@ cc_library( hdrs = ["internal/cordz_statistics.h"], copts = ABSL_DEFAULT_COPTS, deps = [ + ":cordz_update_tracker", "//absl/base:config", ], ) @@ -450,6 +452,8 @@ cc_test( ":cord_internal", ":cordz_handle", ":cordz_info", + ":cordz_statistics", + ":cordz_update_tracker", ":strings", "//absl/base:config", "//absl/debugging:stacktrace", diff --git a/absl/strings/CMakeLists.txt b/absl/strings/CMakeLists.txt index 8f41f0645d8..5a13831fbb1 100644 --- a/absl/strings/CMakeLists.txt +++ b/absl/strings/CMakeLists.txt @@ -641,6 +641,7 @@ absl_cc_library( DEPS absl::config absl::core_headers + absl::cordz_update_tracker absl::synchronization ) @@ -690,6 +691,7 @@ absl_cc_library( absl::cord_internal absl::cordz_handle absl::cordz_statistics + absl::cordz_update_tracker absl::core_headers absl::span absl::stacktrace @@ -707,6 +709,8 @@ absl_cc_test( absl::cord_test_helpers absl::cordz_handle absl::cordz_info + absl::cordz_statistics + absl::cordz_update_tracker absl::span absl::stacktrace absl::symbolize diff --git a/absl/strings/cord.cc b/absl/strings/cord.cc index d33d6a0d45b..770c5a705cc 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_tracker.h" #include "absl/strings/internal/resize_uninitialized.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" @@ -544,7 +545,10 @@ Cord::Cord(absl::string_view src) { if (n <= InlineRep::kMaxInline) { contents_.set_data(src.data(), n, false); } else { - contents_.set_tree(NewTree(src.data(), n, 0)); + contents_.data_.make_tree(NewTree(src.data(), n, 0)); + if (ABSL_PREDICT_FALSE(absl::cord_internal::cordz_should_profile())) { + contents_.StartProfiling(); + } } } diff --git a/absl/strings/cord.h b/absl/strings/cord.h index de1470b2c20..1e6a73a5c6f 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_tracker.h" #include "absl/strings/internal/resize_uninitialized.h" #include "absl/strings/internal/string_constant.h" #include "absl/strings/string_view.h" @@ -668,6 +669,8 @@ class Cord { explicit constexpr Cord(strings_internal::StringConstant); private: + using CordzUpdateTracker = cord_internal::CordzUpdateTracker; + friend class CordTestPeer; friend bool operator==(const Cord& lhs, const Cord& rhs); friend bool operator==(const Cord& lhs, absl::string_view rhs); diff --git a/absl/strings/internal/cordz_info.cc b/absl/strings/internal/cordz_info.cc index 4dec63d5dec..6540d134562 100644 --- a/absl/strings/internal/cordz_info.cc +++ b/absl/strings/internal/cordz_info.cc @@ -19,6 +19,7 @@ #include "absl/strings/internal/cord_internal.h" #include "absl/strings/internal/cordz_handle.h" #include "absl/strings/internal/cordz_statistics.h" +#include "absl/strings/internal/cordz_update_tracker.h" #include "absl/synchronization/mutex.h" #include "absl/types/span.h" @@ -44,18 +45,15 @@ CordzInfo* CordzInfo::Next(const CordzSnapshot& snapshot) const { return ci_next_unsafe(); } -CordzInfo* CordzInfo::TrackCord(CordRep* rep, const CordzInfo* src) { - CordzInfo* ci = new CordzInfo(rep); - if (src) { - ci->parent_stack_depth_ = src->stack_depth_; - memcpy(ci->parent_stack_, src->stack_, sizeof(void*) * src->stack_depth_); - } +CordzInfo* CordzInfo::TrackCord(CordRep* rep, const CordzInfo* src, + MethodIdentifier method) { + CordzInfo* ci = new CordzInfo(rep, src, method); ci->Track(); return ci; } -CordzInfo* CordzInfo::TrackCord(CordRep* rep) { - return TrackCord(rep, nullptr); +CordzInfo* CordzInfo::TrackCord(CordRep* rep, MethodIdentifier method) { + return TrackCord(rep, nullptr, method); } void CordzInfo::UntrackCord(CordzInfo* cordz_info) { @@ -66,12 +64,35 @@ void CordzInfo::UntrackCord(CordzInfo* cordz_info) { } } -CordzInfo::CordzInfo(CordRep* rep) +CordzInfo::MethodIdentifier CordzInfo::GetParentMethod(const CordzInfo* src) { + if (src == nullptr) return MethodIdentifier::kUnknown; + return src->parent_method_ != MethodIdentifier::kUnknown ? src->parent_method_ + : src->method_; +} + +int CordzInfo::FillParentStack(const CordzInfo* src, void** stack) { + assert(stack); + if (src == nullptr) return 0; + if (src->parent_stack_depth_) { + memcpy(stack, src->parent_stack_, src->parent_stack_depth_ * sizeof(void*)); + return src->parent_stack_depth_; + } + memcpy(stack, src->stack_, src->stack_depth_ * sizeof(void*)); + return src->stack_depth_; +} + +CordzInfo::CordzInfo(CordRep* rep, const CordzInfo* src, + MethodIdentifier method) : rep_(rep), stack_depth_(absl::GetStackTrace(stack_, /*max_depth=*/kMaxStackDepth, /*skip_count=*/1)), - parent_stack_depth_(0), - create_time_(absl::Now()) {} + parent_stack_depth_(FillParentStack(src, parent_stack_)), + method_(method), + parent_method_(GetParentMethod(src)), + create_time_(absl::Now()), + size_(rep->length) { + update_tracker_.LossyAdd(method); +} CordzInfo::~CordzInfo() { // `rep_` is potentially kept alive if CordzInfo is included @@ -96,7 +117,7 @@ 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()); + absl::MutexLock lock(&mutex_); rep_ = nullptr; } @@ -120,9 +141,31 @@ void CordzInfo::Untrack() { } } +void CordzInfo::Lock(MethodIdentifier method) + ABSL_EXCLUSIVE_LOCK_FUNCTION(mutex_) { + mutex_.Lock(); + update_tracker_.LossyAdd(method); + assert(rep_); +} + +void CordzInfo::Unlock() ABSL_UNLOCK_FUNCTION(mutex_) { + bool tracked = rep_ != nullptr; + if (rep_) { + size_.store(rep_->length); + } + mutex_.Unlock(); + if (!tracked) { + Untrack(); + CordzHandle::Delete(this); + } +} + void CordzInfo::SetCordRep(CordRep* rep) { - mutex().AssertHeld(); + mutex_.AssertHeld(); rep_ = rep; + if (rep) { + size_.store(rep->length); + } } absl::Span CordzInfo::GetStack() const { @@ -133,6 +176,15 @@ absl::Span CordzInfo::GetParentStack() const { return absl::MakeConstSpan(parent_stack_, parent_stack_depth_); } +CordzStatistics CordzInfo::GetCordzStatistics() const { + CordzStatistics stats; + stats.method = method_; + stats.parent_method = parent_method_; + stats.update_tracker = update_tracker_; + stats.size = size_.load(std::memory_order_relaxed); + return stats; +} + } // namespace cord_internal ABSL_NAMESPACE_END } // namespace absl diff --git a/absl/strings/internal/cordz_info.h b/absl/strings/internal/cordz_info.h index c1090a1cf19..5345be75b3f 100644 --- a/absl/strings/internal/cordz_info.h +++ b/absl/strings/internal/cordz_info.h @@ -24,6 +24,7 @@ #include "absl/strings/internal/cord_internal.h" #include "absl/strings/internal/cordz_handle.h" #include "absl/strings/internal/cordz_statistics.h" +#include "absl/strings/internal/cordz_update_tracker.h" #include "absl/synchronization/mutex.h" #include "absl/types/span.h" @@ -42,13 +43,20 @@ namespace cord_internal { // the destructor of a CordzSampleToken object. class 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. - static CordzInfo* TrackCord(CordRep* rep); + // 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); // Stops tracking changes for a sampled cord, and deletes the provided info. // This function must be called before the sampled cord instance is deleted, @@ -58,10 +66,12 @@ class CordzInfo : public CordzHandle { static void UntrackCord(CordzInfo* cordz_info); // Identical to TrackCord(), except that this function fills the - // 'parent_stack' property of the returned CordzInfo instance from the - // provided `src` instance if `src` is not null. + // `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); + static CordzInfo* TrackCord( + CordRep* rep, const CordzInfo* src, + MethodIdentifier method = MethodIdentifier::kUnknown); CordzInfo() = delete; CordzInfo(const CordzInfo&) = delete; @@ -73,17 +83,20 @@ class CordzInfo : public CordzHandle { // Retrieves the next oldest existing CordzInfo older than 'this' instance. CordzInfo* Next(const CordzSnapshot& snapshot) const; - // Returns a reference to the mutex guarding the `rep` property of this - // instance. CordzInfo instances hold a weak reference to the rep pointer of - // sampled cords, and rely on Cord logic to update the rep pointer when the - // underlying root tree or ring of the cord changes. - absl::Mutex& mutex() const { return mutex_; } + // Locks this instance for the update identified by `method`. + // Increases the count for `method` in `update_tracker`. + void Lock(MethodIdentifier method) ABSL_EXCLUSIVE_LOCK_FUNCTION(mutex_); + + // Unlocks this instance. If the contained `rep` has been set to null + // indicating the Cord has been cleared or is otherwise no longer sampled, + // then this method will delete this CordzInfo instance. + void Unlock() ABSL_UNLOCK_FUNCTION(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 // code can always safely take a reference on the tracked cord. - // Requires `mutex()` to be held. + // Requires a lock to be held through the `Lock()` method. // TODO(b/117940323): annotate with ABSL_EXCLUSIVE_LOCKS_REQUIRED once all // Cord code is in a state where this can be proven true by the compiler. void SetCordRep(CordRep* rep); @@ -106,15 +119,10 @@ class CordzInfo : public CordzHandle { // from, or being assigned the value of an existing (sampled) cord. absl::Span GetParentStack() const; - // Retrieve the CordzStatistics associated with this Cord. The statistics are - // only updated when a Cord goes through a mutation, such as an Append or - // RemovePrefix. The refcounts can change due to external events, so the - // reported refcount stats might be incorrect. - CordzStatistics GetCordzStatistics() const { - CordzStatistics stats; - stats.size = size_.load(std::memory_order_relaxed); - return stats; - } + // Retrieves the CordzStatistics associated with this Cord. The statistics + // are only updated when a Cord goes through a mutation, such as an Append + // or RemovePrefix. + CordzStatistics GetCordzStatistics() const; // Records size metric for this CordzInfo instance. void RecordMetrics(int64_t size) { @@ -124,9 +132,21 @@ class CordzInfo : public CordzHandle { private: static constexpr int kMaxStackDepth = 64; - explicit CordzInfo(CordRep* tree); + explicit CordzInfo(CordRep* rep, const CordzInfo* src, + MethodIdentifier method); ~CordzInfo() override; + // 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. + static MethodIdentifier GetParentMethod(const CordzInfo* src); + + // Fills the provided stack from `src`, copying either `parent_stack_` or + // `stack_` depending on `parent_stack_` being empty, returning the size of + // the parent stack. + // Returns 0 if `src` is null. + static int FillParentStack(const CordzInfo* src, void** stack); + void Track(); void Untrack(); @@ -149,12 +169,15 @@ class CordzInfo : public CordzHandle { std::atomic ci_next_ ABSL_GUARDED_BY(ci_mutex_){nullptr}; mutable absl::Mutex mutex_; - CordRep* rep_ ABSL_GUARDED_BY(mutex()); + CordRep* rep_ ABSL_GUARDED_BY(mutex_); void* stack_[kMaxStackDepth]; void* parent_stack_[kMaxStackDepth]; const int stack_depth_; - int parent_stack_depth_; + const int parent_stack_depth_; + const MethodIdentifier method_; + const MethodIdentifier parent_method_; + CordzUpdateTracker update_tracker_; const absl::Time create_time_; // Last recorded size for the cord. diff --git a/absl/strings/internal/cordz_info_test.cc b/absl/strings/internal/cordz_info_test.cc index be20e4a4cc8..66acf9b61fa 100644 --- a/absl/strings/internal/cordz_info_test.cc +++ b/absl/strings/internal/cordz_info_test.cc @@ -23,6 +23,7 @@ #include "absl/debugging/symbolize.h" #include "absl/strings/internal/cord_rep_flat.h" #include "absl/strings/internal/cordz_handle.h" +#include "absl/strings/internal/cordz_update_tracker.h" #include "absl/strings/str_cat.h" #include "absl/types/span.h" @@ -47,6 +48,12 @@ struct TestCordRep { ~TestCordRep() { CordRepFlat::Delete(rep); } }; +// Used test values +auto constexpr kUnknownMethod = CordzUpdateTracker::kUnknown; +auto constexpr kTrackCordMethod = CordzUpdateTracker::kConstructorString; +auto constexpr kChildMethod = CordzUpdateTracker::kConstructorCord; +auto constexpr kUpdateMethod = CordzUpdateTracker::kAppendString; + // Local less verbose helper std::vector DeleteQueue() { return CordzHandle::DiagnosticsGetDeleteQueue(); @@ -90,15 +97,27 @@ TEST(CordzInfoTest, SetCordRep) { CordzInfo* info = CordzInfo::TrackCord(rep.rep); TestCordRep rep2; - { - absl::MutexLock lock(&info->mutex()); - info->SetCordRep(rep2.rep); - } + info->Lock(CordzUpdateTracker::kAppendCord); + info->SetCordRep(rep2.rep); + info->Unlock(); EXPECT_THAT(info->GetCordRepForTesting(), Eq(rep2.rep)); CordzInfo::UntrackCord(info); } +TEST(CordzInfoTest, SetCordRepNullUntracksCordOnUnlock) { + TestCordRep rep; + CordzInfo* info = CordzInfo::TrackCord(rep.rep); + + info->Lock(CordzUpdateTracker::kAppendString); + info->SetCordRep(nullptr); + EXPECT_THAT(info->GetCordRepForTesting(), Eq(nullptr)); + EXPECT_THAT(CordzInfo::Head(CordzSnapshot()), Eq(info)); + + info->Unlock(); + EXPECT_THAT(CordzInfo::Head(CordzSnapshot()), Eq(nullptr)); +} + #if GTEST_HAS_DEATH_TEST TEST(CordzInfoTest, SetCordRepRequiresMutex) { @@ -191,13 +210,41 @@ 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); + return CordzInfo::TrackCord(rep, parent, kChildMethod); } CordzInfo* TrackParentCord(CordRep* rep) { - return CordzInfo::TrackCord(rep); + return CordzInfo::TrackCord(rep, kTrackCordMethod); } -TEST(CordzInfoTest, ParentStackV2) { +TEST(CordzInfoTest, GetStatistics) { + TestCordRep rep; + CordzInfo* info = TrackParentCord(rep.rep); + + CordzStatistics statistics = info->GetCordzStatistics(); + EXPECT_THAT(statistics.size, Eq(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)); + + CordzInfo::UntrackCord(info); +} + +TEST(CordzInfoTest, LockCountsMethod) { + TestCordRep rep; + CordzInfo* info = TrackParentCord(rep.rep); + + info->Lock(kUpdateMethod); + info->Unlock(); + info->Lock(kUpdateMethod); + info->Unlock(); + + CordzStatistics statistics = info->GetCordzStatistics(); + EXPECT_THAT(statistics.update_tracker.Value(kUpdateMethod), Eq(2)); + + CordzInfo::UntrackCord(info); +} + +TEST(CordzInfoTest, FromParent) { TestCordRep rep; CordzInfo* info_parent = TrackParentCord(rep.rep); CordzInfo* info_child = TrackChildCord(rep.rep, info_parent); @@ -206,18 +253,29 @@ TEST(CordzInfoTest, ParentStackV2) { 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.method, Eq(kChildMethod)); + EXPECT_THAT(statistics.parent_method, Eq(kTrackCordMethod)); + EXPECT_THAT(statistics.update_tracker.Value(kChildMethod), Eq(1)); + CordzInfo::UntrackCord(info_parent); CordzInfo::UntrackCord(info_child); } -TEST(CordzInfoTest, ParentStackEmpty) { +TEST(CordzInfoTest, FromParentNullptr) { TestCordRep rep; CordzInfo* info = TrackChildCord(rep.rep, nullptr); EXPECT_TRUE(info->GetParentStack().empty()); + CordzStatistics statistics = info->GetCordzStatistics(); + EXPECT_THAT(statistics.size, Eq(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)); CordzInfo::UntrackCord(info); } -TEST(CordzInfoTest, CordzStatisticsV2) { +TEST(CordzInfoTest, RecordMetrics) { TestCordRep rep; CordzInfo* info = TrackParentCord(rep.rep); diff --git a/absl/strings/internal/cordz_statistics.h b/absl/strings/internal/cordz_statistics.h index ce7c39aa13d..6e335c0b86c 100644 --- a/absl/strings/internal/cordz_statistics.h +++ b/absl/strings/internal/cordz_statistics.h @@ -18,6 +18,7 @@ #include #include "absl/base/config.h" +#include "absl/strings/internal/cordz_update_tracker.h" namespace absl { ABSL_NAMESPACE_BEGIN @@ -25,6 +26,8 @@ namespace cord_internal { // CordzStatistics captures some meta information about a Cord's shape. struct CordzStatistics { + using MethodIdentifier = CordzUpdateTracker::MethodIdentifier; + // The size of the cord in bytes. This matches the result of Cord::size(). int64_t size = 0; @@ -46,6 +49,15 @@ struct CordzStatistics { // For ring buffer Cords, this includes the 'ring buffer' node. // A value of 0 implies the property has not been recorded. int64_t node_count = 0; + + // The cord method responsible for sampling the cord. + MethodIdentifier method = MethodIdentifier::kUnknown; + + // The cord method responsible for sampling the parent cord if applicable. + MethodIdentifier parent_method = MethodIdentifier::kUnknown; + + // Update tracker tracking invocation count per cord method. + CordzUpdateTracker update_tracker; }; } // namespace cord_internal diff --git a/absl/strings/internal/cordz_update_tracker.h b/absl/strings/internal/cordz_update_tracker.h index 3c617e93986..a090676dc74 100644 --- a/absl/strings/internal/cordz_update_tracker.h +++ b/absl/strings/internal/cordz_update_tracker.h @@ -38,20 +38,24 @@ class CordzUpdateTracker { public: // Tracked update methods. enum MethodIdentifier { - kAssignString, - kAssignCord, - kMoveAssignCord, - kAppendString, + kUnknown, kAppendCord, - kMoveAppendCord, - kPrependString, - kPrependCord, - kMovePrependCord, kAppendExternalMemory, + kAppendString, + kAssignCord, + kAssignString, + kClear, + kConstructorCord, + kConstructorString, kFlatten, kGetAppendRegion, + kMoveAppendCord, + kMoveAssignCord, + kMovePrependCord, + kPrependCord, + kPrependString, kRemovePrefix, - kRemoveSuffic, + kRemoveSuffix, kSubCord, // kNumMethods defines the number of entries: must be the last entry. diff --git a/absl/strings/internal/cordz_update_tracker_test.cc b/absl/strings/internal/cordz_update_tracker_test.cc index 45782046af1..eda662f2fe1 100644 --- a/absl/strings/internal/cordz_update_tracker_test.cc +++ b/absl/strings/internal/cordz_update_tracker_test.cc @@ -36,13 +36,24 @@ using Methods = std::array; // Returns an array of all methods defined in `MethodIdentifier` Methods AllMethods() { - return Methods{Method::kAssignString, Method::kAssignCord, - Method::kMoveAssignCord, Method::kAppendString, - Method::kAppendCord, Method::kMoveAppendCord, - Method::kPrependString, Method::kPrependCord, - Method::kMovePrependCord, Method::kAppendExternalMemory, - Method::kFlatten, Method::kGetAppendRegion, - Method::kRemovePrefix, Method::kRemoveSuffic, + return Methods{Method::kUnknown, + Method::kAppendCord, + Method::kAppendExternalMemory, + Method::kAppendString, + Method::kAssignCord, + Method::kAssignString, + Method::kClear, + Method::kConstructorCord, + Method::kConstructorString, + Method::kFlatten, + Method::kGetAppendRegion, + Method::kMoveAppendCord, + Method::kMoveAssignCord, + Method::kMovePrependCord, + Method::kPrependCord, + Method::kPrependString, + Method::kRemovePrefix, + Method::kRemoveSuffix, Method::kSubCord}; }