From d9637d07816af472bd40bb8e6e9f63f1c48085fe Mon Sep 17 00:00:00 2001
From: Ahmad Samir <a.samirh78@gmail.com>
Date: Thu, 27 Oct 2022 11:34:05 +0200
Subject: [PATCH] QString: don't detach in removeStringImpl()

- If this string is not shared, modify it directly
- If this string is shared, instead of detaching copy the characters
  from this string, except the ones that are going to be removed, to a
  new string and swap it. This is more efficient than detaching, which
  would copy the whole string including the characters that are going
  to be removed.

This affects:
remove(const QString &str, Qt::CaseSensitivity cs)
remove(QLatin1StringView str, Qt::CaseSensitivity cs)

Adjust the unittests to test both code paths.

[ChangeLog][QtCore][QString] Improved the performance of
QString::remove() by avoiding unnecessary data copying. Now, if this
string is (implicitly) shared with another, instead of copying
everything and then removing what we don't want, the characters from
this string are copied to the destination, except the ones that need to
be removed.

Task-number: QTBUG-106181
Change-Id: Id8eba59a44bab641cc8aa662eb45063faf201183
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
---
 src/corelib/text/qstring.cpp                  | 38 +++++++++++++------
 .../auto/corelib/text/qstring/tst_qstring.cpp | 20 ++++++++--
 2 files changed, 43 insertions(+), 15 deletions(-)

diff --git a/src/corelib/text/qstring.cpp b/src/corelib/text/qstring.cpp
index 2ac8306eb4..3ec9682935 100644
--- a/src/corelib/text/qstring.cpp
+++ b/src/corelib/text/qstring.cpp
@@ -3325,19 +3325,33 @@ static void removeStringImpl(QString &s, const T &needle, Qt::CaseSensitivity cs
     if (i < 0)
         return;
 
-    auto beg = s.begin(); // detaches
-    auto dst = beg + i;
-    auto src = beg + i + needleSize;
-    auto end = s.end();
-    // loop invariant: [beg, dst[ is partial result
-    //                 [src, end[ still to be checked for needles
-    while (src < end) {
-        i = s.indexOf(needle, std::distance(beg, src), cs);
-        auto hit = i == -1 ? end : beg + i;
-        dst = std::copy(src, hit, dst);
-        src = hit + needleSize;
+    QString::DataPointer &dptr = s.data_ptr();
+    auto begin = dptr.begin();
+    auto end = dptr.end();
+
+    auto copyFunc = [&](auto &dst) {
+        auto src = begin + i + needleSize;
+        while (src < end) {
+            i = s.indexOf(needle, std::distance(begin, src), cs);
+            auto hit = i == -1 ? end : begin + i;
+            dst = std::copy(src, hit, dst);
+            src = hit + needleSize;
+        }
+        return dst;
+    };
+
+    if (!dptr->needsDetach()) {
+        auto dst = begin + i;
+        dst = copyFunc(dst);
+        s.truncate(std::distance(begin, dst));
+    } else {
+        QString copy{s.size(), Qt::Uninitialized};
+        auto copy_begin = copy.begin();
+        auto dst = std::copy(begin, begin + i, copy_begin); // Chunk before the first hit
+        dst = copyFunc(dst);
+        copy.resize(std::distance(copy_begin, dst));
+        s.swap(copy);
     }
-    s.truncate(std::distance(beg, dst));
 }
 
 /*!
diff --git a/tests/auto/corelib/text/qstring/tst_qstring.cpp b/tests/auto/corelib/text/qstring/tst_qstring.cpp
index 6a40d511e6..600b97f1bf 100644
--- a/tests/auto/corelib/text/qstring/tst_qstring.cpp
+++ b/tests/auto/corelib/text/qstring/tst_qstring.cpp
@@ -3381,10 +3381,16 @@ void tst_QString::remove_string()
             }
         }
 
+        // Test when needsDetach() is true
         QString s3 = string;
         s3.remove( before, cs );
         QCOMPARE(s3, result);
 
+        QString s5 = string;
+        s5.begin(); // Detach so needsDetach() is false
+        s5.remove( before, cs );
+        QCOMPARE(s5, result);
+
         if (QtPrivate::isLatin1(before)) {
             QString s6 = string;
             s6.remove( QLatin1String(before.toLatin1()), cs );
@@ -3440,9 +3446,17 @@ void tst_QString::remove_regexp()
 void tst_QString::remove_extra()
 {
     {
-        QString s = "The quick brown fox jumps over the lazy dog. "
-                    "The lazy dog jumps over the quick brown fox.";
-        s.remove(s);
+        QString quickFox = "The quick brown fox jumps over the lazy dog. "
+                           "The lazy dog jumps over the quick brown fox.";
+        QString s1 = quickFox;
+        QVERIFY(s1.data_ptr().needsDetach());
+        s1.remove(s1);
+        QVERIFY(s1.isEmpty());
+        QVERIFY(!quickFox.isEmpty());
+
+        QVERIFY(!quickFox.data_ptr().needsDetach());
+        quickFox.remove(quickFox);
+        QVERIFY(quickFox.isEmpty());
     }
 
     {