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
wijagels · Apr 22, 2021 · e38e1aa · e38e1aa
1 parent 1ae9b71
commit e38e1aa
Show file tree

Hide file tree

Showing 14 changed files with 427 additions and 165 deletions.
diff --git 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
@@ -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
@@ -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",
@@ -377,6 +379,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"],

diff --git 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
@@ -754,6 +755,38 @@ absl_cc_test(
     gmock_main
 )
 
+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
@@ -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
@@ -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
@@ -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
@@ -329,13 +329,17 @@ 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
   // byte of cordz_info overlaps with the last byte holding the tag.
   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
@@ -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_);
 }