From a6ad755734930fabec6d91c6a516725e80a3084a Mon Sep 17 00:00:00 2001 From: Ahmad Samir Date: Fri, 5 May 2023 20:12:27 +0300 Subject: [PATCH] QStringList: add filter(QStringMatcher) overload Now that users can pass a QStringMatcher to do the matching, change the existing overload to not use QStringMatcher. Thanks to Giuseppe D'Angelo for the idea of passing a QStringMatcher to filter instead of using a magic number to decide whether to use QStringMatcher or not. Results of running filter() and filter_stringMatcher, times are in msecs and this was compiled with gcc -O3: Without With QStringMatcher list10 0.00022 0.000089 list20 0.00040 0.00014 list30 0.00058 0.00018 list40 0.000770 0.00023 list50 0.00094 0.00027 list70 0.0012 0.00037 list80 0.0014 0.00041 list100 0.0018 0.00050 list300 0.0054 0.0014 list500 0.0091 0.0023 list700 0.012 0.0032 list900 0.016 0.0041 list10000 0.17 0.045 Drive-by change: optimize tst_QStringList::populateList(). [ChangeLog][QtCore][QStringList] Added filter(const QStringMatcher &) overload, which may be faster for large lists and/or lists with very long strings. [ChangeLog][Possible Performance Changes][QtCore][QStringList] Changed the implementation of filter(QStringView) overload to not use QStringMatcher by default. Using QStringMatcher adds overhead, so it is beneficial/faster when searching for a pattern in large lists and/or lists with long strings, otherwise using plain string comparison is faster. If using QStringMatcher makes a difference in your code, you can use the newly added filter(QStringMatcher) overload. Change-Id: I7bb1262706d673f0ce0d9b7699f03c995ce28677 Reviewed-by: Thiago Macieira --- src/corelib/doc/snippets/qstringlist/main.cpp | 7 +++ src/corelib/text/qstringlist.cpp | 36 +++++++++++++- src/corelib/text/qstringlist.h | 5 ++ .../text/qstringlist/tst_qstringlist.cpp | 2 + .../qstringlist/tst_bench_qstringlist.cpp | 47 ++++++++++++++++--- 5 files changed, 89 insertions(+), 8 deletions(-) diff --git a/src/corelib/doc/snippets/qstringlist/main.cpp b/src/corelib/doc/snippets/qstringlist/main.cpp index e9567fb1077..1b7453cf6a0 100644 --- a/src/corelib/doc/snippets/qstringlist/main.cpp +++ b/src/corelib/doc/snippets/qstringlist/main.cpp @@ -93,6 +93,13 @@ Widget::Widget(QWidget *parent) // list == ["Bill Clinton", "Bill Murray"] //! [17] + { +//! [18] + QStringList veryLongList; + QStringMatcher matcher(u"Straße", Qt::CaseInsensitive); + QStringList filtered = veryLongList.filter(matcher); +//! [18] + } } int main(int argc, char *argv[]) diff --git a/src/corelib/text/qstringlist.cpp b/src/corelib/text/qstringlist.cpp index 6790c8cbee1..7f2572838f7 100644 --- a/src/corelib/text/qstringlist.cpp +++ b/src/corelib/text/qstringlist.cpp @@ -237,6 +237,17 @@ void QtPrivate::QStringList_sort(QStringList *that, Qt::CaseSensitivity cs) \sa contains() */ +template +static QStringList filter_helper(const QStringList &that, const String &needle, Qt::CaseSensitivity cs) +{ + QStringList res; + for (const auto &s : that) { + if (s.contains(needle, cs)) + res.append(s); + } + return res; +} + /*! \fn QStringList QStringList::filter(QStringView str, Qt::CaseSensitivity cs) const \overload @@ -245,9 +256,30 @@ void QtPrivate::QStringList_sort(QStringList *that, Qt::CaseSensitivity cs) QStringList QtPrivate::QStringList_filter(const QStringList *that, QStringView str, Qt::CaseSensitivity cs) { - QStringMatcher matcher(str, cs); + return filter_helper(*that, str, cs); +} + +/*! + \fn QStringList QStringList::filter(const QStringMatcher &matcher) const + \since 6.7 + \overload + + Returns a list of all the strings matched by \a matcher (i.e. for which + \c matcher.indexIn() returns an index >= 0). + + Using a QStringMatcher may be faster when searching in large lists and/or + in lists with long strings (the best way to find out is benchmarking). + + For example: + \snippet qstringlist/main.cpp 18 + + \sa contains() +*/ + +QStringList QtPrivate::QStringList_filter(const QStringList &that, const QStringMatcher &matcher) +{ QStringList res; - for (const auto &s : *that) { + for (const auto &s : that) { if (matcher.indexIn(s) != -1) res.append(s); } diff --git a/src/corelib/text/qstringlist.h b/src/corelib/text/qstringlist.h index 31e4cf6dc9d..99f51667ce0 100644 --- a/src/corelib/text/qstringlist.h +++ b/src/corelib/text/qstringlist.h @@ -30,6 +30,9 @@ namespace QtPrivate { Q_CORE_EXPORT QString QStringList_join(const QStringList &list, QLatin1StringView sep); QStringList Q_CORE_EXPORT QStringList_filter(const QStringList *that, QStringView str, Qt::CaseSensitivity cs); + Q_CORE_EXPORT QStringList QStringList_filter(const QStringList &that, + const QStringMatcher &matcher); + bool Q_CORE_EXPORT QStringList_contains(const QStringList *that, QStringView str, Qt::CaseSensitivity cs); bool Q_CORE_EXPORT QStringList_contains(const QStringList *that, QLatin1StringView str, Qt::CaseSensitivity cs); void Q_CORE_EXPORT QStringList_replaceInStrings(QStringList *that, QStringView before, QStringView after, @@ -78,6 +81,8 @@ public: inline QString join(QChar sep) const { return QtPrivate::QStringList_join(self(), &sep, 1); } + QStringList filter(const QStringMatcher &matcher) const + { return QtPrivate::QStringList_filter(*self(), matcher); } inline QStringList filter(QStringView str, Qt::CaseSensitivity cs = Qt::CaseSensitive) const { return QtPrivate::QStringList_filter(self(), str, cs); } inline QStringList &replaceInStrings(QStringView before, QStringView after, Qt::CaseSensitivity cs = Qt::CaseSensitive) diff --git a/tests/auto/corelib/text/qstringlist/tst_qstringlist.cpp b/tests/auto/corelib/text/qstringlist/tst_qstringlist.cpp index 573357abd2d..a20e4be1450 100644 --- a/tests/auto/corelib/text/qstringlist/tst_qstringlist.cpp +++ b/tests/auto/corelib/text/qstringlist/tst_qstringlist.cpp @@ -161,6 +161,7 @@ void tst_QStringList::filter() QCOMPARE(list.filter(u"Bill"_s), expected); QCOMPARE(list.filter(u"Bill"), expected); QCOMPARE(list.filter(QRegularExpression(u"[i]ll"_s)), expected); + QCOMPARE(list.filter(QStringMatcher(u"Bill")), expected); } { // CaseInsensitive @@ -169,6 +170,7 @@ void tst_QStringList::filter() QCOMPARE(list.filter(u"bill", Qt::CaseInsensitive), expected); QCOMPARE(list.filter(QRegularExpression(u"[i]ll"_s, QRegularExpression::CaseInsensitiveOption)), expected); + QCOMPARE(list.filter(QStringMatcher(u"Bill", Qt::CaseInsensitive)), expected); } } diff --git a/tests/benchmarks/corelib/text/qstringlist/tst_bench_qstringlist.cpp b/tests/benchmarks/corelib/text/qstringlist/tst_bench_qstringlist.cpp index ee0789261ca..fae879ce373 100644 --- a/tests/benchmarks/corelib/text/qstringlist/tst_bench_qstringlist.cpp +++ b/tests/benchmarks/corelib/text/qstringlist/tst_bench_qstringlist.cpp @@ -8,6 +8,8 @@ #include #include +using namespace Qt::StringLiterals; + class tst_QStringList: public QObject { Q_OBJECT @@ -19,6 +21,11 @@ private slots: void removeDuplicates() const; void removeDuplicates_data() const; + void filter_data() const; + void filter() const; + void filter_stringMatcher_data() const { filter_data(); } + void filter_stringMatcher() const; + void split_qlist_qbytearray() const; void split_qlist_qbytearray_data() const { return split_data(); } @@ -42,12 +49,7 @@ private: QStringList tst_QStringList::populateList(const int count, const QString &unit) { - QStringList retval; - - for (int i = 0; i < count; ++i) - retval.append(unit); - - return retval; + return QStringList(count, unit); } QString tst_QStringList::populateString(const int count, const QString &unit) @@ -130,6 +132,39 @@ void tst_QStringList::removeDuplicates_data() const QTest::addRow("long-dup-0.75") << (l + l + l + l); } +void tst_QStringList::filter_data() const +{ + QTest::addColumn("list"); + QTest::addColumn("expected"); + + for (int i : {10, 20, 30, 40, 50, 70, 80, 100, 300, 500, 700, 900, 10'000}) { + QStringList list = populateList(i, u"A rather long string to test QStringMatcher"_s); + list.append(u"Horse and cart from old"_s); + QTest::addRow("list%d", i) << list << QStringList(u"Horse and cart from old"_s); + } +} + +void tst_QStringList::filter() const +{ + QFETCH(QStringList, list); + QFETCH(QStringList, expected); + + QBENCHMARK { + QCOMPARE(list.filter(u"Horse and cart from old", Qt::CaseSensitive), expected); + } +} + +void tst_QStringList::filter_stringMatcher() const +{ + QFETCH(QStringList, list); + QFETCH(QStringList, expected); + + const QStringMatcher matcher(u"Horse and cart from old", Qt::CaseSensitive); + QBENCHMARK { + QCOMPARE(list.filter(matcher), expected); + } +} + void tst_QStringList::split_data() const { QTest::addColumn("input");