Export of internal Abseil changes
--
a78c34c705b15598ea00171d4ff8755e38fad863 by Derek Mauro <dmauro@google.com>:
Improve compiler support for ABSL_FALLTHROUGH_INTENDED
PiperOrigin-RevId: 370952333
--
aed0c26f1a2aac98e217ad1b44ce4a858780a223 by Martijn Vels <mvels@google.com>:
Add Cordz instrumentation for Flatten
PiperOrigin-RevId: 370815149
--
ff4a58d0109d39dc32ef7a5e5e669ca4e630c6d9 by Martijn Vels <mvels@google.com>:
Add Cordz instrumentation to RemovePrefix and RemoveSuffix
PiperOrigin-RevId: 370751602
--
40220a058b30ddd89c6e547591488d15342137dd by Martijn Vels <mvels@google.com>:
Add Cordz instrumentation to operator=(string_view)
PiperOrigin-RevId: 370737600
--
a2e49604f18b92e50b179b5477dfddb8f57538ca by Martijn Vels <mvels@google.com>:
Add cordz instrumentation for Subcord
PiperOrigin-RevId: 370724107
--
bcc3902e04fb4f14270aef00e18908e6a88474cd by Derek Mauro <dmauro@google.com>:
Internal change
PiperOrigin-RevId: 370707219
GitOrigin-RevId: a78c34c705b15598ea00171d4ff8755e38fad863
Change-Id: I0270e536cbdeaf1f195199da822b314521de3b96
diff --git a/absl/base/attributes.h b/absl/base/attributes.h
index d710f28..5213955 100644
--- a/absl/base/attributes.h
+++ b/absl/base/attributes.h
@@ -599,31 +599,24 @@
// case 42:
// ...
//
-// Notes: when compiled with clang in C++11 mode, the ABSL_FALLTHROUGH_INTENDED
-// macro is expanded to the [[clang::fallthrough]] attribute, which is analysed
-// when performing switch labels fall-through diagnostic
-// (`-Wimplicit-fallthrough`). See clang documentation on language extensions
-// for details:
+// Notes: When supported, GCC and Clang can issue a warning on switch labels
+// with unannotated fallthrough using the warning `-Wimplicit-fallthrough`. See
+// clang documentation on language extensions for details:
// https://clang.llvm.org/docs/AttributeReference.html#fallthrough-clang-fallthrough
//
-// When used with unsupported compilers, the ABSL_FALLTHROUGH_INTENDED macro
-// has no effect on diagnostics. In any case this macro has no effect on runtime
+// When used with unsupported compilers, the ABSL_FALLTHROUGH_INTENDED macro has
+// no effect on diagnostics. In any case this macro has no effect on runtime
// behavior and performance of code.
#ifdef ABSL_FALLTHROUGH_INTENDED
#error "ABSL_FALLTHROUGH_INTENDED should not be defined."
-#endif
-
-// TODO(zhangxy): Use c++17 standard [[fallthrough]] macro, when supported.
-#if defined(__clang__) && defined(__has_warning)
-#if __has_feature(cxx_attributes) && __has_warning("-Wimplicit-fallthrough")
+#elif ABSL_HAVE_CPP_ATTRIBUTE(fallthrough)
+#define ABSL_FALLTHROUGH_INTENDED [[fallthrough]]
+#elif ABSL_HAVE_CPP_ATTRIBUTE(clang::fallthrough)
#define ABSL_FALLTHROUGH_INTENDED [[clang::fallthrough]]
-#endif
-#elif ABSL_INTERNAL_HAVE_MIN_GNUC_VERSION(7, 0)
+#elif ABSL_HAVE_CPP_ATTRIBUTE(gnu::fallthrough)
#define ABSL_FALLTHROUGH_INTENDED [[gnu::fallthrough]]
-#endif
-
-#ifndef ABSL_FALLTHROUGH_INTENDED
+#else
#define ABSL_FALLTHROUGH_INTENDED \
do { \
} while (0)
diff --git a/absl/strings/BUILD.bazel b/absl/strings/BUILD.bazel
index 68c71f0..ae88f7b 100644
--- a/absl/strings/BUILD.bazel
+++ b/absl/strings/BUILD.bazel
@@ -544,6 +544,7 @@
":cordz_sample_token",
":cordz_statistics",
":cordz_update_tracker",
+ ":strings",
"//absl/base:config",
"//absl/base:core_headers",
"@com_google_googletest//:gtest",
diff --git a/absl/strings/CMakeLists.txt b/absl/strings/CMakeLists.txt
index 80ae2a6..8ebe91e 100644
--- a/absl/strings/CMakeLists.txt
+++ b/absl/strings/CMakeLists.txt
@@ -852,6 +852,7 @@
absl::cordz_statistics
absl::cordz_update_tracker
absl::core_headers
+ absl::strings
TESTONLY
)
diff --git a/absl/strings/cord.cc b/absl/strings/cord.cc
index 15d1733..9262aee 100644
--- a/absl/strings/cord.cc
+++ b/absl/strings/cord.cc
@@ -584,26 +584,35 @@
}
Cord& Cord::operator=(absl::string_view src) {
+ auto constexpr method = CordzUpdateTracker::kAssignString;
const char* data = src.data();
size_t length = src.size();
CordRep* tree = contents_.tree();
if (length <= InlineRep::kMaxInline) {
- // Embed into this->contents_
- if (tree) CordzInfo::MaybeUntrackCord(contents_.cordz_info());
+ // Embed into this->contents_, which is somewhat subtle:
+ // - MaybeUntrackCord must be called before Unref(tree).
+ // - MaybeUntrackCord must be called before set_data() clobbers cordz_info.
+ // - set_data() must be called before Unref(tree) as it may reference tree.
+ if (tree != nullptr) CordzInfo::MaybeUntrackCord(contents_.cordz_info());
contents_.set_data(data, length, true);
- if (tree) CordRep::Unref(tree);
+ if (tree != nullptr) CordRep::Unref(tree);
return *this;
}
- if (tree != nullptr && tree->tag >= FLAT &&
- tree->flat()->Capacity() >= length && tree->refcount.IsOne()) {
- // Copy in place if the existing FLAT node is reusable.
- memmove(tree->flat()->Data(), data, length);
- tree->length = length;
- VerifyTree(tree);
- return *this;
+ if (tree != nullptr) {
+ CordzUpdateScope scope(contents_.cordz_info(), method);
+ if (tree->tag >= FLAT && tree->flat()->Capacity() >= length &&
+ tree->refcount.IsOne()) {
+ // Copy in place if the existing FLAT node is reusable.
+ memmove(tree->flat()->Data(), data, length);
+ tree->length = length;
+ VerifyTree(tree);
+ return *this;
+ }
+ contents_.SetTree(NewTree(data, length, 0), scope);
+ CordRep::Unref(tree);
+ } else {
+ contents_.EmplaceTree(NewTree(data, length, 0), method);
}
- contents_.set_tree(NewTree(data, length, 0));
- if (tree) CordRep::Unref(tree);
return *this;
}
@@ -893,12 +902,17 @@
CordRep* tree = contents_.tree();
if (tree == nullptr) {
contents_.remove_prefix(n);
- } else if (tree->tag == RING) {
- contents_.replace_tree(CordRepRing::RemovePrefix(tree->ring(), n));
} else {
- CordRep* newrep = RemovePrefixFrom(tree, n);
- CordRep::Unref(tree);
- contents_.replace_tree(VerifyTree(newrep));
+ auto constexpr method = CordzUpdateTracker::kRemovePrefix;
+ CordzUpdateScope scope(contents_.cordz_info(), method);
+ if (tree->tag == RING) {
+ tree = CordRepRing::RemovePrefix(tree->ring(), n);
+ } else {
+ CordRep* newrep = RemovePrefixFrom(tree, n);
+ CordRep::Unref(tree);
+ tree = VerifyTree(newrep);
+ }
+ contents_.SetTreeOrEmpty(tree, scope);
}
}
@@ -909,12 +923,17 @@
CordRep* tree = contents_.tree();
if (tree == nullptr) {
contents_.reduce_size(n);
- } else if (tree->tag == RING) {
- contents_.replace_tree(CordRepRing::RemoveSuffix(tree->ring(), n));
} else {
- CordRep* newrep = RemoveSuffixFrom(tree, n);
- CordRep::Unref(tree);
- contents_.replace_tree(VerifyTree(newrep));
+ auto constexpr method = CordzUpdateTracker::kRemoveSuffix;
+ CordzUpdateScope scope(contents_.cordz_info(), method);
+ if (tree->tag == RING) {
+ tree = CordRepRing::RemoveSuffix(tree->ring(), n);
+ } else {
+ CordRep* newrep = RemoveSuffixFrom(tree, n);
+ CordRep::Unref(tree);
+ tree = VerifyTree(newrep);
+ }
+ contents_.SetTreeOrEmpty(tree, scope);
}
}
@@ -969,37 +988,51 @@
return results[0];
}
+void Cord::CopyDataAtPosition(size_t pos, size_t new_size, char* dest) const {
+ assert(new_size <= cord_internal::kMaxInline);
+ assert(pos <= size());
+ assert(new_size <= size() - pos);
+ Cord::ChunkIterator it = chunk_begin();
+ it.AdvanceBytes(pos);
+ size_t remaining_size = new_size;
+ while (remaining_size > it->size()) {
+ cord_internal::SmallMemmove(dest, it->data(), it->size());
+ remaining_size -= it->size();
+ dest += it->size();
+ ++it;
+ }
+ cord_internal::SmallMemmove(dest, it->data(), remaining_size);
+}
+
Cord Cord::Subcord(size_t pos, size_t new_size) const {
Cord sub_cord;
size_t length = size();
if (pos > length) pos = length;
if (new_size > length - pos) new_size = length - pos;
+ if (new_size == 0) return sub_cord;
+
CordRep* tree = contents_.tree();
if (tree == nullptr) {
// sub_cord is newly constructed, no need to re-zero-out the tail of
// contents_ memory.
sub_cord.contents_.set_data(contents_.data() + pos, new_size, false);
- } else if (new_size == 0) {
- // We want to return empty subcord, so nothing to do.
- } else if (new_size <= InlineRep::kMaxInline) {
- Cord::ChunkIterator it = chunk_begin();
- it.AdvanceBytes(pos);
- char* dest = sub_cord.contents_.data_.as_chars();
- size_t remaining_size = new_size;
- while (remaining_size > it->size()) {
- cord_internal::SmallMemmove(dest, it->data(), it->size());
- remaining_size -= it->size();
- dest += it->size();
- ++it;
- }
- cord_internal::SmallMemmove(dest, it->data(), remaining_size);
- sub_cord.contents_.set_inline_size(new_size);
- } else if (tree->tag == RING) {
- tree = CordRepRing::SubRing(CordRep::Ref(tree)->ring(), pos, new_size);
- sub_cord.contents_.set_tree(tree);
- } else {
- sub_cord.contents_.set_tree(NewSubRange(tree, pos, new_size));
+ return sub_cord;
}
+
+ if (new_size <= InlineRep::kMaxInline) {
+ CopyDataAtPosition(pos, new_size, sub_cord.contents_.data_.as_chars());
+ sub_cord.contents_.set_inline_size(new_size);
+ return sub_cord;
+ }
+
+ if (tree->tag == RING) {
+ CordRepRing* ring = CordRep::Ref(tree)->ring();
+ tree = CordRepRing::SubRing(ring, pos, new_size);
+ } else {
+ tree = NewSubRange(tree, pos, new_size);
+ }
+ sub_cord.contents_.EmplaceTree(tree, contents_.data_,
+ CordzUpdateTracker::kSubCord);
return sub_cord;
}
@@ -1676,6 +1709,7 @@
}
absl::string_view Cord::FlattenSlowPath() {
+ assert(contents_.is_tree());
size_t total_size = size();
CordRep* new_rep;
char* new_buffer;
@@ -1696,10 +1730,9 @@
s.size());
});
}
- if (CordRep* tree = contents_.tree()) {
- CordRep::Unref(tree);
- }
- contents_.set_tree(new_rep);
+ CordzUpdateScope scope(contents_.cordz_info(), CordzUpdateTracker::kFlatten);
+ CordRep::Unref(contents_.as_tree());
+ contents_.SetTree(new_rep, scope);
return absl::string_view(new_buffer, total_size);
}
diff --git a/absl/strings/cord.h b/absl/strings/cord.h
index d5a13b3..d7b4324 100644
--- a/absl/strings/cord.h
+++ b/absl/strings/cord.h
@@ -692,6 +692,10 @@
// called by Flatten() when the cord was not already flat.
absl::string_view FlattenSlowPath();
+ // Copies `new_size` bytes starting at `pos` into `dest`. Requires at least
+ // `new_size` bytes to be available, and `new_size` to be <= kMaxInline.
+ void CopyDataAtPosition(size_t pos, size_t new_size, char* dest) const;
+
// Actual cord contents are hidden inside the following simple
// class so that we can isolate the bulk of cord.cc from changes
// to the representation.
@@ -745,12 +749,21 @@
// the CordzInfo instance is updated to reference the new `rep` value.
void SetTree(CordRep* rep, const CordzUpdateScope& scope);
+ // Identical to SetTree(), except that `rep` is allowed to be null, in
+ // which case the current instance is reset to an empty value.
+ void SetTreeOrEmpty(CordRep* rep, const CordzUpdateScope& scope);
+
// Sets the tree value for this instance, and randomly samples this cord.
// This function disregards existing contents in `data_`, and should be
// called when a Cord is 'promoted' from an 'uninitialized' or 'inlined'
// value to a non-inlined (tree / ring) value.
void EmplaceTree(CordRep* rep, MethodIdentifier method);
+ // Identical to EmplaceTree, except that it copies the parent stack from
+ // the provided `parent` data if the parent is sampled.
+ void EmplaceTree(CordRep* rep, const InlineData& parent,
+ MethodIdentifier method);
+
// Commits the change of a newly created, or updated `rep` root value into
// this cord. `old_rep` indicates the old (inlined or tree) value of the
// cord, and determines if the commit invokes SetTree() or EmplaceTree().
@@ -1071,6 +1084,12 @@
CordzInfo::MaybeTrackCord(data_, method);
}
+inline void Cord::InlineRep::EmplaceTree(CordRep* rep, const InlineData& parent,
+ MethodIdentifier method) {
+ data_.make_tree(rep);
+ CordzInfo::MaybeTrackCord(data_, parent, method);
+}
+
inline void Cord::InlineRep::SetTree(CordRep* rep,
const CordzUpdateScope& scope) {
assert(rep);
@@ -1079,6 +1098,17 @@
scope.SetCordRep(rep);
}
+inline void Cord::InlineRep::SetTreeOrEmpty(CordRep* rep,
+ const CordzUpdateScope& scope) {
+ assert(data_.is_tree());
+ if (rep) {
+ data_.set_tree(rep);
+ } else {
+ data_ = {};
+ }
+ scope.SetCordRep(rep);
+}
+
inline void Cord::InlineRep::CommitTree(const CordRep* old_rep, CordRep* rep,
const CordzUpdateScope& scope,
MethodIdentifier method) {
diff --git a/absl/strings/cordz_test.cc b/absl/strings/cordz_test.cc
index b16e968..84947cf 100644
--- a/absl/strings/cordz_test.cc
+++ b/absl/strings/cordz_test.cc
@@ -52,6 +52,8 @@
namespace {
+auto constexpr kMaxInline = cord_internal::kMaxInline;
+
// Returns a string_view value of the specified length
// We do this to avoid 'consuming' large strings in Cord by default.
absl::string_view MakeString(size_t size) {
@@ -139,6 +141,16 @@
EXPECT_THAT(cord, HasValidCordzInfoOf(Method::kConstructorString));
}
+TEST_P(CordzUpdateTest, AssignSmallArray) {
+ cord() = MakeString(TestCordSize::kSmall);
+ EXPECT_THAT(cord(), HasValidCordzInfoOf(Method::kAssignString));
+}
+
+TEST_P(CordzUpdateTest, AssignInlinedArray) {
+ cord() = MakeString(TestCordSize::kInlined);
+ EXPECT_THAT(GetCordzInfoForTesting(cord()), Eq(nullptr));
+}
+
TEST_P(CordzUpdateTest, AppendCord) {
Cord src = UnsampledCord(MakeString(TestCordSize::kLarge));
cord().Append(src);
@@ -166,6 +178,56 @@
EXPECT_THAT(cord(), HasValidCordzInfoOf(InitialOr(Method::kAppendString)));
}
+TEST(CordzTest, RemovePrefix) {
+ CordzSamplingIntervalHelper sample_every(1);
+ Cord cord(MakeString(TestCordSize::kLarge));
+
+ // Half the cord
+ cord.RemovePrefix(cord.size() / 2);
+ EXPECT_THAT(cord, HasValidCordzInfoOf(Method::kConstructorString));
+ EXPECT_THAT(cord, CordzMethodCountEq(Method::kRemovePrefix, 1));
+
+ // TODO(mvels): RemovePrefix does not reset to inlined, except if empty?
+ cord.RemovePrefix(cord.size() - kMaxInline);
+ EXPECT_THAT(cord, HasValidCordzInfoOf(Method::kConstructorString));
+ EXPECT_THAT(cord, CordzMethodCountEq(Method::kRemovePrefix, 2));
+
+ cord.RemovePrefix(cord.size());
+ EXPECT_THAT(GetCordzInfoForTesting(cord), Eq(nullptr));
+}
+
+TEST(CordzTest, RemoveSuffix) {
+ CordzSamplingIntervalHelper sample_every(1);
+ Cord cord(MakeString(TestCordSize::kLarge));
+
+ // Half the cord
+ cord.RemoveSuffix(cord.size() / 2);
+ EXPECT_THAT(cord, HasValidCordzInfoOf(Method::kConstructorString));
+ EXPECT_THAT(cord, CordzMethodCountEq(Method::kRemoveSuffix, 1));
+
+ // TODO(mvels): RemoveSuffix does not reset to inlined, except if empty?
+ cord.RemoveSuffix(cord.size() - kMaxInline);
+ EXPECT_THAT(cord, HasValidCordzInfoOf(Method::kConstructorString));
+ EXPECT_THAT(cord, CordzMethodCountEq(Method::kRemoveSuffix, 2));
+
+ cord.RemoveSuffix(cord.size());
+ EXPECT_THAT(GetCordzInfoForTesting(cord), Eq(nullptr));
+}
+
+TEST(CordzTest, SubCord) {
+ CordzSamplingIntervalHelper sample_every{1};
+ Cord src(MakeString(TestCordSize::kLarge));
+
+ Cord cord1 = src.Subcord(10, src.size() / 2);
+ EXPECT_THAT(cord1, HasValidCordzInfoOf(Method::kSubCord));
+
+ Cord cord2 = src.Subcord(10, kMaxInline + 1);
+ EXPECT_THAT(cord2, HasValidCordzInfoOf(Method::kSubCord));
+
+ Cord cord3 = src.Subcord(10, kMaxInline);
+ EXPECT_THAT(GetCordzInfoForTesting(cord3), Eq(nullptr));
+}
+
} // namespace
ABSL_NAMESPACE_END
diff --git a/absl/strings/cordz_test_helpers.h b/absl/strings/cordz_test_helpers.h
index d9573c9..e410eec 100644
--- a/absl/strings/cordz_test_helpers.h
+++ b/absl/strings/cordz_test_helpers.h
@@ -27,6 +27,7 @@
#include "absl/strings/internal/cordz_sample_token.h"
#include "absl/strings/internal/cordz_statistics.h"
#include "absl/strings/internal/cordz_update_tracker.h"
+#include "absl/strings/str_cat.h"
namespace absl {
ABSL_NAMESPACE_BEGIN
@@ -34,7 +35,7 @@
// Returns the CordzInfo for the cord, or nullptr if the cord is not sampled.
inline const cord_internal::CordzInfo* GetCordzInfoForTesting(
const Cord& cord) {
- if (cord.size() <= cord_internal::kMaxInline) return nullptr;
+ if (!cord.contents_.is_tree()) return nullptr;
return cord.contents_.cordz_info();
}
@@ -47,13 +48,11 @@
return false;
}
-// Matcher on Cord* that verifies all of:
+// Matcher on Cord that verifies all of:
// - the cord is sampled
// - the CordzInfo of the cord is listed / discoverable.
// - the reported CordzStatistics match the cord's actual properties
// - the cord has an (initial) UpdateTracker count of 1 for `method`
-// This matcher accepts a const Cord* to avoid having the matcher dump
-// copious amounts of cord data on failures.
MATCHER_P(HasValidCordzInfoOf, method, "CordzInfo matches cord") {
const cord_internal::CordzInfo* cord_info = GetCordzInfoForTesting(arg);
if (cord_info == nullptr) {
@@ -78,6 +77,24 @@
return true;
}
+// Matcher on Cord that verifies that the cord is sampled and that the CordzInfo
+// update tracker has 'method' with a call count of 'n'
+MATCHER_P2(CordzMethodCountEq, method, n,
+ absl::StrCat("CordzInfo method count equals ", n)) {
+ const cord_internal::CordzInfo* cord_info = GetCordzInfoForTesting(arg);
+ if (cord_info == nullptr) {
+ *result_listener << "cord is not sampled";
+ return false;
+ }
+ cord_internal::CordzStatistics stat = cord_info->GetCordzStatistics();
+ if (stat.update_tracker.Value(method) != n) {
+ *result_listener << "Expected method count " << n << " for " << method
+ << ", found " << stat.update_tracker.Value(method);
+ return false;
+ }
+ return true;
+}
+
// Cordz will only update with a new rate once the previously scheduled event
// has fired. When we disable Cordz, a long delay takes place where we won't
// consider profiling new Cords. CordzSampleIntervalHelper will burn through
