From a4798817e824a5c45689933bc62338f3c7d6961b Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Mon, 9 Nov 2020 04:09:55 -0800 Subject: Export of internal Abseil changes -- 197bd3e5e315f408bdc2f7111a0652be7f47609f by Jorg Brown : ConvertibleToStringView wastes a lot of cycles initializing a std::string member that it never uses. This CL restricts ConvertibleToStringView to just the cases that don't nned a std::string member, and uses an additional StrSplit overload to catch when users pass a temporary string. This makes StrSplit() 0-30% faster depending on the use case. PiperOrigin-RevId: 341378221 GitOrigin-RevId: 197bd3e5e315f408bdc2f7111a0652be7f47609f Change-Id: Id863f2a3d298ed5a086ac912afab42dc30601be5 --- absl/strings/internal/str_split_internal.h | 51 ++++++++---------------------- absl/strings/str_split.h | 42 +++++++++++++++++++++--- 2 files changed, 51 insertions(+), 42 deletions(-) diff --git a/absl/strings/internal/str_split_internal.h b/absl/strings/internal/str_split_internal.h index 49ec5392..a2f41c15 100644 --- a/absl/strings/internal/str_split_internal.h +++ b/absl/strings/internal/str_split_internal.h @@ -51,9 +51,9 @@ ABSL_NAMESPACE_BEGIN namespace strings_internal { // This class is implicitly constructible from everything that absl::string_view -// is implicitly constructible from. If it's constructed from a temporary -// string, the data is moved into a data member so its lifetime matches that of -// the ConvertibleToStringView instance. +// is implicitly constructible from, except for rvalue strings. This means it +// can be used as a function parameter in places where passing a temporary +// string might cause memory lifetime issues. class ConvertibleToStringView { public: ConvertibleToStringView(const char* s) // NOLINT(runtime/explicit) @@ -65,41 +65,12 @@ class ConvertibleToStringView { : value_(s) {} // Matches rvalue strings and moves their data to a member. - ConvertibleToStringView(std::string&& s) // NOLINT(runtime/explicit) - : copy_(std::move(s)), value_(copy_) {} - - ConvertibleToStringView(const ConvertibleToStringView& other) - : copy_(other.copy_), - value_(other.IsSelfReferential() ? copy_ : other.value_) {} - - ConvertibleToStringView(ConvertibleToStringView&& other) { - StealMembers(std::move(other)); - } - - ConvertibleToStringView& operator=(ConvertibleToStringView other) { - StealMembers(std::move(other)); - return *this; - } + ConvertibleToStringView(std::string&& s) = delete; + ConvertibleToStringView(const std::string&& s) = delete; absl::string_view value() const { return value_; } private: - // Returns true if ctsp's value refers to its internal copy_ member. - bool IsSelfReferential() const { return value_.data() == copy_.data(); } - - void StealMembers(ConvertibleToStringView&& other) { - if (other.IsSelfReferential()) { - copy_ = std::move(other.copy_); - value_ = copy_; - other.value_ = other.copy_; - } else { - value_ = other.value_; - } - } - - // Holds the data moved from temporary std::string arguments. Declared first - // so that 'value' can refer to 'copy_'. - std::string copy_; absl::string_view value_; }; @@ -273,7 +244,11 @@ struct SplitterIsConvertibleTo // the split strings: only strings for which the predicate returns true will be // kept. A Predicate object is any unary functor that takes an absl::string_view // and returns bool. -template +// +// The StringType parameter can be either string_view or string, depending on +// whether the Splitter refers to a string stored elsewhere, or if the string +// resides inside the Splitter itself. +template class Splitter { public: using DelimiterType = Delimiter; @@ -281,12 +256,12 @@ class Splitter { using const_iterator = strings_internal::SplitIterator; using value_type = typename std::iterator_traits::value_type; - Splitter(ConvertibleToStringView input_text, Delimiter d, Predicate p) + Splitter(StringType input_text, Delimiter d, Predicate p) : text_(std::move(input_text)), delimiter_(std::move(d)), predicate_(std::move(p)) {} - absl::string_view text() const { return text_.value(); } + absl::string_view text() const { return text_; } const Delimiter& delimiter() const { return delimiter_; } const Predicate& predicate() const { return predicate_; } @@ -443,7 +418,7 @@ class Splitter { }; }; - ConvertibleToStringView text_; + StringType text_; Delimiter delimiter_; Predicate predicate_; }; diff --git a/absl/strings/str_split.h b/absl/strings/str_split.h index 1ce17f38..bfbca422 100644 --- a/absl/strings/str_split.h +++ b/absl/strings/str_split.h @@ -369,6 +369,12 @@ struct SkipWhitespace { } }; +template +using EnableSplitIfString = + typename std::enable_if::value || + std::is_same::value, + int>::type; + //------------------------------------------------------------------------------ // StrSplit() //------------------------------------------------------------------------------ @@ -489,22 +495,50 @@ struct SkipWhitespace { // Try not to depend on this distinction because the bug may one day be fixed. template strings_internal::Splitter< - typename strings_internal::SelectDelimiter::type, AllowEmpty> + typename strings_internal::SelectDelimiter::type, AllowEmpty, + absl::string_view> StrSplit(strings_internal::ConvertibleToStringView text, Delimiter d) { using DelimiterType = typename strings_internal::SelectDelimiter::type; - return strings_internal::Splitter( + return strings_internal::Splitter( + text.value(), DelimiterType(d), AllowEmpty()); +} + +template = 0> +strings_internal::Splitter< + typename strings_internal::SelectDelimiter::type, AllowEmpty, + std::string> +StrSplit(StringType&& text, Delimiter d) { + using DelimiterType = + typename strings_internal::SelectDelimiter::type; + return strings_internal::Splitter( std::move(text), DelimiterType(d), AllowEmpty()); } template strings_internal::Splitter< - typename strings_internal::SelectDelimiter::type, Predicate> + typename strings_internal::SelectDelimiter::type, Predicate, + absl::string_view> StrSplit(strings_internal::ConvertibleToStringView text, Delimiter d, Predicate p) { using DelimiterType = typename strings_internal::SelectDelimiter::type; - return strings_internal::Splitter( + return strings_internal::Splitter( + text.value(), DelimiterType(d), std::move(p)); +} + +template = 0> +strings_internal::Splitter< + typename strings_internal::SelectDelimiter::type, Predicate, + std::string> +StrSplit(StringType&& text, Delimiter d, Predicate p) { + using DelimiterType = + typename strings_internal::SelectDelimiter::type; + return strings_internal::Splitter( std::move(text), DelimiterType(d), std::move(p)); } -- cgit v1.2.3