AuroraMiddleware/qt5base-lts
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

QString: don't detach in insert()

Browse Source 
Instead of detaching when the string is shared, or if the the insertion
would cause a reallocation, create a new string and copy characters to
it as needed, then swap it with "this" string. This is more efficient
than detaching which would copy the whole string before inserting, as
some characters would be copied multiple times.

Use detachAndGrow(), otherwise QStringBuilder unitests fail:
PASS   : tst_QStringBuilder1::initTestCase()
FAIL!  : tst_QStringBuilder1::scenario() 'prepends < max_prepends' returned FALSE. ()
   Loc: [tests/auto/corelib/text/qstringbuilder/qstringbuilder1/stringbuilder.cpp(61)]
PASS   : tst_QStringBuilder1::cleanupTestCase()

The issue is that now when inserting, if the string is going to
reallocated, we create a new string, so the freeSpaceAtBegin()
optimization doesn't work the same way.

void checkItWorksWithFreeSpaceAtBegin(const String &chunk, const Separator &separator)
{
    // GIVEN: a String with freeSpaceAtBegin() and less than chunk.size() freeSpaceAtEnd()
    String str;

    int prepends = 0;
    const int max_prepends = 10;
    while (str.data_ptr().freeSpaceAtBegin() < chunk.size() && prepends++ < max_prepends)
        str.prepend(chunk);
    QVERIFY(prepends < max_prepends);
    ...
    ...

each str.prepend() would have reallocated.

[ChangeLog][QtCore][QString] Calling insert() on a currently shared
string is now done more efficiently.

Task-number: QTBUG-106186
Change-Id: I07ce8d6bde50919fdc587433e624ace9cee05be8
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
This commit is contained in:
Ahmad Samir 2022-12-14 23:53:57 +02:00
parent 67108ef8db
commit cc6324665e
2 changed files with 150 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
src/corelib/text/qstring.cpp
View File

@ -2951,6 +2951,23 @@ static void insert_helper(QString &str, qsizetype i, T toInsert)
const qsizetype insert_size = toInsert.size();
const qsizetype newSize = str_d.size + difference + insert_size;
const auto side = i == 0 ? QArrayData::GrowsAtBeginning : QArrayData::GrowsAtEnd;
if (str_d.needsDetach() || needsReallocate(str, newSize)) {
const auto cbegin = str.cbegin();
const auto cend = str.cend();
const auto insert_start = difference == 0 ? std::next(cbegin, i) : cend;
QString other;
// Using detachAndGrow() so that prepend optimization works and QStringBuilder
// unittests pass
other.data_ptr().detachAndGrow(side, newSize, nullptr, nullptr);
other.append(QStringView(cbegin, insert_start));
other.resize(i, u' ');
other.append(toInsert);
other.append(QStringView(insert_start, cend));
str.swap(other);
return;
}
str_d.detachAndGrow(side, difference + insert_size, nullptr, nullptr);
Q_CHECK_PTR(str_d.data());
str.resize(newSize);

173
tests/auto/corelib/text/qstring/tst_qstring.cpp
View File

@ -282,11 +282,22 @@ static void do_apply1(MemFun mf)
QFETCH(A1, a1);
QFETCH(QString, expected);
Arg<ArgType>(arg).apply1(s, mf, a1);
// Test when the string is shared
QString str = s;
Arg<ArgType>(arg).apply1(str, mf, a1);
QCOMPARE(s, expected);
QCOMPARE(s.isEmpty(), expected.isEmpty());
QCOMPARE(s.isNull(), expected.isNull());
QCOMPARE(str, expected);
QCOMPARE(str.isEmpty(), expected.isEmpty());
QCOMPARE(str.isNull(), expected.isNull());
// Test when the string is not shared
str = s;
str.detach();
Arg<ArgType>(arg).apply1(str, mf, a1);
QCOMPARE(str, expected);
QCOMPARE(str.isEmpty(), expected.isEmpty());
// A detached string is not null
// QCOMPARE(str.isNull(), expected.isNull());
}
class tst_QString : public QObject
@ -2870,43 +2881,127 @@ void tst_QString::insert_data(DataOptions options)
void tst_QString::insert_special_cases()
{
QString a;
QString dummy_share;
a = "Ys";
QCOMPARE(a.insert(1,'e'), QString("Yes"));
QCOMPARE(a.insert(3,'!'), QString("Yes!"));
QCOMPARE(a.insert(5,'?'), QString("Yes! ?"));
QCOMPARE(a.insert(-1,'a'), QString("Yes! a?"));
{
// Test when string is not shared
a = "Ys";
QCOMPARE(a.insert(1,'e'), QString("Yes"));
QCOMPARE(a.insert(3,'!'), QString("Yes!"));
QCOMPARE(a.insert(5,'?'), QString("Yes! ?"));
QCOMPARE(a.insert(-1,'a'), QString("Yes! a?"));
}
{
// Test when string is shared
a = "Ys";
dummy_share = a;
QCOMPARE(a.insert(1,'e'), QString("Yes"));
dummy_share = a;
QCOMPARE(a.insert(3,'!'), QString("Yes!"));
dummy_share = a;
QCOMPARE(a.insert(5,'?'), QString("Yes! ?"));
dummy_share = a;
QCOMPARE(a.insert(-1,'a'), QString("Yes! a?"));
}
a = "ABC";
QCOMPARE(a.insert(5,"DEF"), QString("ABC DEF"));
dummy_share = a;
QCOMPARE(dummy_share.insert(5,"DEF"), QString("ABC DEF")); // Shared
QCOMPARE(a.insert(5,"DEF"), QString("ABC DEF")); // Not shared after dummy_shared.insert()
a = "ABC";
QCOMPARE(a.insert(2, QString()), QString("ABC"));
QCOMPARE(a.insert(0,"ABC"), QString("ABCABC"));
QCOMPARE(a, QString("ABCABC"));
QCOMPARE(a.insert(0,a), QString("ABCABCABCABC"));
{
// Test when string is not shared
a = "ABC";
QCOMPARE(a.insert(2, QString()), QString("ABC"));
QCOMPARE(a.insert(0,"ABC"), QString("ABCABC"));
QCOMPARE(a, QString("ABCABC"));
QCOMPARE(a.insert(0,a), QString("ABCABCABCABC"));
QCOMPARE(a, QString("ABCABCABCABC"));
QCOMPARE(a.insert(0,'<'), QString("<ABCABCABCABC"));
QCOMPARE(a.insert(1,'>'), QString("<>ABCABCABCABC"));
QCOMPARE(a, QString("ABCABCABCABC"));
QCOMPARE(a.insert(0,'<'), QString("<ABCABCABCABC"));
QCOMPARE(a.insert(1,'>'), QString("<>ABCABCABCABC"));
}
{
// Test when string is shared
a = "ABC";
dummy_share = a;
QCOMPARE(a.insert(2, QString()), QString("ABC"));
dummy_share = a;
QCOMPARE(a.insert(0,"ABC"), QString("ABCABC"));
dummy_share = a;
QCOMPARE(a, QString("ABCABC"));
dummy_share = a;
QCOMPARE(a.insert(0,a), QString("ABCABCABCABC"));
QCOMPARE(a, QString("ABCABCABCABC"));
dummy_share = a;
QCOMPARE(a.insert(0,'<'), QString("<ABCABCABCABC"));
dummy_share = a;
QCOMPARE(a.insert(1,'>'), QString("<>ABCABCABCABC"));
}
a = "Meal";
const QString montreal = QStringLiteral("Montreal");
QCOMPARE(a.insert(1, QLatin1String("ontr")), montreal);
QCOMPARE(a.insert(4, ""), montreal);
QCOMPARE(a.insert(3, QLatin1String("")), montreal);
QCOMPARE(a.insert(3, QLatin1String(nullptr)), montreal);
QCOMPARE(a.insert(3, static_cast<const char *>(0)), montreal);
QCOMPARE(a.insert(0, QLatin1String("a")), QLatin1String("aMontreal"));
{
// Test when string is not shared
a = "Meal";
QCOMPARE(a.insert(1, QLatin1String("ontr")), montreal);
QCOMPARE(a.insert(4, ""), montreal);
QCOMPARE(a.insert(3, QLatin1String("")), montreal);
QCOMPARE(a.insert(3, QLatin1String(nullptr)), montreal);
QCOMPARE(a.insert(3, static_cast<const char *>(0)), montreal);
QCOMPARE(a.insert(0, QLatin1String("a")), QLatin1String("aMontreal"));
}
{
// Test when string is shared
a = "Meal";
dummy_share = a;
QCOMPARE(a.insert(1, QLatin1String("ontr")), montreal);
dummy_share = a;
QCOMPARE(a.insert(4, ""), montreal);
dummy_share = a;
QCOMPARE(a.insert(3, QLatin1String("")), montreal);
dummy_share = a;
QCOMPARE(a.insert(3, QLatin1String(nullptr)), montreal);
dummy_share = a;
QCOMPARE(a.insert(3, static_cast<const char *>(0)), montreal);
dummy_share = a;
QCOMPARE(a.insert(0, QLatin1String("a")), QLatin1String("aMontreal"));
}
a = "Mont";
QCOMPARE(a.insert(a.size(), QLatin1String("real")), montreal);
QCOMPARE(a.insert(a.size() + 1, QLatin1String("ABC")), QString("Montreal ABC"));
{
// Test when string is not shared
a = "Mont";
QCOMPARE(a.insert(a.size(), QLatin1String("real")), montreal);
QCOMPARE(a.insert(a.size() + 1, QLatin1String("ABC")), QString("Montreal ABC"));
}
{
// Test when string is shared
a = "Mont";
dummy_share = a;
QCOMPARE(a.insert(a.size(), QLatin1String("real")), montreal);
dummy_share = a;
QCOMPARE(a.insert(a.size() + 1, QLatin1String("ABC")), QString("Montreal ABC"));
}
{
// Test when string is not shared
a = "AEF";
QCOMPARE(a.insert(1, QLatin1String("BCD")), QString("ABCDEF"));
QCOMPARE(a.insert(3, QLatin1String("-")), QString("ABC-DEF"));
QCOMPARE(a.insert(a.size() + 1, QLatin1String("XYZ")), QString("ABC-DEF XYZ"));
}
{
// Test when string is shared
a = "AEF";
dummy_share = a ;
QCOMPARE(a.insert(1, QLatin1String("BCD")), QString("ABCDEF"));
dummy_share = a ;
QCOMPARE(a.insert(3, QLatin1String("-")), QString("ABC-DEF"));
dummy_share = a ;
QCOMPARE(a.insert(a.size() + 1, QLatin1String("XYZ")), QString("ABC-DEF XYZ"));
}
a = "AEF";
QCOMPARE(a.insert(1, QLatin1String("BCD")), QString("ABCDEF"));
QCOMPARE(a.insert(3, QLatin1String("-")), QString("ABC-DEF"));
QCOMPARE(a.insert(a.size() + 1, QLatin1String("XYZ")), QString("ABC-DEF XYZ"));
{
a = "one";
@ -2914,6 +3009,12 @@ void tst_QString::insert_special_cases()
QString b(a.data_ptr()->freeSpaceAtEnd(), u'b');
QCOMPARE(a.insert(a.size() + 1, QLatin1String(b.toLatin1())), QString("aone ") + b);
}
{
a = "one";
a.prepend(u'a');
QString b(a.data_ptr()->freeSpaceAtEnd(), u'b');
QCOMPARE(a.insert(a.size() + 1, b), QString("aone ") + b);
}
{
a = "onetwothree";
@ -2922,14 +3023,6 @@ void tst_QString::insert_special_cases()
QString b(a.data_ptr()->freeSpaceAtEnd() + 1, u'b');
QCOMPARE(a.insert(a.size() + 1, QLatin1String(b.toLatin1())), QString("e ") + b);
}
{
a = "one";
a.prepend(u'a');
QString b(a.data_ptr()->freeSpaceAtEnd(), u'b');
QCOMPARE(a.insert(a.size() + 1, b), QString("aone ") + b);
}
{
a = "onetwothree";
while (a.size() - 1)