From 54123d3ffd393903bd5211cfc76cecb51a0b671b Mon Sep 17 00:00:00 2001 From: dgolear Date: Tue, 5 Aug 2025 13:50:26 +0300 Subject: YT: Make trim helpers noncopying; lesser copies in rich ypath parsing commit_hash:88244dc0263029f091579dd66bba1e03132449fe --- library/cpp/yt/string/string.cpp | 38 +++++++----------------- library/cpp/yt/string/string.h | 4 +-- library/cpp/yt/string/unittests/string_ut.cpp | 17 ++++++++++- yt/yt/client/unittests/ypath_ut.cpp | 10 +++++-- yt/yt/client/ypath/parser_detail.cpp | 37 ++++++++++------------- yt/yt/client/ypath/parser_detail.h | 2 +- yt/yt/client/ypath/rich.cpp | 42 +++++++++++---------------- yt/yt/client/ypath/rich.h | 20 ++++++++----- yt/yt/core/misc/proc.cpp | 5 ++-- 9 files changed, 85 insertions(+), 90 deletions(-) diff --git a/library/cpp/yt/string/string.cpp b/library/cpp/yt/string/string.cpp index a346219144c..d155da578e4 100644 --- a/library/cpp/yt/string/string.cpp +++ b/library/cpp/yt/string/string.cpp @@ -65,42 +65,26 @@ TString CamelCaseToUnderscoreCase(TStringBuf str) //////////////////////////////////////////////////////////////////////////////// -TString TrimLeadingWhitespaces(TStringBuf str) +[[nodiscard]] TStringBuf TrimLeadingWhitespaces(TStringBuf str Y_LIFETIME_BOUND) { - for (size_t i = 0; i < str.size(); ++i) { - if (str[i] != ' ') { - return TString(str.substr(i)); - } - } - return ""; + auto begin = str.find_first_not_of(' '); + return begin == TStringBuf::npos ? "" : str.substr(begin); } -TString Trim(TStringBuf str, TStringBuf whitespaces) +[[nodiscard]] TStringBuf Trim(TStringBuf str Y_LIFETIME_BOUND, TStringBuf whitespaces) { - size_t end = str.size(); - while (end > 0) { - size_t i = end - 1; - bool isWhitespace = false; - for (auto c : whitespaces) { - if (str[i] == c) { - isWhitespace = true; - break; - } - } - if (!isWhitespace) { - break; - } - --end; + if (whitespaces.empty()) { + return str; } + auto end = str.find_last_not_of(whitespaces); - if (end == 0) { + if (end == TStringBuf::npos) { return ""; } - size_t begin = str.find_first_not_of(whitespaces); - YT_VERIFY(begin != TString::npos); - YT_VERIFY(begin < end); - return TString(str.substr(begin, end - begin)); + auto begin = str.find_first_not_of(whitespaces); + YT_VERIFY(begin != TStringBuf::npos); + return str.substr(begin, end - begin + 1); } //////////////////////////////////////////////////////////////////////////////// diff --git a/library/cpp/yt/string/string.h b/library/cpp/yt/string/string.h index 9e521204838..6758c25c022 100644 --- a/library/cpp/yt/string/string.h +++ b/library/cpp/yt/string/string.h @@ -148,8 +148,8 @@ TString UnderscoreCaseToCamelCase(TStringBuf str); void CamelCaseToUnderscoreCase(TStringBuilderBase* builder, TStringBuf str); TString CamelCaseToUnderscoreCase(TStringBuf str); -TString TrimLeadingWhitespaces(TStringBuf str); -TString Trim(TStringBuf str, TStringBuf whitespaces); +[[nodiscard]] TStringBuf TrimLeadingWhitespaces(TStringBuf str Y_LIFETIME_BOUND); +[[nodiscard]] TStringBuf Trim(TStringBuf str Y_LIFETIME_BOUND, TStringBuf whitespaces = " "); //////////////////////////////////////////////////////////////////////////////// diff --git a/library/cpp/yt/string/unittests/string_ut.cpp b/library/cpp/yt/string/unittests/string_ut.cpp index 3e12312af0b..073516006fb 100644 --- a/library/cpp/yt/string/unittests/string_ut.cpp +++ b/library/cpp/yt/string/unittests/string_ut.cpp @@ -45,8 +45,23 @@ TEST(TStringTest, CamelCaseToUnderscoreCase) } } +TEST(TStringTest, TrimWhitespaces) +{ + EXPECT_EQ("", TrimLeadingWhitespaces("")); + EXPECT_EQ("", TrimLeadingWhitespaces(" ")); + EXPECT_EQ("foo", TrimLeadingWhitespaces(" foo")); + EXPECT_EQ("foo ", TrimLeadingWhitespaces(" foo ")); + EXPECT_EQ("f oo ", TrimLeadingWhitespaces(" f oo ")); + + EXPECT_EQ("", Trim("", "")); + EXPECT_EQ("", Trim(" ", " ")); + EXPECT_EQ(" ", Trim(" ", "\t")); + EXPECT_EQ("", Trim(" \t", " \t")); + EXPECT_EQ("foo", Trim(" foo ", " ")); + EXPECT_EQ("f", Trim(" f ", " ")); +} + //////////////////////////////////////////////////////////////////////////////// } // namespace } // namespace NYT - diff --git a/yt/yt/client/unittests/ypath_ut.cpp b/yt/yt/client/unittests/ypath_ut.cpp index 93a3c199d1a..4202fa46f7c 100644 --- a/yt/yt/client/unittests/ypath_ut.cpp +++ b/yt/yt/client/unittests/ypath_ut.cpp @@ -308,7 +308,7 @@ TEST_F(TYPathTest, ParseRichYPath1) TEST_F(TYPathTest, ParseRichYPath2) { - auto path = NYPath::TRichYPath::Parse("//home"); + auto path = NYPath::TRichYPath::Parse(" //home"); EXPECT_EQ(path.GetPath(), "//home"); EXPECT_TRUE( AreNodesEqual( @@ -388,7 +388,7 @@ TEST_F(TYPathTest, ParseRichYPath9) { EXPECT_THROW_MESSAGE_HAS_SUBSTR( TRichYPath::Parse("@home"), - std::exception, + TErrorException, "does not start with a valid root-designator"); } @@ -408,6 +408,12 @@ TEST_F(TYPathTest, ParseRichYPath11) "does not start with a valid root-designator"); } +TEST_F(TYPathTest, ParseRichYPath12) +{ + auto path = TRichYPath::Parse("<>//home"); + EXPECT_TRUE(path.Attributes().ListKeys().empty()); +} + TEST_F(TYPathTest, IgnoreAmpersand1) { Set("&/a", "b"); diff --git a/yt/yt/client/ypath/parser_detail.cpp b/yt/yt/client/ypath/parser_detail.cpp index 1f60d6a9a37..777844f4faa 100644 --- a/yt/yt/client/ypath/parser_detail.cpp +++ b/yt/yt/client/ypath/parser_detail.cpp @@ -42,9 +42,8 @@ void ThrowUnexpectedToken(const TToken& token) token); } -TString ParseAttributes(const TString& str, const IAttributeDictionaryPtr& attributes) +TStringBuf ParseAttributes(TStringBuf str, const IAttributeDictionaryPtr& attributes) { - int spaceCount = 0; { size_t index = 0; while (index < str.size() && IsSpace(str[index])) { @@ -53,17 +52,16 @@ TString ParseAttributes(const TString& str, const IAttributeDictionaryPtr& attri if (index == str.size() || str[index] != TokenTypeToChar(NYson::ETokenType::LeftAngle)) { return str; } - spaceCount = index; } - NYson::TTokenizer tokenizer(TStringBuf(str).SubStr(spaceCount)); + NYson::TTokenizer tokenizer(str); tokenizer.ParseNext(); if (tokenizer.CurrentToken().GetType() != NYson::ETokenType::LeftAngle) { ThrowUnexpectedToken(tokenizer.CurrentToken()); } int depth = 0; - int attrStartPosition = spaceCount + 1; + int attrStartPosition = tokenizer.GetPosition(); while (true) { switch (tokenizer.CurrentToken().GetType()) { @@ -86,7 +84,8 @@ TString ParseAttributes(const TString& str, const IAttributeDictionaryPtr& attri } } - int attrEndPosition = spaceCount + tokenizer.GetPosition() - 1; + int attrEndPosition = tokenizer.GetPosition() - 1; + YT_ASSERT(attrEndPosition >= attrStartPosition); int pathStartPosition = attrEndPosition + 1; TYsonString attrYson( @@ -117,7 +116,7 @@ bool StartsWithRootDesignator(TStringBuf str) return true; } -TString ParseCluster(TString str, const IAttributeDictionaryPtr& attributes) +TStringBuf ParseCluster(TStringBuf str, const IAttributeDictionaryPtr& attributes) { if (str.empty()) { return str; @@ -128,31 +127,27 @@ TString ParseCluster(TString str, const IAttributeDictionaryPtr& attributes) } auto clusterSeparatorIndex = str.find_first_of(':'); - if (clusterSeparatorIndex == TString::npos) { - THROW_ERROR_EXCEPTION( - "Path %Qv does not start with a valid root-designator", + if (clusterSeparatorIndex == TStringBuf::npos) { + THROW_ERROR_EXCEPTION("Path %Qv does not start with a valid root-designator", str); } - const auto clusterName = str.substr(0, clusterSeparatorIndex); + auto clusterName = str.substr(0, clusterSeparatorIndex); if (clusterName.empty()) { - THROW_ERROR_EXCEPTION( - "Cluster name in path %Qv cannot be empty", + THROW_ERROR_EXCEPTION("Cluster name in path %Qv cannot be empty", str); } - auto illegalSymbolIt = std::find_if_not(clusterName.begin(), clusterName.end(), &IsValidClusterSymbol); + auto illegalSymbolIt = std::ranges::find_if_not(clusterName, &IsValidClusterSymbol); if (illegalSymbolIt != clusterName.end()) { - THROW_ERROR_EXCEPTION( - "Possible cluster name in path %Qv contains illegal symbol %Qv", + THROW_ERROR_EXCEPTION("Possible cluster name in path %Qv contains illegal symbol %Qv", str, *illegalSymbolIt); } auto remainingString = str.substr(clusterSeparatorIndex + 1); if (!StartsWithRootDesignator(remainingString)) { - THROW_ERROR_EXCEPTION( - "Path %Qv does not start with a valid root-designator", + THROW_ERROR_EXCEPTION("Path %Qv does not start with a valid root-designator", str); } @@ -404,18 +399,18 @@ void ParseRowRanges(NYson::TTokenizer& tokenizer, IAttributeDictionary* attribut //////////////////////////////////////////////////////////////////////////////// -TRichYPath ParseRichYPathImpl(const TString& str) +TRichYPath ParseRichYPathImpl(TStringBuf str) { auto attributes = CreateEphemeralAttributes(); auto strWithoutAttributes = ParseAttributes(str, attributes); - strWithoutAttributes = ParseCluster(std::move(strWithoutAttributes), attributes); + strWithoutAttributes = ParseCluster(strWithoutAttributes, attributes); TTokenizer ypathTokenizer(strWithoutAttributes); while (ypathTokenizer.GetType() != ETokenType::EndOfStream && ypathTokenizer.GetType() != ETokenType::Range) { ypathTokenizer.Advance(); } - auto path = TYPath(ypathTokenizer.GetPrefix()); + TYPath path(ypathTokenizer.GetPrefix()); auto rangeStr = ypathTokenizer.GetToken(); if (ypathTokenizer.GetType() == ETokenType::Range) { diff --git a/yt/yt/client/ypath/parser_detail.h b/yt/yt/client/ypath/parser_detail.h index 953df87034e..d3b424040ed 100644 --- a/yt/yt/client/ypath/parser_detail.h +++ b/yt/yt/client/ypath/parser_detail.h @@ -6,7 +6,7 @@ namespace NYT::NYPath { //////////////////////////////////////////////////////////////////////////////// -TRichYPath ParseRichYPathImpl(const TString& str); +TRichYPath ParseRichYPathImpl(TStringBuf str); //////////////////////////////////////////////////////////////////////////////// diff --git a/yt/yt/client/ypath/rich.cpp b/yt/yt/client/ypath/rich.cpp index 156d7e6b99a..6bde2cacae3 100644 --- a/yt/yt/client/ypath/rich.cpp +++ b/yt/yt/client/ypath/rich.cpp @@ -25,10 +25,7 @@ using namespace NSecurityClient; //////////////////////////////////////////////////////////////////////////////// -TRichYPath::TRichYPath() -{ } - -TRichYPath::TRichYPath(const TRichYPath& other) +TRichYPath::TRichYPath(const TRichYPath& other) noexcept : Path_(other.Path_) , Attributes_(other.Attributes_ ? other.Attributes_->Clone() : nullptr) { } @@ -39,30 +36,34 @@ TRichYPath::TRichYPath(const char* path) *this = Normalize(); } -TRichYPath::TRichYPath(const TYPath& path) - : Path_(path) +TRichYPath::TRichYPath(TYPath path) + : Path_(std::move(path)) { *this = Normalize(); } -TRichYPath::TRichYPath(TRichYPath&& other) - : Path_(std::move(other.Path_)) - , Attributes_(std::move(other.Attributes_)) -{ } - -TRichYPath::TRichYPath(const TYPath& path, const IAttributeDictionary& attributes) - : Path_(path) +TRichYPath::TRichYPath(TYPath path, const IAttributeDictionary& attributes) + : Path_(std::move(path)) , Attributes_(attributes.Clone()) { } +TRichYPath& TRichYPath::operator=(const TRichYPath& other) noexcept +{ + if (this != &other) { + Path_ = other.Path_; + Attributes_ = other.Attributes_ ? other.Attributes_->Clone() : nullptr; + } + return *this; +} + const TYPath& TRichYPath::GetPath() const { return Path_; } -void TRichYPath::SetPath(const TYPath& path) +void TRichYPath::SetPath(TYPath path) { - Path_ = path; + Path_ = std::move(path); } const IAttributeDictionary& TRichYPath::Attributes() const @@ -78,15 +79,6 @@ IAttributeDictionary& TRichYPath::Attributes() return *Attributes_; } -TRichYPath& TRichYPath::operator = (const TRichYPath& other) -{ - if (this != &other) { - Path_ = other.Path_; - Attributes_ = other.Attributes_ ? other.Attributes_->Clone() : nullptr; - } - return *this; -} - //////////////////////////////////////////////////////////////////////////////// bool operator== (const TRichYPath& lhs, const TRichYPath& rhs) @@ -151,7 +143,7 @@ TYsonString FindAttributeYson(const TRichYPath& path, const std::string& key) } // namespace -TRichYPath TRichYPath::Parse(const TString& str) +TRichYPath TRichYPath::Parse(TStringBuf str) { return ParseRichYPathImpl(str); } diff --git a/yt/yt/client/ypath/rich.h b/yt/yt/client/ypath/rich.h index 520947c05a6..dc13115095a 100644 --- a/yt/yt/client/ypath/rich.h +++ b/yt/yt/client/ypath/rich.h @@ -27,19 +27,23 @@ namespace NYT::NYPath { class TRichYPath { public: - TRichYPath(); - TRichYPath(const TRichYPath& other); - TRichYPath(TRichYPath&& other); + TRichYPath() noexcept = default; + + TRichYPath(const TRichYPath& other) noexcept; + TRichYPath& operator=(const TRichYPath& other) noexcept; + + TRichYPath(TRichYPath&& other) noexcept = default; + TRichYPath& operator=(TRichYPath&& other) noexcept = default; + TRichYPath(const char* path); - TRichYPath(const TYPath& path); - TRichYPath(const TYPath& path, const NYTree::IAttributeDictionary& attributes); - TRichYPath& operator = (const TRichYPath& other); + TRichYPath(TYPath path); + TRichYPath(TYPath path, const NYTree::IAttributeDictionary& attributes); - static TRichYPath Parse(const TString& str); + static TRichYPath Parse(TStringBuf str); TRichYPath Normalize() const; const TYPath& GetPath() const; - void SetPath(const TYPath& path); + void SetPath(TYPath path); const NYTree::IAttributeDictionary& Attributes() const; NYTree::IAttributeDictionary& Attributes(); diff --git a/yt/yt/core/misc/proc.cpp b/yt/yt/core/misc/proc.cpp index 351123651f5..2426199f9d3 100644 --- a/yt/yt/core/misc/proc.cpp +++ b/yt/yt/core/misc/proc.cpp @@ -584,8 +584,7 @@ std::optional GetCgroupAnonymousMemoryLimit( { #ifdef _linux_ TString path = cgroupMountPoint + "/memory" + cgroupPath + "/memory.anon.limit"; - auto content = Trim(TUnbufferedFileInput(path).ReadAll(), "\n"); - return FromString(content); + return FromString(Trim(TUnbufferedFileInput(path).ReadAll(), "\n")); #else Y_UNUSED(cgroupPath, cgroupMountPoint); return {}; @@ -635,7 +634,7 @@ TString GetProcessName(int pid) { #ifdef _linux_ TString path = Format("/proc/%v/comm", pid); - return Trim(TUnbufferedFileInput(path).ReadAll(), "\n"); + return TString(Trim(TUnbufferedFileInput(path).ReadAll(), "\n")); #else Y_UNUSED(pid); return ""; -- cgit v1.3