[util] StripInPlace should modify string in-place

ref:2aec19528c2378a0994534ec032550413a095389

3 files changed, 107 insertions, 22 deletions

diff --git a/util/string/benchmark/strip/main.cpp b/util/string/benchmark/strip/main.cpp
new file mode 100644
index 00000000000..35a266e4f4d
--- /dev/null
+++ b/util/string/benchmark/strip/main.cpp
@@ -0,0 +1,65 @@
+#include <library/cpp/testing/benchmark/bench.h>
+
+#include <util/string/strip.h>
+#include <util/generic/xrange.h>
+
+static const TString SHORT_STRING = "  foo ";
+static const TString LONG_STRING = TString(200, ' ') + TString(200, 'f') + TString(200, ' ');
+
+Y_CPU_BENCHMARK(StripInPlaceShortNoChange, iface) {
+    TString s = "foo";
+    for (const auto i : xrange(iface.Iterations())) {
+        Y_UNUSED(i);
+        StripInPlace(s);
+        Y_DO_NOT_OPTIMIZE_AWAY(s);
+    }
+}
+
+Y_CPU_BENCHMARK(StripInPlaceLongNoChange, iface) {
+    TString s = TString{200, 'f'};
+    for (const auto i : xrange(iface.Iterations())) {
+        Y_UNUSED(i);
+        StripInPlace(s);
+        Y_DO_NOT_OPTIMIZE_AWAY(s);
+    }
+}
+
+Y_CPU_BENCHMARK(StripInPlaceShort, iface) {
+    TString s;
+    for (const auto i : xrange(iface.Iterations())) {
+        Y_UNUSED(i);
+        s.assign(SHORT_STRING.begin(), SHORT_STRING.end());
+        StripInPlace(s);
+        Y_DO_NOT_OPTIMIZE_AWAY(s);
+    }
+}
+
+Y_CPU_BENCHMARK(StripInPlaceLong, iface) {
+    TString s;
+    for (const auto i : xrange(iface.Iterations())) {
+        Y_UNUSED(i);
+        s.assign(LONG_STRING.begin(), LONG_STRING.end());
+        StripInPlace(s);
+        Y_DO_NOT_OPTIMIZE_AWAY(s);
+    }
+}
+
+Y_CPU_BENCHMARK(StripInPlaceShortMut, iface) {
+    TString s = SHORT_STRING;
+    for (const auto i : xrange(iface.Iterations())) {
+        Y_UNUSED(i);
+        s.append(' ');
+        StripInPlace(s);
+        Y_DO_NOT_OPTIMIZE_AWAY(s);
+    }
+}
+
+Y_CPU_BENCHMARK(StripInPlaceLongMut, iface) {
+    TString s = LONG_STRING;
+    for (const auto i : xrange(iface.Iterations())) {
+        Y_UNUSED(i);
+        s.append(100, ' ');
+        StripInPlace(s);
+        Y_DO_NOT_OPTIMIZE_AWAY(s);
+    }
+}
diff --git a/util/string/strip.h b/util/string/strip.h
index c2ea61adce0..8d9e5ed988d 100644
--- a/util/string/strip.h
+++ b/util/string/strip.h
@@ -4,6 +4,7 @@
 
 #include <util/generic/string.h>
 #include <util/generic/strbuf.h>
+#include <util/generic/typetraits.h>
 #include <utility>
 
 template <class It>
@@ -86,7 +87,11 @@ struct TStripImpl {
         auto e = from.end();
 
         if (StripRange(b, e, criterion)) {
-            to = T(b, e - b);
+            if constexpr (::TIsTemplateBaseOf<std::basic_string_view, T>::value) {
+                to = T(b, e - b);
+            } else {
+                to.assign(b, e - b);
+            }
 
             return true;
         }
diff --git a/util/string/strip_ut.cpp b/util/string/strip_ut.cpp
index df4f9bc57d3..0b1687ffa7c 100644
--- a/util/string/strip_ut.cpp
+++ b/util/string/strip_ut.cpp
@@ -5,28 +5,29 @@
 #include <util/charset/wide.h>
 
 Y_UNIT_TEST_SUITE(TStripStringTest) {
-    Y_UNIT_TEST(TestStrip) {
-        struct TTest {
-            const char* Str;
-            const char* StripLeftRes;
-            const char* StripRightRes;
-            const char* StripRes;
-        };
-        static const TTest tests[] = {
-            {"  012  ", "012  ", "  012", "012"},
-            {"  012", "012", "  012", "012"},
-            {"012\t\t", "012\t\t", "012", "012"},
-            {"\t012\t", "012\t", "\t012", "012"},
-            {"012", "012", "012", "012"},
-            {"012\r\n", "012\r\n", "012", "012"},
-            {"\n012\r", "012\r", "\n012", "012"},
-            {"\n \t\r", "", "", ""},
-            {"", "", "", ""},
-            {"abc", "abc", "abc", "abc"},
-            {"a c", "a c", "a c", "a c"},
-        };
+    struct TStripTest {
+        TStringBuf Str;
+        TStringBuf StripLeftRes;
+        TStringBuf StripRightRes;
+        TStringBuf StripRes;
+    };
+    static constexpr TStripTest StripTests[] = {
+        {"  012  ", "012  ", "  012", "012"},
+        {"  012", "012", "  012", "012"},
+        {"012\t\t", "012\t\t", "012", "012"},
+        {"\t012\t", "012\t", "\t012", "012"},
+        {"012", "012", "012", "012"},
+        {"012\r\n", "012\r\n", "012", "012"},
+        {"\n012\r", "012\r", "\n012", "012"},
+        {"\n \t\r", "", "", ""},
+        {"", "", "", ""},
+        {"abc", "abc", "abc", "abc"},
+        {"a c", "a c", "a c", "a c"},
+        {"  long string to avoid SSO            \n", "long string to avoid SSO            \n", "  long string to avoid SSO", "long string to avoid SSO"},
+    };
 
-        for (const auto& test : tests) {
+    Y_UNIT_TEST(TestStrip) {
+        for (const auto& test : StripTests) {
             TString inputStr(test.Str);
 
             TString s;
@@ -44,6 +45,20 @@ Y_UNIT_TEST_SUITE(TStripStringTest) {
         };
     }
 
+    Y_UNIT_TEST(TestStripInPlace) {
+        for (const auto& test : StripTests) {
+            TString str(test.Str);
+            Y_ASSERT(str.IsDetached() || str.empty()); // prerequisite of the test; check that we don't try to modify shared COW-string in-place by accident
+            const void* stringPtrPrior = str.data();
+            StripInPlace(str);
+            const void* stringPtrAfter = str.data();
+            UNIT_ASSERT_VALUES_EQUAL(str, test.StripRes);
+            if (!test.Str.empty()) {
+                UNIT_ASSERT_EQUAL_C(stringPtrPrior, stringPtrAfter, TString(test.Str).Quote()); // StripInPlace should reuse buffer of original string
+            }
+        }
+    }
+
     Y_UNIT_TEST(TestCustomStrip) {
         struct TTest {
             const char* Str;