From adf30afcb0ca46b6eedf9bdedbd9275f5b52e4f4 Mon Sep 17 00:00:00 2001 From: Rex Dieter Date: Feb 01 2015 04:05:52 +0000 Subject: latest 1.13 branch fixes --- diff --git a/0001-FindSqlite-Use-CMAKE_FLAGS-the-right-way-in-try_comp.patch b/0001-FindSqlite-Use-CMAKE_FLAGS-the-right-way-in-try_comp.patch index 0bad3aa..c7f6ea7 100644 --- a/0001-FindSqlite-Use-CMAKE_FLAGS-the-right-way-in-try_comp.patch +++ b/0001-FindSqlite-Use-CMAKE_FLAGS-the-right-way-in-try_comp.patch @@ -1,7 +1,7 @@ From b60702e0b7041c56a3cb52c209204d28406f3ce5 Mon Sep 17 00:00:00 2001 From: Raphael Kubo da Costa Date: Wed, 13 Aug 2014 14:43:04 +0300 -Subject: [PATCH 1/5] FindSqlite: Use CMAKE_FLAGS the right way in +Subject: [PATCH 01/30] FindSqlite: Use CMAKE_FLAGS the right way in try_compile(). This fixes f90774f1 ("Check whether Sqlite is compiled with @@ -34,5 +34,5 @@ index ad8cdb4..c43a7b5 100644 message(STATUS "Sqlite ${SQLITE_VERSION} was found, but it is not compiled with -DSQLITE_ENABLE_UNLOCK_NOTIFY") endif() -- -1.9.3 +2.1.0 diff --git a/0002-Do-not-enter-the-test-directories-if-AKONADI_BUILD_T.patch b/0002-Do-not-enter-the-test-directories-if-AKONADI_BUILD_T.patch index 8efd56e..e5088e6 100644 --- a/0002-Do-not-enter-the-test-directories-if-AKONADI_BUILD_T.patch +++ b/0002-Do-not-enter-the-test-directories-if-AKONADI_BUILD_T.patch @@ -1,8 +1,8 @@ From 2146519108ec66300328b7b3979477c7789795d3 Mon Sep 17 00:00:00 2001 From: Raphael Kubo da Costa Date: Wed, 13 Aug 2014 23:22:11 +0300 -Subject: [PATCH 2/5] Do not enter the test/ directories if AKONADI_BUILD_TESTS - is off. +Subject: [PATCH 02/30] Do not enter the test/ directories if + AKONADI_BUILD_TESTS is off. enable_testing() only determines whether a "test" target and the related CTest files will be created. And in Akonadi's case it is actually @@ -78,5 +78,5 @@ index e4829f3..275938d 100644 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${_ENABLE_EXCEPTIONS}") if(MYSQLD_EXECUTABLE) -- -1.9.3 +2.1.0 diff --git a/0003-STORE-Allow-modifying-items-tags-via-Tag-RID-or-GID.patch b/0003-STORE-Allow-modifying-items-tags-via-Tag-RID-or-GID.patch index 8538502..e2a495c 100644 --- a/0003-STORE-Allow-modifying-items-tags-via-Tag-RID-or-GID.patch +++ b/0003-STORE-Allow-modifying-items-tags-via-Tag-RID-or-GID.patch @@ -1,7 +1,7 @@ From 9734074267bacd39aeb29c7a0d7df7cadb212d89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20Vr=C3=A1til?= Date: Fri, 11 Jul 2014 18:33:39 +0200 -Subject: [PATCH 3/5] STORE: Allow modifying items tags via Tag RID or GID +Subject: [PATCH 03/30] STORE: Allow modifying items tags via Tag RID or GID Tags RID is of course allowed only to resources @@ -197,5 +197,5 @@ index 763ea30..634a26c 100644 throw HandlerException("Unable to resolve tags"); } -- -1.9.3 +2.1.0 diff --git a/0004-Fix-typo-in-if-condition.patch b/0004-Fix-typo-in-if-condition.patch index b3febdc..b0ae0da 100644 --- a/0004-Fix-typo-in-if-condition.patch +++ b/0004-Fix-typo-in-if-condition.patch @@ -1,7 +1,7 @@ From e52f9be20e566e507e77421f1243f51aa2fe8e55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20Vr=C3=A1til?= Date: Mon, 25 Aug 2014 14:35:14 +0200 -Subject: [PATCH 4/5] Fix typo in if condition +Subject: [PATCH 04/30] Fix typo in if condition BUG: 338483 FIXED-IN: 1.13.1 @@ -23,5 +23,5 @@ index 43f03ba..ad3682f 100644 } -- -1.9.3 +2.1.0 diff --git a/0005-Fix-buffer-overflow-in-AKTEST_FAKESERVER_MAIN.patch b/0005-Fix-buffer-overflow-in-AKTEST_FAKESERVER_MAIN.patch index 79ca35f..0203e45 100644 --- a/0005-Fix-buffer-overflow-in-AKTEST_FAKESERVER_MAIN.patch +++ b/0005-Fix-buffer-overflow-in-AKTEST_FAKESERVER_MAIN.patch @@ -1,7 +1,7 @@ From 01c86229f9e26d9e036f6f2ab405659ed836b5c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20Vr=C3=A1til?= Date: Mon, 8 Sep 2014 15:36:18 +0200 -Subject: [PATCH 5/9] Fix buffer overflow in AKTEST_FAKESERVER_MAIN() +Subject: [PATCH 05/30] Fix buffer overflow in AKTEST_FAKESERVER_MAIN() --- shared/aktest.h | 2 +- @@ -21,5 +21,5 @@ index b1b9caa..3026304 100644 fakeArgv[i] = options[i]; \ } \ -- -1.9.3 +2.1.0 diff --git a/0006-Don-t-crash-when-setmntent-returns-NULL.patch b/0006-Don-t-crash-when-setmntent-returns-NULL.patch index 892b402..e9f2e4c 100644 --- a/0006-Don-t-crash-when-setmntent-returns-NULL.patch +++ b/0006-Don-t-crash-when-setmntent-returns-NULL.patch @@ -1,7 +1,7 @@ From ca59eb345cfef368242929ea33beca4bff837e9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20Vr=C3=A1til?= Date: Thu, 18 Sep 2014 16:54:26 +0200 -Subject: [PATCH 6/9] Don't crash when setmntent returns NULL +Subject: [PATCH 06/30] Don't crash when setmntent returns NULL setmntent can fail when there's no /etc/mtab file for instance and passing NULL pointer to getmntent crashes, so we need to return when @@ -25,5 +25,5 @@ index b04a812..b51c330 100644 if (qstrcmp(mnt->mnt_type, MNTTYPE_IGNORE) == 0) { continue; -- -1.9.3 +2.1.0 diff --git a/0007-Don-t-call-insert-from-Q_ASSERT-breaks-unit-tests-in.patch b/0007-Don-t-call-insert-from-Q_ASSERT-breaks-unit-tests-in.patch index 5335649..15386e4 100644 --- a/0007-Don-t-call-insert-from-Q_ASSERT-breaks-unit-tests-in.patch +++ b/0007-Don-t-call-insert-from-Q_ASSERT-breaks-unit-tests-in.patch @@ -1,8 +1,8 @@ From c516ec5c28d603aea0df6165f66a3a5d0a0191c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20Vr=C3=A1til?= Date: Fri, 19 Sep 2014 10:50:23 +0200 -Subject: [PATCH 7/9] Don't call insert() from Q_ASSERT - breaks unit-tests in - Release mode +Subject: [PATCH 07/30] Don't call insert() from Q_ASSERT - breaks unit-tests + in Release mode --- server/tests/unittest/collectionreferencetest.cpp | 6 ++++-- @@ -33,5 +33,5 @@ index 1700c75..1b10c55 100644 } -- -1.9.3 +2.1.0 diff --git a/0008-Suppress-unused-variable-warnings-in-release-mode.patch b/0008-Suppress-unused-variable-warnings-in-release-mode.patch index 91fe98a..1f4c67e 100644 --- a/0008-Suppress-unused-variable-warnings-in-release-mode.patch +++ b/0008-Suppress-unused-variable-warnings-in-release-mode.patch @@ -1,7 +1,7 @@ From b35fcb64c3ba3df95f62d0d129adb791ce2bad15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20Vr=C3=A1til?= Date: Fri, 19 Sep 2014 11:10:13 +0200 -Subject: [PATCH 8/9] Suppress unused variable warnings in release mode +Subject: [PATCH 08/30] Suppress unused variable warnings in release mode --- server/tests/unittest/collectionreferencetest.cpp | 2 ++ @@ -28,5 +28,5 @@ index 1b10c55..9c98f28 100644 } -- -1.9.3 +2.1.0 diff --git a/0009-Test-whether-compiler-supports-all-required-C-11-fea.patch b/0009-Test-whether-compiler-supports-all-required-C-11-fea.patch index f965f37..7bd55ce 100644 --- a/0009-Test-whether-compiler-supports-all-required-C-11-fea.patch +++ b/0009-Test-whether-compiler-supports-all-required-C-11-fea.patch @@ -1,7 +1,7 @@ From e1c69c277ea6005cc358434679b83fa1cb752756 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20Vr=C3=A1til?= Date: Tue, 23 Sep 2014 18:00:34 +0200 -Subject: [PATCH 9/9] Test whether compiler supports all required C++11 +Subject: [PATCH 09/30] Test whether compiler supports all required C++11 features at configure time To prevent ugly compilation errors when someone tries to compile Akonadi @@ -41,5 +41,5 @@ index e081d23..2d790c9 100644 set(CMAKE_CXX_FLAGS_DEBUGFULL "-g3 -fno-inline" CACHE STRING "Flags used by the C++ compiler during debugfull builds." FORCE) set(CMAKE_C_FLAGS_DEBUGFULL "-g3 -fno-inline" CACHE STRING "Flags used by the C compiler during debugfull builds." FORCE) -- -1.9.3 +2.1.0 diff --git a/0010-prevent-starting-a-QTimer-with-a-negative-interval.patch b/0010-prevent-starting-a-QTimer-with-a-negative-interval.patch new file mode 100644 index 0000000..59b52a3 --- /dev/null +++ b/0010-prevent-starting-a-QTimer-with-a-negative-interval.patch @@ -0,0 +1,27 @@ +From de9bd9043e8878fc472ced1669bc7d49b07c2062 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Ren=C3=A9=20J=2EV=2E=20Bertin?= +Date: Mon, 3 Nov 2014 16:56:56 +0100 +Subject: [PATCH 10/30] prevent starting a QTimer with a negative interval + Review: 120800 + +--- + server/src/collectionscheduler.cpp | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) + +diff --git a/server/src/collectionscheduler.cpp b/server/src/collectionscheduler.cpp +index 8d4cd5c..9ba632f 100644 +--- a/server/src/collectionscheduler.cpp ++++ b/server/src/collectionscheduler.cpp +@@ -82,7 +82,8 @@ class PauseableTimer : public QTimer + return; + } + +- start( interval() - ( mStarted.secsTo( mPaused ) * 1000 ) ); ++ const int remainder = interval() - ( mStarted.secsTo( mPaused ) * 1000 ); ++ start( qMax( 0, remainder ) ); + mPaused = QDateTime(); + // Update mStarted so that pause() can be called repeatedly + mStarted = QDateTime::currentDateTime(); +-- +2.1.0 + diff --git a/0011-Convert-some-qDebugs-to-akDebugs.patch b/0011-Convert-some-qDebugs-to-akDebugs.patch new file mode 100644 index 0000000..f4ee199 --- /dev/null +++ b/0011-Convert-some-qDebugs-to-akDebugs.patch @@ -0,0 +1,119 @@ +From 1d79c645ffbd858517f07cee3143dc64fac7c3e9 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Dan=20Vr=C3=A1til?= +Date: Mon, 10 Nov 2014 11:51:45 +0100 +Subject: [PATCH 11/30] Convert some qDebugs to akDebugs + +This should make Akonadi in release mode even less chatty. +--- + server/src/handler/merge.cpp | 3 ++- + server/src/handler/modify.cpp | 6 +++--- + server/src/handler/remove.cpp | 4 ++-- + server/src/search/searchmanager.cpp | 12 ++++++------ + 4 files changed, 13 insertions(+), 12 deletions(-) + +diff --git a/server/src/handler/merge.cpp b/server/src/handler/merge.cpp +index fffe100..c26917d 100644 +--- a/server/src/handler/merge.cpp ++++ b/server/src/handler/merge.cpp +@@ -328,8 +328,9 @@ bool Merge::parseStream() + } + + } else { ++ akDebug() << "Multiple merge candidates:"; + Q_FOREACH (const PimItem &item, result) { +- qDebug() << item.id() << item.remoteId() << item.gid(); ++ akDebug() << "\t" << item.id() << item.remoteId() << item.gid(); + } + // Nor GID or RID are guaranteed to be unique, so make sure we don't merge + // something we don't want +diff --git a/server/src/handler/modify.cpp b/server/src/handler/modify.cpp +index 9671fb9..ad329db 100644 +--- a/server/src/handler/modify.cpp ++++ b/server/src/handler/modify.cpp +@@ -216,9 +216,9 @@ bool Modify::parseStream() + + queryAttributes = attrs.join( QLatin1String( " " ) ); + +- qDebug() << collection.queryAttributes() << queryAttributes; +- qDebug() << collection.queryCollections() << queryCollections; +- qDebug() << collection.queryString() << queryString; ++ akDebug() << collection.queryAttributes() << queryAttributes; ++ akDebug() << collection.queryCollections() << queryCollections; ++ akDebug() << collection.queryString() << queryString; + + if ( collection.queryAttributes() != queryAttributes + || collection.queryCollections() != queryCollections +diff --git a/server/src/handler/remove.cpp b/server/src/handler/remove.cpp +index 090531f..daec5a0 100644 +--- a/server/src/handler/remove.cpp ++++ b/server/src/handler/remove.cpp +@@ -40,8 +40,8 @@ bool Remove::parseStream() + { + mScope.parseScope( m_streamParser ); + connection()->context()->parseContext( m_streamParser ); +- qDebug() << "Tag context:" << connection()->context()->tagId(); +- qDebug() << "Collection context: " << connection()->context()->collectionId(); ++ akDebug() << "Tag context:" << connection()->context()->tagId(); ++ akDebug() << "Collection context: " << connection()->context()->collectionId(); + + + SelectQueryBuilder qb; +diff --git a/server/src/search/searchmanager.cpp b/server/src/search/searchmanager.cpp +index 35e76e1..c821aa3 100644 +--- a/server/src/search/searchmanager.cpp ++++ b/server/src/search/searchmanager.cpp +@@ -159,7 +159,7 @@ void SearchManager::loadSearchPlugins() + Q_FOREACH ( const QString &pluginDir, dirs ) { + QDir dir( pluginDir + QLatin1String( "/akonadi" ) ); + const QStringList desktopFiles = dir.entryList( QStringList() << QLatin1String( "*.desktop" ), QDir::Files ); +- qDebug() << "SEARCH MANAGER: searching in " << pluginDir + QLatin1String( "/akonadi" ) << ":" << desktopFiles; ++ akDebug() << "SEARCH MANAGER: searching in " << pluginDir + QLatin1String( "/akonadi" ) << ":" << desktopFiles; + + Q_FOREACH ( const QString &desktopFileName, desktopFiles ) { + QSettings desktop( pluginDir + QLatin1String( "/akonadi/" ) + desktopFileName, QSettings::IniFormat ); +@@ -170,13 +170,13 @@ void SearchManager::loadSearchPlugins() + + const QString libraryName = desktop.value( QLatin1String( "X-Akonadi-Library" ) ).toString(); + if ( loadedPlugins.contains( libraryName ) ) { +- qDebug() << "Already loaded one version of this plugin, skipping: " << libraryName; ++ akDebug() << "Already loaded one version of this plugin, skipping: " << libraryName; + continue; + } + // When search plugin override is active, ignore all plugins except for the override + if ( !pluginOverride.isEmpty() ) { + if ( libraryName != pluginOverride ) { +- qDebug() << desktopFileName << "skipped because of AKONADI_OVERRIDE_SEARCHPLUGIN"; ++ akDebug() << desktopFileName << "skipped because of AKONADI_OVERRIDE_SEARCHPLUGIN"; + continue; + } + +@@ -198,7 +198,7 @@ void SearchManager::loadSearchPlugins() + continue; + } + +- qDebug() << "SearchManager: loaded search plugin" << libraryName; ++ akDebug() << "SearchManager: loaded search plugin" << libraryName; + mPlugins << plugin; + loadedPlugins << libraryName; + } +@@ -390,7 +390,7 @@ void SearchManager::searchUpdateResultsAvailable( const QSet &results ) + } + } + +- qDebug() << "Got" << newMatches.count() << "results, out of which" << existingMatches.count() << "are already in the collection"; ++ akDebug() << "Got" << newMatches.count() << "results, out of which" << existingMatches.count() << "are already in the collection"; + + newMatches = newMatches - existingMatches; + +@@ -405,7 +405,7 @@ void SearchManager::searchUpdateResultsAvailable( const QSet &results ) + Collection::addPimItem( collection.id(), id ); + } + +- qDebug() << "Added" << newMatches.count(); ++ akDebug() << "Added" << newMatches.count(); + + if ( !existingTransaction && !DataStore::self()->commitTransaction() ) { + akDebug() << "Failed to commit transaction"; +-- +2.1.0 + diff --git a/0012-Optimize-Reduce-the-amount-of-allocations-required-t.patch b/0012-Optimize-Reduce-the-amount-of-allocations-required-t.patch new file mode 100644 index 0000000..2b42a63 --- /dev/null +++ b/0012-Optimize-Reduce-the-amount-of-allocations-required-t.patch @@ -0,0 +1,604 @@ +From 63f49d233ca8a4fdd3e8937ea1c80d5e57a1cbdc Mon Sep 17 00:00:00 2001 +From: Milian Wolff +Date: Tue, 25 Nov 2014 20:16:41 +0100 +Subject: [PATCH 12/30] Optimize: Reduce the amount of allocations required to + build a query. + +The initial implementation of the QueryBuilder was quite naive, when +you look at the amount of string allocations it does to build the +final query we sent to the SQL server. + +This was found with Linux perf (no, not even heaptrack!). It +showed a huge number of cycles spent in malloc/free, all called +eventually by the QueryBuilder. + +This patch removes most of these allocations. It can further be +improved in the future, I bet. Also, the amount of queries we create +is pretty large. I guess using stored procedures or something similar +might also help the performance. At least, we should try to "remember" +some of our queries, and make it possible to reuse them in the +functions that run often. + +The added benchmark shows that the cost is not as big as I'd initially +assumed. There are simply many more allocation occurrences in Akonadi +currently. Still, I think it's worth it, as it also decreases the +memory fragmentation and improves cache locality: + +Before: +RESULT : QueryBuilderTest::benchQueryBuilder(): + 0.0115 msecs per iteration (total: 116, iterations: 10000) + +113.10MB bytes allocated in total (ignoring deallocations) +over 1203089 calls to allocation functions. +peak heap memory consumption: 254.46KB + +After: +RESULT : QueryBuilderTest::benchQueryBuilder(): + 0.0065 msecs per iteration (total: 66, iterations: 10000) + +62.42MB bytes allocated in total (ignoring deallocations) +over 343089 calls to allocation functions. +peak heap memory consumption: 254.96KB + +So before, we had approx. 60 allocations per query build in the +benchmark (note that Qt for some reason executes the loop twice, +so while the time is measured for 10k iterations, heaptrack will +see 20k). With this patch applied, we only need ~20 allocations +per query we build up. + +The remaining allocations are the various append operations to +the QList/QVectors mostly, as well as QueryBuilder::addAggregation. + +REVIEW: 121247 +--- + server/src/storage/querybuilder.cpp | 210 ++++++++++++++++------------- + server/src/storage/querybuilder.h | 14 +- + server/tests/unittest/querybuildertest.cpp | 58 ++++++-- + server/tests/unittest/querybuildertest.h | 2 + + 4 files changed, 173 insertions(+), 111 deletions(-) + +diff --git a/server/src/storage/querybuilder.cpp b/server/src/storage/querybuilder.cpp +index c079059..3017867 100644 +--- a/server/src/storage/querybuilder.cpp ++++ b/server/src/storage/querybuilder.cpp +@@ -31,7 +31,7 @@ + + using namespace Akonadi::Server; + +-static QString compareOperatorToString( Query::CompareOperator op ) ++static QLatin1String compareOperatorToString( Query::CompareOperator op ) + { + switch ( op ) { + case Query::Equals: +@@ -58,10 +58,10 @@ static QString compareOperatorToString( Query::CompareOperator op ) + return QLatin1String( " LIKE " ); + } + Q_ASSERT_X( false, "QueryBuilder::compareOperatorToString()", "Unknown compare operator." ); +- return QString(); ++ return QLatin1String(""); + } + +-static QString logicOperatorToString( Query::LogicOperator op ) ++static QLatin1String logicOperatorToString( Query::LogicOperator op ) + { + switch ( op ) { + case Query::And: +@@ -70,10 +70,10 @@ static QString logicOperatorToString( Query::LogicOperator op ) + return QLatin1String( " OR " ); + } + Q_ASSERT_X( false, "QueryBuilder::logicOperatorToString()", "Unknown logic operator." ); +- return QString(); ++ return QLatin1String(""); + } + +-static QString sortOrderToString( Query::SortOrder order ) ++static QLatin1String sortOrderToString( Query::SortOrder order ) + { + switch ( order ) { + case Query::Ascending: +@@ -82,7 +82,17 @@ static QString sortOrderToString( Query::SortOrder order ) + return QLatin1String( " DESC" ); + } + Q_ASSERT_X( false, "QueryBuilder::sortOrderToString()", "Unknown sort order." ); +- return QString(); ++ return QLatin1String(""); ++} ++ ++static void appendJoined( QString *statement, const QStringList &strings, const QLatin1String &glue = QLatin1String( ", " ) ) ++{ ++ for (int i = 0, c = strings.size(); i < c; ++i) { ++ *statement += strings.at( i ); ++ if (i + 1 < c) { ++ *statement += glue; ++ } ++ } + } + + QueryBuilder::QueryBuilder( const QString &table, QueryBuilder::QueryType type ) +@@ -94,10 +104,12 @@ QueryBuilder::QueryBuilder( const QString &table, QueryBuilder::QueryType type ) + , mDatabaseType( DbType::Unknown ) + #endif + , mType( type ) +- , mIdentificationColumn( QLatin1String( "id" ) ) ++ , mIdentificationColumn( ) + , mLimit( -1 ) + , mDistinct( false ) + { ++ static const QString defaultIdColumn = QLatin1String( "id" ); ++ mIdentificationColumn = defaultIdColumn; + } + + void QueryBuilder::setDatabaseType( DbType::Type type ) +@@ -175,60 +187,65 @@ void QueryBuilder::sqliteAdaptUpdateJoin( Query::Condition &condition ) + qb.addCondition( joinCondition.second ); + + // Convert the subquery to string +- condition.mColumn = QLatin1String( "( " ) + qb.buildQuery() + QLatin1String( " )" ); ++ condition.mColumn.reserve(1024); ++ condition.mColumn.resize(0); ++ condition.mColumn += QLatin1String( "( " ); ++ qb.buildQuery(&condition.mColumn); ++ condition.mColumn += QLatin1String( " )" ); + } + +- +-QString QueryBuilder::buildQuery() ++void QueryBuilder::buildQuery(QString *statement) + { +- QString statement; +- + // we add the ON conditions of Inner Joins in a Update query here + // but don't want to change the mRootCondition on each exec(). + Query::Condition whereCondition = mRootCondition[WhereCondition]; + + switch ( mType ) { + case Select: +- statement += QLatin1String( "SELECT " ); ++ *statement += QLatin1String( "SELECT " ); + if ( mDistinct ) { +- statement += QLatin1String( "DISTINCT " ); ++ *statement += QLatin1String( "DISTINCT " ); + } + Q_ASSERT_X( mColumns.count() > 0, "QueryBuilder::exec()", "No columns specified" ); +- statement += mColumns.join( QLatin1String( ", " ) ); +- statement += QLatin1String( " FROM " ); +- statement += mTable; ++ appendJoined( statement, mColumns ); ++ *statement += QLatin1String( " FROM " ); ++ *statement += mTable; + Q_FOREACH ( const QString &joinedTable, mJoinedTables ) { + const QPair &join = mJoins.value( joinedTable ); + switch ( join.first ) { + case LeftJoin: +- statement += QLatin1String( " LEFT JOIN " ); ++ *statement += QLatin1String( " LEFT JOIN " ); + break; + case InnerJoin: +- statement += QLatin1String( " INNER JOIN " ); ++ *statement += QLatin1String( " INNER JOIN " ); + break; + } +- statement += joinedTable; +- statement += QLatin1String( " ON " ); +- statement += buildWhereCondition( join.second ); ++ *statement += joinedTable; ++ *statement += QLatin1String( " ON " ); ++ buildWhereCondition( statement, join.second ); + } + break; + case Insert: + { +- statement += QLatin1String( "INSERT INTO " ); +- statement += mTable; +- statement += QLatin1String( " (" ); +- typedef QPair StringVariantPair; +- QStringList cols, vals; +- Q_FOREACH ( const StringVariantPair &p, mColumnValues ) { +- cols.append( p.first ); +- vals.append( bindValue( p.second ) ); ++ *statement += QLatin1String( "INSERT INTO " ); ++ *statement += mTable; ++ *statement += QLatin1String( " (" ); ++ for (int i = 0, c = mColumnValues.size(); i < c; ++i) { ++ *statement += mColumnValues.at(i).first; ++ if (i + 1 < c) { ++ *statement += QLatin1String( ", " ); ++ } ++ } ++ *statement += QLatin1String( ") VALUES (" ); ++ for (int i = 0, c = mColumnValues.size(); i < c; ++i) { ++ bindValue( statement, mColumnValues.at(i).second ); ++ if (i + 1 < c) { ++ *statement += QLatin1String( ", " ); ++ } + } +- statement += cols.join( QLatin1String( ", " ) ); +- statement += QLatin1String( ") VALUES (" ); +- statement += vals.join( QLatin1String( ", " ) ); +- statement += QLatin1Char( ')' ); ++ *statement += QLatin1Char( ')' ); + if ( mDatabaseType == DbType::PostgreSQL && !mIdentificationColumn.isEmpty() ) { +- statement += QLatin1String( " RETURNING " ) + mIdentificationColumn; ++ *statement += QLatin1String( " RETURNING " ) + mIdentificationColumn; + } + break; + } +@@ -246,78 +263,75 @@ QString QueryBuilder::buildQuery() + sqliteAdaptUpdateJoin( whereCondition ); + } + +- statement += QLatin1String( "UPDATE " ); +- statement += mTable; ++ *statement += QLatin1String( "UPDATE " ); ++ *statement += mTable; + + if ( mDatabaseType == DbType::MySQL && !mJoinedTables.isEmpty() ) { + // for mysql we list all tables directly +- statement += QLatin1String( ", " ); +- statement += mJoinedTables.join( QLatin1String( ", " ) ); ++ *statement += QLatin1String( ", " ); ++ appendJoined( statement, mJoinedTables ); + } + +- statement += QLatin1String( " SET " ); ++ *statement += QLatin1String( " SET " ); + Q_ASSERT_X( mColumnValues.count() >= 1, "QueryBuilder::exec()", "At least one column needs to be changed" ); +- typedef QPair StringVariantPair; +- QStringList updStmts; +- Q_FOREACH ( const StringVariantPair &p, mColumnValues ) { +- QString updStmt = p.first; +- updStmt += QLatin1String( " = " ); +- updStmt += bindValue( p.second ); +- updStmts << updStmt; ++ for (int i = 0, c = mColumnValues.size(); i < c; ++i) { ++ const QPair& p = mColumnValues.at( i ); ++ *statement += p.first; ++ *statement += QLatin1String( " = " ); ++ bindValue( statement, p.second ); ++ if (i + 1 < c) { ++ *statement += QLatin1String( ", " ); ++ } + } +- statement += updStmts.join( QLatin1String( ", " ) ); + + if ( mDatabaseType == DbType::PostgreSQL && !mJoinedTables.isEmpty() ) { + // PSQL have this syntax + // FROM t1 JOIN t2 JOIN ... +- statement += QLatin1String( " FROM " ); +- statement += mJoinedTables.join( QLatin1String( " JOIN " ) ); ++ *statement += QLatin1String( " FROM " ); ++ appendJoined( statement, mJoinedTables, QLatin1String( " JOIN " ) ); + } + + break; + } + case Delete: +- statement += QLatin1String( "DELETE FROM " ); +- statement += mTable; ++ *statement += QLatin1String( "DELETE FROM " ); ++ *statement += mTable; + break; + default: + Q_ASSERT_X( false, "QueryBuilder::exec()", "Unknown enum value" ); + } + + if ( !whereCondition.isEmpty() ) { +- statement += QLatin1String( " WHERE " ); +- statement += buildWhereCondition( whereCondition ); ++ *statement += QLatin1String( " WHERE " ); ++ buildWhereCondition( statement, whereCondition ); + } + + if ( !mGroupColumns.isEmpty() ) { +- statement += QLatin1String( " GROUP BY " ); +- statement += mGroupColumns.join( QLatin1String( ", " ) ); ++ *statement += QLatin1String( " GROUP BY " ); ++ appendJoined( statement, mGroupColumns ); + } + + if ( !mRootCondition[HavingCondition].isEmpty() ) { +- statement += QLatin1String( " HAVING " ); +- statement += buildWhereCondition( mRootCondition[HavingCondition] ); ++ *statement += QLatin1String( " HAVING " ); ++ buildWhereCondition( statement, mRootCondition[HavingCondition] ); + } + + if ( !mSortColumns.isEmpty() ) { + Q_ASSERT_X( mType == Select, "QueryBuilder::exec()", "Order statements are only valid for SELECT queries" ); +- QStringList orderStmts; +- typedef QPair SortColumnInfo; +- Q_FOREACH ( const SortColumnInfo &order, mSortColumns ) { +- QString orderStmt; +- orderStmt += order.first; +- orderStmt += sortOrderToString( order.second ); +- orderStmts << orderStmt; ++ *statement += QLatin1String( " ORDER BY " ); ++ for (int i = 0, c = mSortColumns.size(); i < c; ++i) { ++ const QPair& order = mSortColumns.at( i ); ++ *statement += order.first; ++ *statement += sortOrderToString( order.second ); ++ if (i + 1 < c) { ++ *statement += QLatin1String( ", " ); ++ } + } +- statement += QLatin1String( " ORDER BY " ); +- statement += orderStmts.join( QLatin1String( ", " ) ); + } + + if ( mLimit > 0 ) { +- statement += QLatin1Literal( " LIMIT " ) + QString::number( mLimit ); ++ *statement += QLatin1Literal( " LIMIT " ) + QString::number( mLimit ); + } +- +- return statement; + } + + bool QueryBuilder::retryLastTransaction( bool rollback ) +@@ -334,7 +348,9 @@ bool QueryBuilder::retryLastTransaction( bool rollback ) + + bool QueryBuilder::exec() + { +- const QString statement = buildQuery(); ++ QString statement; ++ statement.reserve(1024); ++ buildQuery(&statement); + + #ifndef QUERYBUILDER_UNITTEST + if ( QueryCache::contains( statement ) ) { +@@ -443,52 +459,54 @@ void QueryBuilder::addColumn( const QString &col ) + + void QueryBuilder::addAggregation( const QString &col, const QString &aggregate ) + { +- QString s( aggregate ); +- s += QLatin1Char( '(' ); +- s += col; +- s += QLatin1Char( ')' ); +- mColumns.append( s ); ++ mColumns.append( aggregate + QLatin1Char( '(' ) + col + QLatin1Char( ')' ) ); + } + +-QString QueryBuilder::bindValue( const QVariant &value ) ++void QueryBuilder::bindValue( QString *query, const QVariant &value ) + { + mBindValues << value; +- return QLatin1Char( ':' ) + QString::number( mBindValues.count() - 1 ); ++ *query += QLatin1Char( ':' ) + QString::number( mBindValues.count() - 1 ); + } + +-QString QueryBuilder::buildWhereCondition( const Query::Condition &cond ) ++void QueryBuilder::buildWhereCondition( QString *query, const Query::Condition &cond ) + { + if ( !cond.isEmpty() ) { +- QStringList conds; +- Q_FOREACH ( const Query::Condition &c, cond.subConditions() ) { +- conds << buildWhereCondition( c ); ++ *query += QLatin1String( "( " ); ++ const QLatin1String glue = logicOperatorToString( cond.mCombineOp ); ++ const Query::Condition::List& subConditions = cond.subConditions(); ++ for (int i = 0, c = subConditions.size(); i < c; ++i) { ++ buildWhereCondition(query, subConditions.at(i)); ++ if (i + 1 < c) { ++ *query += glue; ++ } + } +- return QLatin1String( "( " ) + conds.join( logicOperatorToString( cond.mCombineOp ) ) + QLatin1String( " )" ); ++ *query += QLatin1String( " )" ); + } else { +- QString stmt = cond.mColumn; +- stmt += compareOperatorToString( cond.mCompareOp ); ++ *query += cond.mColumn; ++ *query += compareOperatorToString( cond.mCompareOp ); + if ( cond.mComparedColumn.isEmpty() ) { + if ( cond.mComparedValue.isValid() ) { + if ( cond.mComparedValue.canConvert( QVariant::List ) ) { +- stmt += QLatin1String( "( " ); +- QStringList entries; +- Q_ASSERT_X( !cond.mComparedValue.toList().isEmpty(), ++ *query += QLatin1String( "( " ); ++ const QVariantList& entries = cond.mComparedValue.toList(); ++ Q_ASSERT_X( !entries.isEmpty(), + "QueryBuilder::buildWhereCondition()", "No values given for IN condition." ); +- Q_FOREACH ( const QVariant &entry, cond.mComparedValue.toList() ) { +- entries << bindValue( entry ); ++ for (int i = 0, c = entries.size(); i < c; ++i) { ++ bindValue( query, entries.at(i) ); ++ if (i + 1 < c) { ++ *query += QLatin1String( ", " ); ++ } + } +- stmt += entries.join( QLatin1String( ", " ) ); +- stmt += QLatin1String( " )" ); ++ *query += QLatin1String( " )" ); + } else { +- stmt += bindValue( cond.mComparedValue ); ++ bindValue( query, cond.mComparedValue ); + } + } else { +- stmt += QLatin1String( "NULL" ); ++ *query += QLatin1String( "NULL" ); + } + } else { +- stmt += cond.mComparedColumn; ++ *query += cond.mComparedColumn; + } +- return stmt; + } + } + +diff --git a/server/src/storage/querybuilder.h b/server/src/storage/querybuilder.h +index b380f93..df7c362 100644 +--- a/server/src/storage/querybuilder.h ++++ b/server/src/storage/querybuilder.h +@@ -70,7 +70,9 @@ class QueryBuilder + WhereCondition, + /// add condition to HAVING part of the query + /// NOTE: only supported for SELECT queries +- HavingCondition ++ HavingCondition, ++ ++ NUM_CONDITIONS + }; + + /** +@@ -234,9 +236,9 @@ class QueryBuilder + qint64 insertId(); + + private: +- QString buildQuery(); +- QString bindValue( const QVariant &value ); +- QString buildWhereCondition( const Query::Condition &cond ); ++ void buildQuery( QString *query ); ++ void bindValue( QString *query, const QVariant &value ); ++ void buildWhereCondition( QString *query, const Query::Condition &cond ); + + /** + * SQLite does not support JOINs with UPDATE, so we have to convert it into +@@ -249,11 +251,11 @@ class QueryBuilder + private: + QString mTable; + DbType::Type mDatabaseType; +- QHash mRootCondition; ++ Query::Condition mRootCondition[NUM_CONDITIONS]; + QSqlQuery mQuery; + QueryType mType; + QStringList mColumns; +- QList mBindValues; ++ QVector mBindValues; + QVector > mSortColumns; + QStringList mGroupColumns; + QVector > mColumnValues; +diff --git a/server/tests/unittest/querybuildertest.cpp b/server/tests/unittest/querybuildertest.cpp +index 0aba8a1..92df2a2 100644 +--- a/server/tests/unittest/querybuildertest.cpp ++++ b/server/tests/unittest/querybuildertest.cpp +@@ -29,26 +29,29 @@ + + QTEST_MAIN( QueryBuilderTest ) + ++Q_DECLARE_METATYPE(QVector) ++ + using namespace Akonadi::Server; + + void QueryBuilderTest::testQueryBuilder_data() + { ++ qRegisterMetaType >(); + mBuilders.clear(); + QTest::addColumn( "qbId" ); + QTest::addColumn( "sql" ); +- QTest::addColumn >( "bindValues" ); ++ QTest::addColumn >( "bindValues" ); + + QueryBuilder qb( "table", QueryBuilder::Select ); + qb.addColumn( "col1" ); + mBuilders << qb; +- QTest::newRow( "simple select" ) << mBuilders.count() << QString( "SELECT col1 FROM table" ) << QList(); ++ QTest::newRow( "simple select" ) << mBuilders.count() << QString( "SELECT col1 FROM table" ) << QVector(); + + qb.addColumn( "col2" ); + mBuilders << qb; +- QTest::newRow( "simple select 2" ) << mBuilders.count() << QString( "SELECT col1, col2 FROM table" ) << QList(); ++ QTest::newRow( "simple select 2" ) << mBuilders.count() << QString( "SELECT col1, col2 FROM table" ) << QVector(); + + qb.addValueCondition( "col1", Query::Equals, QVariant( 5 ) ); +- QList bindVals; ++ QVector bindVals; + bindVals << QVariant( 5 ); + mBuilders << qb; + QTest::newRow( "single where" ) << mBuilders.count() << QString( "SELECT col1, col2 FROM table WHERE ( col1 = :0 )" ) << bindVals; +@@ -71,17 +74,17 @@ void QueryBuilderTest::testQueryBuilder_data() + qb = QueryBuilder( "table" ); + qb.addAggregation( "col1", "count" ); + mBuilders << qb; +- QTest::newRow( "single aggregation" ) << mBuilders.count() << QString( "SELECT count(col1) FROM table" ) << QList(); ++ QTest::newRow( "single aggregation" ) << mBuilders.count() << QString( "SELECT count(col1) FROM table" ) << QVector(); + + qb = QueryBuilder( "table" ); + qb.addColumn( "col1" ); + qb.addSortColumn( "col1" ); + mBuilders << qb; +- QTest::newRow( "single order by" ) << mBuilders.count() << QString( "SELECT col1 FROM table ORDER BY col1 ASC" ) << QList(); ++ QTest::newRow( "single order by" ) << mBuilders.count() << QString( "SELECT col1 FROM table ORDER BY col1 ASC" ) << QVector(); + + qb.addSortColumn( "col2", Query::Descending ); + mBuilders << qb; +- QTest::newRow( "multiple order by" ) << mBuilders.count() << QString( "SELECT col1 FROM table ORDER BY col1 ASC, col2 DESC" ) << QList(); ++ QTest::newRow( "multiple order by" ) << mBuilders.count() << QString( "SELECT col1 FROM table ORDER BY col1 ASC, col2 DESC" ) << QVector(); + + qb = QueryBuilder( "table" ); + qb.addColumn( "col1" ); +@@ -98,7 +101,7 @@ void QueryBuilderTest::testQueryBuilder_data() + qb.addColumn( "col1" ); + qb.setLimit( 1 ); + mBuilders << qb; +- QTest::newRow( "SELECT with LIMIT" ) << mBuilders.count() << QString( "SELECT col1 FROM table LIMIT 1" ) << QList(); ++ QTest::newRow( "SELECT with LIMIT" ) << mBuilders.count() << QString( "SELECT col1 FROM table LIMIT 1" ) << QVector(); + + qb = QueryBuilder( "table", QueryBuilder::Update ); + qb.setColumnValue( "col1", QString( "bla" ) ); +@@ -263,7 +266,7 @@ void QueryBuilderTest::testQueryBuilder() + { + QFETCH( int, qbId ); + QFETCH( QString, sql ); +- QFETCH( QList, bindValues ); ++ QFETCH( QVector, bindValues ); + + --qbId; + +@@ -271,3 +274,40 @@ void QueryBuilderTest::testQueryBuilder() + QCOMPARE( mBuilders[qbId].mStatement, sql ); + QCOMPARE( mBuilders[qbId].mBindValues, bindValues ); + } ++ ++void QueryBuilderTest::benchQueryBuilder() ++{ ++ const QString table1 = QLatin1String("Table1"); ++ const QString table2 = QLatin1String("Table2"); ++ const QString table3 = QLatin1String("Table3"); ++ const QString table1_id = QLatin1String("Table1.id"); ++ const QString table2_id = QLatin1String("Table2.id"); ++ const QString table3_id = QLatin1String("Table3.id"); ++ const QString aggregate = QLatin1String("COUNT"); ++ const QVariant value = QVariant::fromValue(QString("asdf")); ++ ++ const QStringList columns = QStringList() ++ << QLatin1String("Table1.id") ++ << QLatin1String("Table1.fooAsdf") ++ << QLatin1String("Table2.barLala") ++ << QLatin1String("Table3.xyzFsd"); ++ ++ bool executed = true; ++ ++ QBENCHMARK { ++ QueryBuilder builder( table1, QueryBuilder::Select ); ++ builder.setDatabaseType( DbType::MySQL ); ++ builder.addColumns( columns ); ++ builder.addJoin( QueryBuilder::InnerJoin, table2, table2_id, table1_id ); ++ builder.addJoin( QueryBuilder::LeftJoin, table3, table1_id, table3_id ); ++ builder.addAggregation( columns.first(), aggregate ); ++ builder.addColumnCondition( columns.at(1), Query::LessOrEqual, columns.last() ); ++ builder.addValueCondition( columns.at(3), Query::Equals, value ); ++ builder.addSortColumn( columns.at(2) ); ++ builder.setLimit( 10 ); ++ builder.addGroupColumn( columns.at(3) ); ++ executed = executed && builder.exec(); ++ } ++ ++ QVERIFY(executed); ++} +\ No newline at end of file +diff --git a/server/tests/unittest/querybuildertest.h b/server/tests/unittest/querybuildertest.h +index 3bb6b22..1bca2cc 100644 +--- a/server/tests/unittest/querybuildertest.h ++++ b/server/tests/unittest/querybuildertest.h +@@ -37,6 +37,8 @@ class QueryBuilderTest : public QObject + void testQueryBuilder_data(); + void testQueryBuilder(); + ++ void benchQueryBuilder(); ++ + private: + QList< Akonadi::Server::QueryBuilder > mBuilders; + }; +-- +2.1.0 + diff --git a/0013-Intern-entity-strings-for-table-and-column-names.patch b/0013-Intern-entity-strings-for-table-and-column-names.patch new file mode 100644 index 0000000..1b40f2a --- /dev/null +++ b/0013-Intern-entity-strings-for-table-and-column-names.patch @@ -0,0 +1,176 @@ +From a04809a44c235bed854adc3bd49ca75b9673bf1f Mon Sep 17 00:00:00 2001 +From: Milian Wolff +Date: Wed, 26 Nov 2014 13:20:05 +0100 +Subject: [PATCH 13/30] Intern entity strings for table and column names. + +This should drastically cut down on the amount of allocations done +by the AkonadiServer. Currently, the getters will do the conversion +from QLatin1String to QString on every call. By reusing the data +via a function-local static const QString object, we can eliminate +all of these allocations and increase the cache locality as well. + +REVIEW: 121255 +--- + server/src/storage/entities-source.xsl | 56 +++++++++++++++++++++------------- + server/src/storage/entities.xsl | 4 +-- + 2 files changed, 36 insertions(+), 24 deletions(-) + +diff --git a/server/src/storage/entities-source.xsl b/server/src/storage/entities-source.xsl +index 174cf4f..7090c31 100644 +--- a/server/src/storage/entities-source.xsl ++++ b/server/src/storage/entities-source.xsl +@@ -214,36 +214,41 @@ void ::::tableName() + { +- return QLatin1String( "" ); ++ static const QString tableName = QLatin1String( "" ); ++ return tableName; + } + + QStringList ::columnNames() + { +- QStringList rv; ++ static const QStringList columns = QStringList() + +- rv.append( QLatin1String( "" ) ); ++ << Column() + +- return rv; ++ ; ++ return columns; + } + + QStringList ::fullColumnNames() + { +- QStringList rv; ++ static const QStringList columns = QStringList() + +- rv.append( QLatin1String( "." ) ); ++ << FullColumnName() + +- return rv; ++ ; ++ return columns; + } + + + QString ::Column() + { +- return QLatin1String( "" ); ++ static const QString column = QLatin1String( "" ); ++ return column; + } + + QString ::FullColumnName() + { +- return tableName() + QLatin1String( "." ); ++ static const QString column = QLatin1String( "." ); ++ return column; + } + + +@@ -399,7 +404,6 @@ QVector<> Relation + + +-Table + + // data retrieval for n:m relations + QVector<> ::s() const +@@ -408,14 +412,17 @@ QVector<> >(); + +- QueryBuilder qb( QLatin1String(""), QueryBuilder::Select ); ++ QueryBuilder qb( ::tableName(), QueryBuilder::Select ); ++ static const QStringList columns = QStringList() + +- qb.addColumn( QLatin1String("." ) ); ++ << ::FullColumnName() + +- qb.addJoin( QueryBuilder::InnerJoin, QLatin1String(""), +- QLatin1String("._"), +- QLatin1String(".") ); +- qb.addValueCondition( QLatin1String("._"), Query::Equals, id() ); ++ ; ++ qb.addColumns(columns); ++ qb.addJoin( QueryBuilder::InnerJoin, ::tableName(), ++ ::rightFullColumnName(), ++ ::FullColumnName() ); ++ qb.addValueCondition( ::leftFullColumnName(), Query::Equals, id() ); + + if ( !qb.exec() ) { + akDebug() << "Error during selection of records from table Relation" +@@ -546,7 +553,7 @@ bool ::update() + + + +- qb.addValueCondition( QLatin1String("id"), Query::Equals, id() ); ++ qb.addValueCondition( idColumn(), Query::Equals, id() ); + + + if ( !qb.exec() ) { +@@ -622,27 +629,32 @@ void ::enableCache( bool enable ) + // SQL table information + QString ::tableName() + { +- return QLatin1String( "" ); ++ static const QString table = QLatin1String( "" ); ++ return table; + } + + QString ::leftColumn() + { +- return QLatin1String( "_" ); ++ static const QString column = QLatin1String( "_" ); ++ return column; + } + + QString ::leftFullColumnName() + { +- return tableName() + QLatin1String( "." ) + leftColumn(); ++ static const QString column = QLatin1String( "._" ); ++ return column; + } + + QString ::rightColumn() + { +- return QLatin1String( "_" ); ++ static const QString column = QLatin1String( "_" ); ++ return column; + } + + QString ::rightFullColumnName() + { +- return tableName() + QLatin1String( "." ) + rightColumn(); ++ static const QString column = QLatin1String( "._" ); ++ return column; + } + + +diff --git a/server/src/storage/entities.xsl b/server/src/storage/entities.xsl +index 033e292..8b0ed03 100644 +--- a/server/src/storage/entities.xsl ++++ b/server/src/storage/entities.xsl +@@ -114,7 +114,7 @@ using namespace Akonadi::Server; + + QVector<QString> Akonadi::Server::allDatabaseTables() + { +- static QVector<QString> allTables = QVector<QString>() ++ static const QVector<QString> allTables = QVector<QString>() + + << QLatin1String( "Table" ) + +@@ -182,7 +182,7 @@ set( ) + + QueryBuilder qb( tableName(), QueryBuilder::Select ); + qb.addColumns( columnNames() ); +- qb.addValueCondition( QLatin1String(""), Query::Equals, ); ++ qb.addValueCondition( Column(), Query::Equals, ); + if ( !qb.exec() ) { + akDebug() << "Error during selection of record with " + << << "from table" << tableName() +-- +2.1.0 + diff --git a/0014-No-semicolon-after-Q_DECLARE_METATYPE.patch b/0014-No-semicolon-after-Q_DECLARE_METATYPE.patch new file mode 100644 index 0000000..684ece9 --- /dev/null +++ b/0014-No-semicolon-after-Q_DECLARE_METATYPE.patch @@ -0,0 +1,38 @@ +From f49b99f5a49da1a78b0ced930b6438bb53b49fdd Mon Sep 17 00:00:00 2001 +From: Volker Krause +Date: Sun, 30 Nov 2014 11:25:56 +0100 +Subject: [PATCH 14/30] No semicolon after Q_DECLARE_METATYPE. + +--- + server/src/storage/entity.h | 2 +- + server/tests/unittest/akappendhandlertest.cpp | 2 +- + 2 files changed, 2 insertions(+), 2 deletions(-) + +diff --git a/server/src/storage/entity.h b/server/src/storage/entity.h +index eb180e4..acebb0b 100644 +--- a/server/src/storage/entity.h ++++ b/server/src/storage/entity.h +@@ -192,6 +192,6 @@ namespace _detail { + } // namespace Server + } // namespace Akonadi + +-Q_DECLARE_METATYPE(Akonadi::Server::Tristate); ++Q_DECLARE_METATYPE(Akonadi::Server::Tristate) + + #endif +diff --git a/server/tests/unittest/akappendhandlertest.cpp b/server/tests/unittest/akappendhandlertest.cpp +index c221a3a..d7f57f9 100644 +--- a/server/tests/unittest/akappendhandlertest.cpp ++++ b/server/tests/unittest/akappendhandlertest.cpp +@@ -41,7 +41,7 @@ + using namespace Akonadi; + using namespace Akonadi::Server; + +-Q_DECLARE_METATYPE(PimItem); ++Q_DECLARE_METATYPE(PimItem) + Q_DECLARE_METATYPE(QVector) + Q_DECLARE_METATYPE(QVector) + Q_DECLARE_METATYPE(QVector) +-- +2.1.0 + diff --git a/0015-Use-QMutexLocker-instead-of-manual-lock-unlock-calls.patch b/0015-Use-QMutexLocker-instead-of-manual-lock-unlock-calls.patch new file mode 100644 index 0000000..8b4f79c --- /dev/null +++ b/0015-Use-QMutexLocker-instead-of-manual-lock-unlock-calls.patch @@ -0,0 +1,112 @@ +From f5a0e3f1f4787b6a48880e42463ae38dce336a8f Mon Sep 17 00:00:00 2001 +From: Milian Wolff +Date: Mon, 1 Dec 2014 11:36:31 +0100 +Subject: [PATCH 15/30] Use QMutexLocker instead of manual lock/unlock calls. + +Just a minor cleanup patch, no change of behavior. +--- + server/src/storage/entities-source.xsl | 17 +++++------------ + server/src/storage/entities.xsl | 4 +--- + 2 files changed, 6 insertions(+), 15 deletions(-) + +diff --git a/server/src/storage/entities-source.xsl b/server/src/storage/entities-source.xsl +index 7090c31..05a8cb1 100644 +--- a/server/src/storage/entities-source.xsl ++++ b/server/src/storage/entities-source.xsl +@@ -125,14 +125,13 @@ void ::Private::addToCache( const +- cacheMutex.lock(); ++ QMutexLocker lock(&cacheMutex); + + idCache.insert( entry.id(), entry ); + + + nameCache.insert( entry.name(), entry ); + +- cacheMutex.unlock(); + } + + +@@ -264,12 +263,10 @@ int ::count( const QString &column, const + bool ::exists( qint64 id ) + { + if ( Private::cacheEnabled ) { +- Private::cacheMutex.lock(); ++ QMutexLocker lock(&Private::cacheMutex); + if ( Private::idCache.contains( id ) ) { +- Private::cacheMutex.unlock(); + return true; + } +- Private::cacheMutex.unlock(); + } + return count( idColumn(), id ) > 0; + } +@@ -278,12 +275,10 @@ bool ::exists( qint64 id ) + bool ::exists( const &name ) + { + if ( Private::cacheEnabled ) { +- Private::cacheMutex.lock(); ++ QMutexLocker lock(&Private::cacheMutex); + if ( Private::nameCache.contains( name ) ) { +- Private::cacheMutex.unlock(); + return true; + } +- Private::cacheMutex.unlock(); + } + return count( nameColumn(), name ) > 0; + } +@@ -588,28 +583,26 @@ bool ::remove( qint64 id ) + void ::invalidateCache() const + { + if ( Private::cacheEnabled ) { +- Private::cacheMutex.lock(); ++ QMutexLocker lock(&Private::cacheMutex); + + Private::idCache.remove( id() ); + + + Private::nameCache.remove( name() ); + +- Private::cacheMutex.unlock(); + } + } + + void ::invalidateCompleteCache() + { + if ( Private::cacheEnabled ) { +- Private::cacheMutex.lock(); ++ QMutexLocker lock(&Private::cacheMutex); + + Private::idCache.clear(); + + + Private::nameCache.clear(); + +- Private::cacheMutex.unlock(); + } + } + +diff --git a/server/src/storage/entities.xsl b/server/src/storage/entities.xsl +index 8b0ed03..a397544 100644 +--- a/server/src/storage/entities.xsl ++++ b/server/src/storage/entities.xsl +@@ -167,13 +167,11 @@ set( ) + + + if ( Private::cacheEnabled ) { +- Private::cacheMutex.lock(); ++ QMutexLocker lock(&Private::cacheMutex); + if ( Private::.contains( ) ) { + const tmp = Private::.value( ); +- Private::cacheMutex.unlock(); + return tmp; + } +- Private::cacheMutex.unlock(); + } + + QSqlDatabase db = DataStore::self()->database(); +-- +2.1.0 + diff --git a/0016-Use-an-QAtomicInt-instead-of-a-plain-bool-for-Entity.patch b/0016-Use-an-QAtomicInt-instead-of-a-plain-bool-for-Entity.patch new file mode 100644 index 0000000..658e55f --- /dev/null +++ b/0016-Use-an-QAtomicInt-instead-of-a-plain-bool-for-Entity.patch @@ -0,0 +1,37 @@ +From 8a113985cda1693c8158916065bd54e57d028cda Mon Sep 17 00:00:00 2001 +From: Milian Wolff +Date: Mon, 1 Dec 2014 11:39:33 +0100 +Subject: [PATCH 16/30] Use an QAtomicInt instead of a plain bool for + Entity::cacheEnabled. + +A plain bool is not thread safe and leads to undefined behavior. +So better be safe than sorry and use a thread safe QAtomicInt. +--- + server/src/storage/entities-source.xsl | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/server/src/storage/entities-source.xsl b/server/src/storage/entities-source.xsl +index 05a8cb1..e398da5 100644 +--- a/server/src/storage/entities-source.xsl ++++ b/server/src/storage/entities-source.xsl +@@ -99,7 +99,7 @@ class ::Private : public QSharedData + static void addToCache( const & entry ); + + // cache +- static bool cacheEnabled; ++ static QAtomicInt cacheEnabled; + static QMutex cacheMutex; + + static QHash<qint64, > idCache; +@@ -111,7 +111,7 @@ class ::Private : public QSharedData + + + // static members +-bool ::Private::cacheEnabled = false; ++QAtomicInt ::Private::cacheEnabled(0); + QMutex ::Private::cacheMutex; + + QHash<qint64, > ::Private::idCache; +-- +2.1.0 + diff --git a/0017-Optimize-Only-do-one-hash-lookup-to-retrieve-value-f.patch b/0017-Optimize-Only-do-one-hash-lookup-to-retrieve-value-f.patch new file mode 100644 index 0000000..c2a0728 --- /dev/null +++ b/0017-Optimize-Only-do-one-hash-lookup-to-retrieve-value-f.patch @@ -0,0 +1,32 @@ +From 202ffa522668087cc133026febf21a7de8963218 Mon Sep 17 00:00:00 2001 +From: Milian Wolff +Date: Mon, 1 Dec 2014 11:51:04 +0100 +Subject: [PATCH 17/30] Optimize: Only do one hash lookup to retrieve value + from cache. + +Compilers do not merge the call to contains() and the successive +value() lookup. Using iterators thus saves us one QHash lookup. +--- + server/src/storage/entities.xsl | 6 +++--- + 1 file changed, 3 insertions(+), 3 deletions(-) + +diff --git a/server/src/storage/entities.xsl b/server/src/storage/entities.xsl +index a397544..9471293 100644 +--- a/server/src/storage/entities.xsl ++++ b/server/src/storage/entities.xsl +@@ -168,9 +168,9 @@ set( ) + + if ( Private::cacheEnabled ) { + QMutexLocker lock(&Private::cacheMutex); +- if ( Private::.contains( ) ) { +- const tmp = Private::.value( ); +- return tmp; ++ QHash<, >::const_iterator it = Private::.constFind(); ++ if ( it != Private::.constEnd() ) { ++ return it.value(); + } + } + +-- +2.1.0 + diff --git a/0018-Optimize-Skip-value-condition-on-invalid-flags.patch b/0018-Optimize-Skip-value-condition-on-invalid-flags.patch new file mode 100644 index 0000000..391d5c9 --- /dev/null +++ b/0018-Optimize-Skip-value-condition-on-invalid-flags.patch @@ -0,0 +1,38 @@ +From 7cbff48f5782d1f7f844678e6b785aeb419b0c47 Mon Sep 17 00:00:00 2001 +From: Milian Wolff +Date: Mon, 1 Dec 2014 11:59:12 +0100 +Subject: [PATCH 18/30] Optimize: Skip value condition on invalid flags. + +HandlerHelper::itemWithFlagsCount gets called quite often apparently +and I noticed that it was relatively slow from the Query Debugger +in Akonadi Console. EXPLAIN'ing the query showed that it was using +a slow-path for the WHERE FOO AND (BAR OR ASDF) condition. Here, +ASDF was always id = -1, the id of the $IGNORED flag, which +I apparently don't have. Getting rid of that condition simplifies +the query to WHERE FOO AND BAR, which is apparently much better +optimizable. Before, the query often showed a runtime of ~15ms. +Now it is down to ~9ms. + +REVIEW: 121306 +--- + server/src/handlerhelper.cpp | 4 ++++ + 1 file changed, 4 insertions(+) + +diff --git a/server/src/handlerhelper.cpp b/server/src/handlerhelper.cpp +index 634a26c..82347b4 100644 +--- a/server/src/handlerhelper.cpp ++++ b/server/src/handlerhelper.cpp +@@ -123,6 +123,10 @@ int HandlerHelper::itemWithFlagsCount( const Collection &col, const QStringList + // it hits an in-memory cache. + Q_FOREACH ( const QString &flag, flags ) { + const Flag f = Flag::retrieveByName( flag ); ++ if (!f.isValid()) { ++ // since we OR this condition, we can skip invalid flags to speed up the query ++ continue; ++ } + cond.addValueCondition( PimItemFlagRelation::rightFullColumnName(), Query::Equals, f.id() ); + } + qb.addCondition( cond ); +-- +2.1.0 + diff --git a/0019-Optimize-queries-Do-not-retrieve-known-key-used-in-t.patch b/0019-Optimize-queries-Do-not-retrieve-known-key-used-in-t.patch new file mode 100644 index 0000000..bb8c333 --- /dev/null +++ b/0019-Optimize-queries-Do-not-retrieve-known-key-used-in-t.patch @@ -0,0 +1,107 @@ +From e52b57b7a9f0303c0c710e60870d0ec265d32541 Mon Sep 17 00:00:00 2001 +From: Milian Wolff +Date: Mon, 1 Dec 2014 14:11:19 +0100 +Subject: [PATCH 19/30] Optimize queries: Do not retrieve known key used in the + condition. + +There is no point in doing a select like: + +SELECT foo, bar FROM table WHERE foo = needle; + +That can be rewritten to say + +SELECT bar FROM table WHERE foo = needle; + +This reduces the data traffic with the mysql server. Additionally, it +work-arounds some issues in Qt SQL, which lead to bad performance: +QSqlResult::value incurs multiple temporary allocations, and string +conversions, even to read a simple integer ID for example. Finally, +by reusing an externally provided QString name e.g., we can leverage +Qt's implicit sharing, instead of duplicating the string in a separate +QString instance, with the contents read from SQL server. + +REVIEW: 121310 +--- + server/src/storage/entities.xsl | 50 +++++++++++++++++++++++++++++------------ + 1 file changed, 36 insertions(+), 14 deletions(-) + +diff --git a/server/src/storage/entities.xsl b/server/src/storage/entities.xsl +index 9471293..c8fb1fd 100644 +--- a/server/src/storage/entities.xsl ++++ b/server/src/storage/entities.xsl +@@ -104,6 +104,12 @@ Q_DECLARE_TYPEINFO( Akonadi::Server::, Q_MOVABLE_T + + using namespace Akonadi::Server; + ++static QStringList removeEntry(QStringList list, const QString& entry) ++{ ++ list.removeOne(entry); ++ return list; ++} ++ + + + +@@ -179,7 +185,8 @@ set( ) + return (); + + QueryBuilder qb( tableName(), QueryBuilder::Select ); +- qb.addColumns( columnNames() ); ++ static const QStringList columns = removeEntry(columnNames(), Column()); ++ qb.addColumns( columns ); + qb.addValueCondition( Column(), Query::Equals, ); + if ( !qb.exec() ) { + akDebug() << "Error during selection of record with " +@@ -191,21 +198,36 @@ set( ) + return (); + } + ++ ++ int valueIndex = 0; ++ ++ const value = ++ ++ ++ ; ++ ++ ++ (qb.query().isNull(valueIndex)) ? ++ () : ++ ++ ++ Utils::variantToString( qb.query().value( valueIndex ) ) ++ ++ ++ static_cast<Tristate>(qb.query().value( valueIndex ).value<int>()) ++ ++ ++ qb.query().value( valueIndex ).value<>() ++ ++ ++ ; ++valueIndex; ++ ++ ++ ++ + rv( + +- (qb.query().isNull()) ? +- () : +- +- +- Utils::variantToString( qb.query().value( ) ) +- +- +- static_cast<Tristate>(qb.query().value( ).value<int>()) +- +- +- qb.query().value( ).value<>() +- +- ++ value + , + + ); +-- +2.1.0 + diff --git a/0020-Avoid-ridiculous-amount-of-SQL-queries-by-caching-Pa.patch b/0020-Avoid-ridiculous-amount-of-SQL-queries-by-caching-Pa.patch new file mode 100644 index 0000000..b433f23 --- /dev/null +++ b/0020-Avoid-ridiculous-amount-of-SQL-queries-by-caching-Pa.patch @@ -0,0 +1,262 @@ +From 215b188d891d5236fe94131d176d7ddc3ae02d5d Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Dan=20Vr=C3=A1til?= +Date: Fri, 5 Dec 2014 17:12:28 +0100 +Subject: [PATCH 20/30] Avoid ridiculous amount of SQL queries by caching + PartTypes + +PartTypes are identified by their FQ name, which is in form NAMESPACE:NAME, +where namespace and name are stored in individual columns. For this reason +the standard ::retrieveByName() and name cache generated from entities.xslt +does not work. This patch adds special handling for PartType table, so that +a special PartType::retrieveByFQName() method as well as PartType name cache +handling are generated during the XSL Transformation, allowing us to cache +all the PartTypes. + +This reduces the amount of SQL queries by at least two for each single AKAPPEND, +MERGE, STORE and FETCH command, providing a nice performance boost during +sync. +--- + server/src/handler/append.cpp | 4 ++-- + server/src/storage/datastore.cpp | 4 +++- + server/src/storage/entities-header.xsl | 7 ++++++- + server/src/storage/entities-source.xsl | 31 ++++++++++++++++++++++++++++++- + server/src/storage/entities.xsl | 7 ++++++- + server/src/storage/parttypehelper.cpp | 29 +---------------------------- + server/src/storage/parttypehelper.h | 13 ------------- + 7 files changed, 48 insertions(+), 47 deletions(-) + +diff --git a/server/src/handler/append.cpp b/server/src/handler/append.cpp +index c503216..b594e27 100644 +--- a/server/src/handler/append.cpp ++++ b/server/src/handler/append.cpp +@@ -134,7 +134,7 @@ bool Append::commit() + + // wrap data into a part + Part part; +- part.setPartType( PartTypeHelper::fromName( "PLD", "RFC822" ) ); ++ part.setPartType( PartType::retrieveByFQName( QLatin1String("PLD"), QLatin1String("RFC822") ) ); + part.setData( m_data ); + part.setPimItemId( item.id() ); + part.setDatasize( dataSize ); +@@ -148,7 +148,7 @@ bool Append::commit() + //akDebug() << "Append handler: doPreprocessing is" << doPreprocessing; + if ( doPreprocessing ) { + Part hiddenAttribute; +- hiddenAttribute.setPartType( PartTypeHelper::fromName( "ATR", "HIDDEN" ) ); ++ hiddenAttribute.setPartType( PartType::retrieveByFQName( QLatin1String("ATR"), QLatin1String("HIDDEN") ) ); + hiddenAttribute.setData( QByteArray() ); + hiddenAttribute.setPimItemId( item.id() ); + hiddenAttribute.setDatasize( 0 ); +diff --git a/server/src/storage/datastore.cpp b/server/src/storage/datastore.cpp +index ae78bab..304f0e8 100644 +--- a/server/src/storage/datastore.cpp ++++ b/server/src/storage/datastore.cpp +@@ -183,6 +183,7 @@ bool DataStore::init() + Flag::enableCache( true ); + Resource::enableCache( true ); + Collection::enableCache( true ); ++ PartType::enableCache( true ); + + return true; + } +@@ -1025,7 +1026,8 @@ bool DataStore::unhideAllPimItems() + akDebug() << "DataStore::unhideAllPimItems()"; + + try { +- return PartHelper::remove( Part::partTypeIdFullColumnName(), PartTypeHelper::fromName( "ATR", "HIDDEN" ).id() ); ++ return PartHelper::remove( Part::partTypeIdFullColumnName(), ++ PartType::retrieveByFQName( QLatin1String("ATR"), QLatin1String("HIDDEN") ).id() ); + } catch ( ... ) {} // we can live with this failing + + return false; +diff --git a/server/src/storage/entities-header.xsl b/server/src/storage/entities-header.xsl +index 4966966..d515fd3 100644 +--- a/server/src/storage/entities-header.xsl ++++ b/server/src/storage/entities-header.xsl +@@ -133,11 +133,16 @@ class : private Entity + static retrieveById( qint64 id ); + + +- ++ + /** Returns the record with name @p name. */ + static retrieveByName( const &name ); + + ++ ++ ++ static PartType retrieveByFQName( const QString &ns, const QString &name ); ++ ++ + /** Retrieve all records from this table. */ + static ::List retrieveAll(); + /** Retrieve all records with value @p value in column @p key. */ +diff --git a/server/src/storage/entities-source.xsl b/server/src/storage/entities-source.xsl +index e398da5..46ef3a6 100644 +--- a/server/src/storage/entities-source.xsl ++++ b/server/src/storage/entities-source.xsl +@@ -130,7 +130,15 @@ void ::Private::addToCache( const + ++ ++ ++ ++ nameCache.insert( entry.ns() + QLatin1Char(':') + entry.name(), entry ); ++ ++ + nameCache.insert( entry.name(), entry ); ++ ++ + + } + +@@ -323,7 +331,7 @@ QVector< > ++ + ::retrieveByName( const &name ) + { + +@@ -333,6 +341,19 @@ QVector< > ++PartType PartType::retrieveByFQName( const QString & ns, const QString & name ) ++{ ++ const QString fqname = ns + QLatin1Char(':') + name; ++ ++ ns ++ name ++ fqname ++ nameCache ++ ++} ++ ++ + QVector<> ::retrieveAll() + { + QSqlDatabase db = DataStore::self()->database(); +@@ -588,7 +609,15 @@ void ::invalidateCache() const + Private::idCache.remove( id() ); + + ++ ++ ++ ++ Private::nameCache.remove( ns() + QLatin1Char(':') + name() ); ++ ++ + Private::nameCache.remove( name() ); ++ ++ + + } + } +diff --git a/server/src/storage/entities.xsl b/server/src/storage/entities.xsl +index c8fb1fd..2cf96c4 100644 +--- a/server/src/storage/entities.xsl ++++ b/server/src/storage/entities.xsl +@@ -169,12 +169,14 @@ set( ) + + + ++ ++ + + + + if ( Private::cacheEnabled ) { + QMutexLocker lock(&Private::cacheMutex); +- QHash<, >::const_iterator it = Private::.constFind(); ++ QHash<, >::const_iterator it = Private::.constFind(); + if ( it != Private::.constEnd() ) { + return it.value(); + } +@@ -188,6 +190,9 @@ set( ) + static const QStringList columns = removeEntry(columnNames(), Column()); + qb.addColumns( columns ); + qb.addValueCondition( Column(), Query::Equals, ); ++ ++ qb.addValueCondition( Column(), Query::Equals, ); ++ + if ( !qb.exec() ) { + akDebug() << "Error during selection of record with " + << << "from table" << tableName() +diff --git a/server/src/storage/parttypehelper.cpp b/server/src/storage/parttypehelper.cpp +index b73dcd5..7654108 100644 +--- a/server/src/storage/parttypehelper.cpp ++++ b/server/src/storage/parttypehelper.cpp +@@ -37,7 +37,7 @@ QPair< QString, QString > PartTypeHelper::parseFqName(const QString& fqName) + PartType PartTypeHelper::fromFqName(const QString& fqName) + { + const QPair p = parseFqName( fqName ); +- return fromName( p.first, p.second ); ++ return PartType::retrieveByFQName(p.first, p.second); + } + + PartType PartTypeHelper::fromFqName(const QByteArray& fqName) +@@ -45,33 +45,6 @@ PartType PartTypeHelper::fromFqName(const QByteArray& fqName) + return fromFqName( QLatin1String(fqName) ); + } + +-PartType PartTypeHelper::fromName(const QString& ns, const QString& typeName) +-{ +- SelectQueryBuilder qb; +- qb.addValueCondition( PartType::nsColumn(), Query::Equals, ns ); +- qb.addValueCondition( PartType::nameColumn(), Query::Equals, typeName ); +- if ( !qb.exec() ) +- throw PartTypeException( "Unable to query part type table." ); +- const PartType::List result = qb.result(); +- if ( result.size() == 1 ) +- return result.first(); +- if ( result.size() > 1 ) +- throw PartTypeException( "Part type uniqueness constraint violation." ); +- +- // doesn't exist yet, so let's create a new one +- PartType type; +- type.setName( typeName ); +- type.setNs( ns ); +- if ( !type.insert() ) +- throw PartTypeException( "Creating a new part type failed." ); +- return type; +-} +- +-PartType PartTypeHelper::fromName(const char* ns, const char* typeName) +-{ +- return fromName( QLatin1String(ns), QLatin1String(typeName) ); +-} +- + Query::Condition PartTypeHelper::conditionFromFqName(const QString& fqName) + { + const QPair p = parseFqName( fqName ); +diff --git a/server/src/storage/parttypehelper.h b/server/src/storage/parttypehelper.h +index 38cb858..4c4f42f 100644 +--- a/server/src/storage/parttypehelper.h ++++ b/server/src/storage/parttypehelper.h +@@ -48,19 +48,6 @@ namespace PartTypeHelper + PartType fromFqName( const QByteArray &fqName ); + + /** +- * Retrieve (or create) PartType for the given namespace and type name. +- * @param ns Namespace +- * @param typeName Part type name. +- * @throws PartTypeException +- */ +- PartType fromName( const QString &ns, const QString &typeName ); +- +- /** +- * Convenience overload of the above. +- */ +- PartType fromName( const char *ns, const char *typeName ); +- +- /** + * Returns a query condition that matches the given part. + * @param fqName fully-qualified part type name + * @throws PartTypeException +-- +2.1.0 + diff --git a/0021-Implement-support-for-CASE.WHEN.THEN-SQL-statements-.patch b/0021-Implement-support-for-CASE.WHEN.THEN-SQL-statements-.patch new file mode 100644 index 0000000..d452356 --- /dev/null +++ b/0021-Implement-support-for-CASE.WHEN.THEN-SQL-statements-.patch @@ -0,0 +1,241 @@ +From 9698d589e4c2b489f406fe1a823d4bb42c322f71 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Dan=20Vr=C3=A1til?= +Date: Fri, 5 Dec 2014 18:21:18 +0100 +Subject: [PATCH 21/30] Implement support for CASE...WHEN...THEN SQL statements + SELECT columns + +CASE...WHEN...THEN is a useful construct especially for aggregation +queries. +--- + server/src/storage/query.cpp | 38 ++++++++++++++++++++++++++++++ + server/src/storage/query.h | 19 +++++++++++++++ + server/src/storage/querybuilder.cpp | 30 +++++++++++++++++++++++ + server/src/storage/querybuilder.h | 14 +++++++++++ + server/tests/unittest/querybuildertest.cpp | 38 +++++++++++++++++++++++++++++- + 5 files changed, 138 insertions(+), 1 deletion(-) + +diff --git a/server/src/storage/query.cpp b/server/src/storage/query.cpp +index 6fb6c6e..c938ade 100644 +--- a/server/src/storage/query.cpp ++++ b/server/src/storage/query.cpp +@@ -68,3 +68,41 @@ void Query::Condition::addCondition( const Condition &condition ) + { + mSubConditions << condition; + } ++ ++ ++Case::Case(const Condition &when, const QString &then, const QString &elseBranch) ++{ ++ addCondition(when, then); ++ setElse(elseBranch); ++} ++ ++Case::Case(const QString &column, CompareOperator op, const QVariant &value, const QString &when, const QString &elseBranch) ++{ ++ addValueCondition(column, op, value, when); ++ setElse(elseBranch); ++} ++ ++void Case::addCondition(const Condition &when, const QString &then) ++{ ++ mWhenThen.append(qMakePair(when, then)); ++} ++ ++void Case::addValueCondition(const QString &column, CompareOperator op, const QVariant &value, const QString &then) ++{ ++ Condition when; ++ when.addValueCondition(column, op, value); ++ addCondition(when, then); ++} ++ ++void Case::addColumnCondition(const QString &column, CompareOperator op, const QString &column2, const QString &then) ++{ ++ Condition when; ++ when.addColumnCondition(column, op, column2); ++ addCondition(when, then); ++} ++ ++void Case::setElse(const QString &elseBranch) ++{ ++ mElse = elseBranch; ++} ++ +diff --git a/server/src/storage/query.h b/server/src/storage/query.h +index f4f1ac0..c8f35a7 100644 +--- a/server/src/storage/query.h ++++ b/server/src/storage/query.h +@@ -130,6 +130,25 @@ class Condition + + }; // class Condition + ++ ++class Case ++{ ++ friend class Akonadi::Server::QueryBuilder; ++ public: ++ Case(const Condition &when, const QString &then, const QString &elseBranch = QString()); ++ Case(const QString &column, Query::CompareOperator op, const QVariant &value, const QString &when, const QString &elseBranch = QString()); ++ ++ void addCondition(const Condition &when, const QString &then); ++ void addValueCondition(const QString &column, Query::CompareOperator op, const QVariant &value, const QString &then); ++ void addColumnCondition(const QString &column, Query::CompareOperator op, const QString &column2, const QString &then); ++ ++ void setElse(const QString &elseBranch); ++ ++ private: ++ QVector > mWhenThen; ++ QString mElse; ++}; ++ + } // namespace Query + } // namespace Server + } // namespace Akonadi +diff --git a/server/src/storage/querybuilder.cpp b/server/src/storage/querybuilder.cpp +index 3017867..74ed2da 100644 +--- a/server/src/storage/querybuilder.cpp ++++ b/server/src/storage/querybuilder.cpp +@@ -457,11 +457,27 @@ void QueryBuilder::addColumn( const QString &col ) + mColumns << col; + } + ++void QueryBuilder::addColumn( const Query::Case &caseStmt ) ++{ ++ QString query; ++ buildCaseStatement(&query, caseStmt); ++ mColumns.append(query); ++} ++ + void QueryBuilder::addAggregation( const QString &col, const QString &aggregate ) + { + mColumns.append( aggregate + QLatin1Char( '(' ) + col + QLatin1Char( ')' ) ); + } + ++void QueryBuilder::addAggregation(const Query::Case &caseStmt, const QString &aggregate) ++{ ++ QString query(aggregate + QLatin1Char('(')); ++ buildCaseStatement(&query, caseStmt); ++ query += QLatin1Char(')'); ++ ++ mColumns.append(query); ++} ++ + void QueryBuilder::bindValue( QString *query, const QVariant &value ) + { + mBindValues << value; +@@ -510,6 +526,20 @@ void QueryBuilder::buildWhereCondition( QString *query, const Query::Condition & + } + } + ++void QueryBuilder::buildCaseStatement(QString *query, const Query::Case &caseStmt) ++{ ++ *query += QLatin1String("CASE "); ++ for (const auto whenThen : caseStmt.mWhenThen) { ++ *query += QLatin1String("WHEN "); ++ buildWhereCondition(query, whenThen.first); // When ++ *query += QLatin1String(" THEN ") + whenThen.second; // then ++ } ++ if (!caseStmt.mElse.isEmpty()) { ++ *query += QLatin1String(" ELSE ") + caseStmt.mElse; ++ } ++ *query += QLatin1String(" END"); ++} ++ + void QueryBuilder::setSubQueryMode( Query::LogicOperator op, ConditionType type ) + { + Q_ASSERT( type == WhereCondition || ( type == HavingCondition && mType == Select ) ); +diff --git a/server/src/storage/querybuilder.h b/server/src/storage/querybuilder.h +index df7c362..0304108 100644 +--- a/server/src/storage/querybuilder.h ++++ b/server/src/storage/querybuilder.h +@@ -123,6 +123,12 @@ class QueryBuilder + void addColumn( const QString &col ); + + /** ++ * Adds the given case statement to a select query. ++ * @param caseStmt The case statement to add. ++ */ ++ void addColumn( const Query::Case &caseStmt ); ++ ++ /** + * Adds an aggregation statement. + * @param col The column to aggregate on + * @param aggregate The aggregation function. +@@ -130,6 +136,13 @@ class QueryBuilder + void addAggregation( const QString &col, const QString &aggregate ); + + /** ++ * Adds and aggregation statement with CASE ++ * @param caseStmt The case statement to aggregate on ++ * @param aggregate The aggregation function. ++ */ ++ void addAggregation( const Query::Case &caseStmt, const QString &aggregate ); ++ ++ /** + Add a WHERE or HAVING condition which compares a column with a given value. + @param column The column that should be compared. + @param op The operator used for comparison +@@ -239,6 +252,7 @@ class QueryBuilder + void buildQuery( QString *query ); + void bindValue( QString *query, const QVariant &value ); + void buildWhereCondition( QString *query, const Query::Condition &cond ); ++ void buildCaseStatement( QString *query, const Query::Case &caseStmt ); + + /** + * SQLite does not support JOINs with UPDATE, so we have to convert it into +diff --git a/server/tests/unittest/querybuildertest.cpp b/server/tests/unittest/querybuildertest.cpp +index 92df2a2..848829d 100644 +--- a/server/tests/unittest/querybuildertest.cpp ++++ b/server/tests/unittest/querybuildertest.cpp +@@ -217,6 +217,42 @@ void QueryBuilderTest::testQueryBuilder_data() + } + + { ++ /// SELECT with CASE ++ QueryBuilder qbTpl = QueryBuilder("table1", QueryBuilder::Select ); ++ qbTpl.setDatabaseType( DbType::MySQL ); ++ ++ QueryBuilder qb = qbTpl; ++ qb.addColumn( "col" ); ++ qb.addColumn( Query::Case( "col1", Query::Greater, 42, "1", "0" ) ); ++ bindVals.clear(); ++ bindVals << 42; ++ mBuilders << qb; ++ QTest::newRow( "select case simple") << mBuilders.count() ++ << QString( "SELECT col, CASE WHEN ( col1 > :0 ) THEN 1 ELSE 0 END FROM table1" ) << bindVals; ++ ++ ++ qb = qbTpl; ++ qb.addAggregation( "table1.col1", "sum" ); ++ qb.addAggregation( "table1.col2", "count" ); ++ Query::Condition cond( Query::Or ); ++ cond.addValueCondition( "table3.col2", Query::Equals, "value1" ); ++ cond.addValueCondition( "table3.col2", Query::Equals, "value2" );\ ++ Query::Case caseStmt( cond, "1", "0" ); ++ qb.addAggregation( caseStmt, "sum" ); ++ qb.addJoin( QueryBuilder::LeftJoin, "table2", "table1.col3", "table2.col1" ); ++ qb.addJoin( QueryBuilder::LeftJoin, "table3", "table2.col2", "table3.col1" ); ++ bindVals.clear(); ++ bindVals << QString("value1") << QString("value2"); ++ mBuilders < +Date: Fri, 5 Dec 2014 18:23:33 +0100 +Subject: [PATCH 22/30] Implement cache for CollectionStatistics to + significantly reduce amount of SQL queries + +Collection statistics are being requested extremely often (basically whenever +a PimItem is changed, or when a Collection itself is changed), and it's always +requested by at least 5 or so clients (including agents that listen to +everything). + +To decrease the load on database we now cache the Collection statistics and +we only invalidate a cache entry when respective collection (or it's content) +is changed. The invalidation is invoked from NotificationCollector, which is +basically a hack, but performance-wise it's the best place to avoid additional +expensive queries. + +This patch also optimizes the SQL query needed to get up-to-date statistics. +We now have only one query to get both full count and read items count, which +a bit is faster as the database only has to deal with one large JOIN. + +Thanks to the cache the number of SQL queries for Collection statistics have +reduced by 70%-80%, and average query duration is now between 20 and 80ms +depending on average collection size and database used. +--- + server/CMakeLists.txt | 1 + + server/src/handler/link.cpp | 2 +- + server/src/handler/merge.cpp | 4 +- + server/src/handler/select.cpp | 14 ++-- + server/src/handler/status.cpp | 20 ++--- + server/src/handlerhelper.cpp | 81 ++------------------ + server/src/handlerhelper.h | 22 ------ + server/src/storage/collectionstatistics.cpp | 108 +++++++++++++++++++++++++++ + server/src/storage/collectionstatistics.h | 70 +++++++++++++++++ + server/src/storage/datastore.cpp | 8 +- + server/src/storage/datastore.h | 6 +- + server/src/storage/notificationcollector.cpp | 8 ++ + server/tests/unittest/fakedatastore.cpp | 8 +- + server/tests/unittest/fakedatastore.h | 2 + + 14 files changed, 224 insertions(+), 130 deletions(-) + create mode 100644 server/src/storage/collectionstatistics.cpp + create mode 100644 server/src/storage/collectionstatistics.h + +diff --git a/server/CMakeLists.txt b/server/CMakeLists.txt +index 275938d..f0e0093 100644 +--- a/server/CMakeLists.txt ++++ b/server/CMakeLists.txt +@@ -161,6 +161,7 @@ set(libakonadiprivate_SRCS + src/search/searchmanager.cpp + + src/storage/collectionqueryhelper.cpp ++ src/storage/collectionstatistics.cpp + src/storage/entity.cpp + ${CMAKE_CURRENT_BINARY_DIR}/entities.cpp + ${CMAKE_CURRENT_BINARY_DIR}/akonadischema.cpp +diff --git a/server/src/handler/link.cpp b/server/src/handler/link.cpp +index ce18e47..227de11 100644 +--- a/server/src/handler/link.cpp ++++ b/server/src/handler/link.cpp +@@ -25,10 +25,10 @@ + #include "storage/itemqueryhelper.h" + #include "storage/transaction.h" + #include "storage/selectquerybuilder.h" ++#include "storage/collectionqueryhelper.h" + #include "entities.h" + + #include "imapstreamparser.h" +-#include + + using namespace Akonadi::Server; + +diff --git a/server/src/handler/merge.cpp b/server/src/handler/merge.cpp +index c26917d..5149916 100644 +--- a/server/src/handler/merge.cpp ++++ b/server/src/handler/merge.cpp +@@ -88,7 +88,7 @@ bool Merge::mergeItem( PimItem &newItem, PimItem ¤tItem, + if ( !itemFlags.removed.isEmpty() ) { + const Flag::List removedFlags = HandlerHelper::resolveFlags( itemFlags.removed ); + DataStore::self()->removeItemsFlags( PimItem::List() << currentItem, removedFlags, +- &flagsRemoved, true ); ++ &flagsRemoved, col, true ); + } + + if ( flagsAdded || flagsRemoved ) { +@@ -98,7 +98,7 @@ bool Merge::mergeItem( PimItem &newItem, PimItem ¤tItem, + bool flagsChanged = false; + const Flag::List flags = HandlerHelper::resolveFlags( itemFlags.added ); + DataStore::self()->setItemsFlags( PimItem::List() << currentItem, flags, +- &flagsChanged, true ); ++ &flagsChanged, col, true ); + if ( flagsChanged ) { + mChangedParts << AKONADI_PARAM_FLAGS; + } +diff --git a/server/src/handler/select.cpp b/server/src/handler/select.cpp +index 1c5dd8a..f1ecc44 100644 +--- a/server/src/handler/select.cpp ++++ b/server/src/handler/select.cpp +@@ -27,6 +27,7 @@ + #include "handlerhelper.h" + #include "imapstreamparser.h" + #include "storage/selectquerybuilder.h" ++#include "storage/collectionstatistics.h" + #include "commandcontext.h" + + #include "response.h" +@@ -96,19 +97,14 @@ bool Select::parseStream() + response.setString( "FLAGS (" + Flag::joinByName( Flag::retrieveAll(), QLatin1String( " " ) ).toLatin1() + ")" ); + Q_EMIT responseAvailable( response ); + +- const int itemCount = HandlerHelper::itemCount( col ); +- if ( itemCount < 0 ) { ++ const CollectionStatistics::Statistics stats = CollectionStatistics::instance()->statistics(col); ++ if ( stats.count == -1 ) { + return failureResponse( "Unable to determine item count" ); + } +- response.setString( QByteArray::number( itemCount ) + " EXISTS" ); ++ response.setString( QByteArray::number( stats.count ) + " EXISTS" ); + Q_EMIT responseAvailable( response ); + +- int readCount = HandlerHelper::itemWithFlagsCount( col, QStringList() << QLatin1String( AKONADI_FLAG_SEEN ) +- << QLatin1String( AKONADI_FLAG_IGNORED ) ); +- if ( readCount < 0 || itemCount < readCount ) { +- return failureResponse( "Unable to retrieve unseen count" ); +- } +- response.setString( "OK [UNSEEN " + QByteArray::number( itemCount - readCount ) + "] Message 0 is first unseen" ); ++ response.setString( "OK [UNSEEN " + QByteArray::number( stats.count - stats.read ) + "] Message 0 is first unseen" ); + Q_EMIT responseAvailable( response ); + } + +diff --git a/server/src/handler/status.cpp b/server/src/handler/status.cpp +index 8c6823d..283532c 100644 +--- a/server/src/handler/status.cpp ++++ b/server/src/handler/status.cpp +@@ -25,6 +25,7 @@ + #include "storage/datastore.h" + #include "storage/entity.h" + #include "storage/countquerybuilder.h" ++#include "storage/collectionstatistics.h" + + #include "response.h" + #include "handlerhelper.h" +@@ -62,9 +63,9 @@ bool Status::parseStream() + // Responses: + // REQUIRED untagged responses: STATUS + +- qint64 itemCount, itemSize; +- if ( !HandlerHelper::itemStatistics( col, itemCount, itemSize ) ) { +- return failureResponse( "Failed to query statistics." ); ++ const CollectionStatistics::Statistics &stats = CollectionStatistics::instance()->statistics(col); ++ if (stats.count == -1) { ++ return failureResponse( "Failed to query statistics." ); + } + + // build STATUS response +@@ -72,7 +73,7 @@ bool Status::parseStream() + // MESSAGES - The number of messages in the mailbox + if ( attributeList.contains( AKONADI_ATTRIBUTE_MESSAGES ) ) { + statusResponse += AKONADI_ATTRIBUTE_MESSAGES " "; +- statusResponse += QByteArray::number( itemCount ); ++ statusResponse += QByteArray::number( stats.count ); + } + + if ( attributeList.contains( AKONADI_ATTRIBUTE_UNSEEN ) ) { +@@ -80,21 +81,14 @@ bool Status::parseStream() + statusResponse += " "; + } + statusResponse += AKONADI_ATTRIBUTE_UNSEEN " "; +- +- // itemWithFlagCount is twice as fast as itemWithoutFlagCount... +- const int count = HandlerHelper::itemWithFlagsCount( col, QStringList() << QLatin1String( AKONADI_FLAG_SEEN ) +- << QLatin1String( AKONADI_FLAG_IGNORED ) ); +- if ( count < 0 ) { +- return failureResponse( "Unable to retrieve unread count" ); +- } +- statusResponse += QByteArray::number( itemCount - count ); ++ statusResponse += QByteArray::number( stats.count - stats.read ); + } + if ( attributeList.contains( AKONADI_PARAM_SIZE ) ) { + if ( !statusResponse.isEmpty() ) { + statusResponse += " "; + } + statusResponse += AKONADI_PARAM_SIZE " "; +- statusResponse += QByteArray::number( itemSize ); ++ statusResponse += QByteArray::number( stats.size ); + } + + Response response; +diff --git a/server/src/handlerhelper.cpp b/server/src/handlerhelper.cpp +index 82347b4..39583ce 100644 +--- a/server/src/handlerhelper.cpp ++++ b/server/src/handlerhelper.cpp +@@ -22,6 +22,7 @@ + #include "storage/countquerybuilder.h" + #include "storage/datastore.h" + #include "storage/selectquerybuilder.h" ++#include "storage/collectionstatistics.h" + #include "storage/queryhelper.h" + #include "libs/imapparser_p.h" + #include "libs/protocol_p.h" +@@ -78,74 +79,6 @@ QString HandlerHelper::pathForCollection( const Collection &col ) + return parts.join( QLatin1String( "/" ) ); + } + +-bool HandlerHelper::itemStatistics( const Collection &col, qint64 &count, qint64 &size ) +-{ +- QueryBuilder qb( PimItem::tableName() ); +- qb.addAggregation( PimItem::idColumn(), QLatin1String( "count" ) ); +- qb.addAggregation( PimItem::sizeColumn(), QLatin1String( "sum" ) ); +- +- if ( col.isVirtual() ) { +- qb.addJoin( QueryBuilder::InnerJoin, CollectionPimItemRelation::tableName(), +- CollectionPimItemRelation::rightFullColumnName(), PimItem::idFullColumnName() ); +- qb.addValueCondition( CollectionPimItemRelation::leftFullColumnName(), Query::Equals, col.id() ); +- } else { +- qb.addValueCondition( PimItem::collectionIdColumn(), Query::Equals, col.id() ); +- } +- +- if ( !qb.exec() ) { +- return false; +- } +- if ( !qb.query().next() ) { +- akError() << "Error during retrieving result of statistics query:" << qb.query().lastError().text(); +- return false; +- } +- count = qb.query().value( 0 ).toLongLong(); +- size = qb.query().value( 1 ).toLongLong(); +- return true; +-} +- +-int HandlerHelper::itemWithFlagsCount( const Collection &col, const QStringList &flags ) +-{ +- CountQueryBuilder qb( PimItem::tableName(), PimItem::idFullColumnName(), CountQueryBuilder::Distinct ); +- qb.addJoin( QueryBuilder::InnerJoin, PimItemFlagRelation::tableName(), +- PimItem::idFullColumnName(), PimItemFlagRelation::leftFullColumnName() ); +- if ( col.isVirtual() ) { +- qb.addJoin( QueryBuilder::InnerJoin, CollectionPimItemRelation::tableName(), +- CollectionPimItemRelation::rightFullColumnName(), PimItem::idFullColumnName() ); +- qb.addValueCondition( CollectionPimItemRelation::leftFullColumnName(), Query::Equals, col.id() ); +- } else { +- qb.addValueCondition( PimItem::collectionIdFullColumnName(), Query::Equals, col.id() ); +- } +- Query::Condition cond( Query::Or ); +- // We use the below instead of an inner join in the query above because postgres seems +- // to struggle to optimize the two inner joins, despite having indices that should +- // facilitate that. This exploits the fact that the Flag::retrieveByName is fast because +- // it hits an in-memory cache. +- Q_FOREACH ( const QString &flag, flags ) { +- const Flag f = Flag::retrieveByName( flag ); +- if (!f.isValid()) { +- // since we OR this condition, we can skip invalid flags to speed up the query +- continue; +- } +- cond.addValueCondition( PimItemFlagRelation::rightFullColumnName(), Query::Equals, f.id() ); +- } +- qb.addCondition( cond ); +- if ( !qb.exec() ) { +- return -1; +- } +- return qb.result(); +-} +- +-int HandlerHelper::itemCount( const Collection &col ) +-{ +- CountQueryBuilder qb( PimItem::tableName() ); +- qb.addValueCondition( PimItem::collectionIdColumn(), Query::Equals, col.id() ); +- if ( !qb.exec() ) { +- return -1; +- } +- return qb.result(); +-} +- + int HandlerHelper::parseCachePolicy( const QByteArray &data, Collection &col, int start, bool *changed ) + { + bool inheritChanged = false; +@@ -233,14 +166,12 @@ QByteArray HandlerHelper::collectionToByteArray( const Collection &col, bool hid + b += " " AKONADI_PARAM_VIRTUAL " " + QByteArray::number( col.isVirtual() ) + ' '; + + if ( includeStatistics ) { +- qint64 itemCount, itemSize; +- if ( itemStatistics( col, itemCount, itemSize ) ) { +- b += AKONADI_ATTRIBUTE_MESSAGES " " + QByteArray::number( itemCount ) + ' '; +- // itemWithFlagCount is twice as fast as itemWithoutFlagCount, so emulated that... ++ const CollectionStatistics::Statistics &stats = CollectionStatistics::instance()->statistics(col); ++ if (stats.count > -1) { ++ b += AKONADI_ATTRIBUTE_MESSAGES " " + QByteArray::number( stats.count ) + ' '; + b += AKONADI_ATTRIBUTE_UNSEEN " "; +- b += QByteArray::number( itemCount - itemWithFlagsCount( col, QStringList() << QLatin1String( AKONADI_FLAG_SEEN ) +- << QLatin1String( AKONADI_FLAG_IGNORED ) ) ); +- b += " " AKONADI_PARAM_SIZE " " + QByteArray::number( itemSize ) + ' '; ++ b += QByteArray::number( stats.count - stats.read) ; ++ b += " " AKONADI_PARAM_SIZE " " + QByteArray::number( stats.size ) + ' '; + } + } + +diff --git a/server/src/handlerhelper.h b/server/src/handlerhelper.h +index 22e6e1c..cf9ac22 100644 +--- a/server/src/handlerhelper.h ++++ b/server/src/handlerhelper.h +@@ -52,28 +52,6 @@ class HandlerHelper + static QString pathForCollection( const Collection &col ); + + /** +- Returns the amount of existing items in the given collection. +- @return -1 on error +- */ +- static int itemCount( const Collection &col ); +- +- /** +- * Queries for collection statistics. +- * @param col The collection to query. +- * @param count The total amount of items in this collection. +- * @param size The size of all items in this collection. +- * @return @c false on a query error, @c true otherwise +- */ +- static bool itemStatistics( const Collection &col, qint64 &count, qint64 &size ); +- +- /** +- Returns the amount of existing items in the given collection +- which have a given flag set. +- @return -1 on error. +- */ +- static int itemWithFlagsCount( const Collection &col, const QStringList &flags ); +- +- /** + Parse cache policy and update the given Collection object accoordingly. + @param changed Indicates whether or not the cache policy already available in @p col + has actually changed +diff --git a/server/src/storage/collectionstatistics.cpp b/server/src/storage/collectionstatistics.cpp +new file mode 100644 +index 0000000..85ee449 +--- /dev/null ++++ b/server/src/storage/collectionstatistics.cpp +@@ -0,0 +1,108 @@ ++/* ++ * Copyright (C) 2014 Daniel Vrátil ++ * ++ * This library is free software; you can redistribute it and/or ++ * modify it under the terms of the GNU Lesser General Public ++ * License as published by the Free Software Foundation; either ++ * version 2.1 of the License, or (at your option) any later version. ++ * ++ * This library is distributed in the hope that it will be useful, ++ * but WITHOUT ANY WARRANTY; without even the implied warranty of ++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU ++ * Lesser General Public License for more details. ++ * ++ * You should have received a copy of the GNU Lesser General Public ++ * License along with this library; if not, write to the Free Software ++ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA ++ * ++ */ ++ ++#include "collectionstatistics.h" ++#include "querybuilder.h" ++#include "countquerybuilder.h" ++#include "akdebug.h" ++#include "entities.h" ++ ++#include ++ ++#include ++ ++using namespace Akonadi::Server; ++ ++CollectionStatistics *CollectionStatistics::sInstance = 0; ++ ++CollectionStatistics* CollectionStatistics::instance() ++{ ++ static QMutex lock; ++ lock.lock(); ++ if (sInstance == 0) { ++ sInstance = new CollectionStatistics(); ++ } ++ lock.unlock(); ++ return sInstance; ++} ++ ++void CollectionStatistics::invalidateCollection(const Collection &col) ++{ ++ QMutexLocker lock(&mCacheLock); ++ mCache.remove(col.id()); ++} ++ ++const CollectionStatistics::Statistics& CollectionStatistics::statistics(const Collection &col) ++{ ++ QMutexLocker lock(&mCacheLock); ++ auto it = mCache.find(col.id()); ++ if (it == mCache.constEnd()) { ++ it = mCache.insert(col.id(), getCollectionStatistics(col)); ++ } ++ return it.value(); ++} ++ ++CollectionStatistics::Statistics CollectionStatistics::getCollectionStatistics(const Collection &col) ++{ ++ QueryBuilder qb(PimItem::tableName()); ++ // COUNT(DISTINCT PimItemTable.id) ++ qb.addAggregation(QString::fromLatin1("DISTINCT %1") ++ .arg(PimItem::idFullColumnName()), ++ QLatin1String("count")); ++ // SUM(PimItemTable.size) ++ qb.addAggregation(PimItem::sizeFullColumnName(), QLatin1String("sum")); ++ // SUM(CASE WHEN FlagTable.name IN ('\SEEN', '$IGNORED') THEN 1 ELSE 0 END) ++ // This allows us to get read messages count in a single query with the other ++ // statistics. It is much than doing two queries, because the database ++ // only has to calculate the JOINs once. ++ // ++ // Flag::retrieveByName() will hit the Entity cache, which allows us to avoid ++ // a second JOIN with FlagTable, which PostgreSQL seems to struggle to optimize. ++ Query::Condition cond(Query::Or); ++ cond.addValueCondition(PimItemFlagRelation::rightFullColumnName(), ++ Query::Equals, ++ Flag::retrieveByName(QLatin1String(AKONADI_FLAG_SEEN)).id()); ++ cond.addValueCondition(PimItemFlagRelation::rightFullColumnName(), ++ Query::Equals, ++ Flag::retrieveByName(QLatin1String(AKONADI_FLAG_IGNORED)).id()); ++ Query::Case caseStmt(cond, QLatin1String("1"), QLatin1String("0")); ++ qb.addAggregation(caseStmt, QLatin1String("sum")); ++ ++ qb.addJoin(QueryBuilder::LeftJoin, PimItemFlagRelation::tableName(), ++ PimItem::idFullColumnName(), PimItemFlagRelation::leftFullColumnName()); ++ if (col.isVirtual()) { ++ qb.addJoin(QueryBuilder::InnerJoin, CollectionPimItemRelation::tableName(), ++ CollectionPimItemRelation::rightFullColumnName(), PimItem::idFullColumnName()); ++ qb.addValueCondition(CollectionPimItemRelation::leftFullColumnName(), Query::Equals, col.id()); ++ } else { ++ qb.addValueCondition(PimItem::collectionIdColumn(), Query::Equals, col.id()); ++ } ++ ++ if (!qb.exec()) { ++ return { -1, -1, -1 }; ++ } ++ if (!qb.query().next()) { ++ akError() << "Error during retrieving result of statistics query:" << qb.query().lastError().text(); ++ return { -1, -1, -1 }; ++ } ++ ++ return { qb.query().value(0).toLongLong(), ++ qb.query().value(1).toLongLong(), ++ qb.query().value(2).toLongLong() }; ++} +diff --git a/server/src/storage/collectionstatistics.h b/server/src/storage/collectionstatistics.h +new file mode 100644 +index 0000000..2c0af6a +--- /dev/null ++++ b/server/src/storage/collectionstatistics.h +@@ -0,0 +1,70 @@ ++/* ++ * Copyright (C) 2014 Daniel Vrátil ++ * ++ * This library is free software; you can redistribute it and/or ++ * modify it under the terms of the GNU Lesser General Public ++ * License as published by the Free Software Foundation; either ++ * version 2.1 of the License, or (at your option) any later version. ++ * ++ * This library is distributed in the hope that it will be useful, ++ * but WITHOUT ANY WARRANTY; without even the implied warranty of ++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU ++ * Lesser General Public License for more details. ++ * ++ * You should have received a copy of the GNU Lesser General Public ++ * License along with this library; if not, write to the Free Software ++ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA ++ * ++ */ ++ ++#ifndef AKONADI_SERVER_COLLECTIONSTATISTICS_H ++#define AKONADI_SERVER_COLLECTIONSTATISTICS_H ++ ++class QMutex; ++ ++#include ++#include ++ ++namespace Akonadi { ++namespace Server { ++ ++class Collection; ++ ++/** ++ * Provides cache for collection statistics ++ * ++ * Collection statistics are requested very often, so to take some load from the ++ * database we cache the results until the statistics are invalidated (see ++ * NotificationCollector, which takes care for invalidating the statistics). ++ * ++ * The cache (together with optimization of the actual SQL query) seems to ++ * massively improve initial folder listing on system start (when IO and CPU loads ++ * are very high). ++ */ ++class CollectionStatistics ++{ ++public: ++ struct Statistics { ++ qint64 count; ++ qint64 size; ++ qint64 read; ++ }; ++ ++ static CollectionStatistics* instance(); ++ ++ const Statistics& statistics(const Collection &col); ++ void invalidateCollection(const Collection &col); ++ ++private: ++ Statistics getCollectionStatistics(const Collection &col); ++ ++ QMutex mCacheLock; ++ QHash mCache; ++ ++ static CollectionStatistics *sInstance; ++}; ++ ++} // namespace Server ++} // namespace Akonadi ++ ++#endif // AKONADI_SERVER_COLLECTIONSTATISTICS_H +diff --git a/server/src/storage/datastore.cpp b/server/src/storage/datastore.cpp +index 304f0e8..0983d84 100644 +--- a/server/src/storage/datastore.cpp ++++ b/server/src/storage/datastore.cpp +@@ -209,7 +209,7 @@ DataStore *DataStore::self() + /* --- ItemFlags ----------------------------------------------------- */ + + bool DataStore::setItemsFlags( const PimItem::List &items, const QVector &flags, +- bool *flagsChanged, bool silent ) ++ bool *flagsChanged, const Collection &col, bool silent ) + { + QSet removedFlags; + QSet addedFlags; +@@ -258,7 +258,7 @@ bool DataStore::setItemsFlags( const PimItem::List &items, const QVector & + } + + if ( !silent && ( !addedFlags.isEmpty() || !removedFlags.isEmpty() ) ) { +- mNotificationCollector->itemsFlagsChanged( items, addedFlags, removedFlags ); ++ mNotificationCollector->itemsFlagsChanged( items, addedFlags, removedFlags, col ); + } + + setBoolPtr( flagsChanged, ( addedFlags != removedFlags ) ); +@@ -361,7 +361,7 @@ bool DataStore::appendItemsFlags( const PimItem::List &items, const QVector &flags, +- bool *flagsChanged, bool silent ) ++ bool *flagsChanged, const Collection &col, bool silent ) + { + QSet removedFlags; + QVariantList itemsIds; +@@ -393,7 +393,7 @@ bool DataStore::removeItemsFlags( const PimItem::List &items, const QVectoritemsFlagsChanged( items, QSet(), removedFlags ); ++ mNotificationCollector->itemsFlagsChanged( items, QSet(), removedFlags, col ); + } + } + +diff --git a/server/src/storage/datastore.h b/server/src/storage/datastore.h +index 395b227..a2d8a42 100644 +--- a/server/src/storage/datastore.h ++++ b/server/src/storage/datastore.h +@@ -119,10 +119,12 @@ class DataStore : public QObject + static DataStore *self(); + + /* --- ItemFlags ----------------------------------------------------- */ +- virtual bool setItemsFlags( const PimItem::List &items, const QVector &flags, bool *flagsChanged = 0, bool silent = false ); ++ virtual bool setItemsFlags( const PimItem::List &items, const QVector &flags, ++ bool *flagsChanged = 0, const Collection &col = Collection(), bool silent = false ); + virtual bool appendItemsFlags( const PimItem::List &items, const QVector &flags, bool *flagsChanged = 0, + bool checkIfExists = true, const Collection &col = Collection(), bool silent = false ); +- virtual bool removeItemsFlags( const PimItem::List &items, const QVector &flags, bool *tagsChanged = 0, bool silent = false ); ++ virtual bool removeItemsFlags( const PimItem::List &items, const QVector &flags, bool *tagsChanged = 0, ++ const Collection &collection = Collection(), bool silent = false ); + + /* --- ItemTags ----------------------------------------------------- */ + virtual bool setItemsTags( const PimItem::List &items, const Tag::List &tags, bool *tagsChanged = 0, bool silent = false ); +diff --git a/server/src/storage/notificationcollector.cpp b/server/src/storage/notificationcollector.cpp +index 67f57d1..dbc7883 100644 +--- a/server/src/storage/notificationcollector.cpp ++++ b/server/src/storage/notificationcollector.cpp +@@ -20,6 +20,7 @@ + #include "notificationcollector.h" + #include "storage/datastore.h" + #include "storage/entity.h" ++#include "storage/collectionstatistics.h" + #include "handlerhelper.h" + #include "cachecleaner.h" + #include "intervalcheck.h" +@@ -133,6 +134,7 @@ void NotificationCollector::collectionChanged( const Collection &collection, + if ( AkonadiServer::instance()->intervalChecker() ) { + AkonadiServer::instance()->intervalChecker()->collectionAdded( collection.id() ); + } ++ CollectionStatistics::instance()->invalidateCollection(collection); + collectionNotification( NotificationMessageV2::Modify, collection, collection.parentId(), -1, resource, changes.toSet() ); + } + +@@ -159,6 +161,8 @@ void NotificationCollector::collectionRemoved( const Collection &collection, + if ( AkonadiServer::instance()->intervalChecker() ) { + AkonadiServer::instance()->intervalChecker()->collectionRemoved( collection.id() ); + } ++ CollectionStatistics::instance()->invalidateCollection(collection); ++ + collectionNotification( NotificationMessageV2::Remove, collection, collection.parentId(), -1, resource ); + } + +@@ -183,6 +187,8 @@ void NotificationCollector::collectionUnsubscribed( const Collection &collection + if ( AkonadiServer::instance()->intervalChecker() ) { + AkonadiServer::instance()->intervalChecker()->collectionRemoved( collection.id() ); + } ++ CollectionStatistics::instance()->invalidateCollection(collection); ++ + collectionNotification( NotificationMessageV2::Unsubscribe, collection, collection.parentId(), -1, resource, QSet() ); + } + +@@ -282,6 +288,7 @@ void NotificationCollector::itemNotification( NotificationMessageV2::Operation o + copy.setParentCollection( iter.key() ); + copy.setResource( resource ); + ++ CollectionStatistics::instance()->invalidateCollection(Collection::retrieveById(iter.key())); + dispatchNotification( copy ); + } + +@@ -304,6 +311,7 @@ void NotificationCollector::itemNotification( NotificationMessageV2::Operation o + } + msg.setResource( res ); + ++ CollectionStatistics::instance()->invalidateCollection(col); + dispatchNotification( msg ); + } + +diff --git a/server/tests/unittest/fakedatastore.cpp b/server/tests/unittest/fakedatastore.cpp +index 12214fa..43ef7e6 100644 +--- a/server/tests/unittest/fakedatastore.cpp ++++ b/server/tests/unittest/fakedatastore.cpp +@@ -91,13 +91,15 @@ bool FakeDataStore::init() + bool FakeDataStore::setItemsFlags( const PimItem::List &items, + const QVector &flags, + bool *flagsChanged, ++ const Collection &col, + bool silent ) + { + mChanges.insert( QLatin1String( "setItemsFlags" ), + QVariantList() << QVariant::fromValue( items ) + << QVariant::fromValue( flags ) ++ << QVariant::fromValue( col ) + << silent ); +- return DataStore::setItemsFlags( items, flags, flagsChanged, silent ); ++ return DataStore::setItemsFlags( items, flags, flagsChanged, col, silent ); + } + + bool FakeDataStore::appendItemsFlags( const PimItem::List &items, +@@ -119,13 +121,15 @@ bool FakeDataStore::appendItemsFlags( const PimItem::List &items, + bool FakeDataStore::removeItemsFlags( const PimItem::List &items, + const QVector &flags, + bool *flagsChanged, ++ const Collection &col, + bool silent ) + { + mChanges.insert( QLatin1String( "removeItemsFlags" ), + QVariantList() << QVariant::fromValue( items ) + << QVariant::fromValue( flags ) ++ << QVariant::fromValue( col ) + << silent ); +- return DataStore::removeItemsFlags( items, flags, flagsChanged, silent ); ++ return DataStore::removeItemsFlags( items, flags, flagsChanged, col, silent ); + } + + +diff --git a/server/tests/unittest/fakedatastore.h b/server/tests/unittest/fakedatastore.h +index 62c5b75..cd9ab13 100644 +--- a/server/tests/unittest/fakedatastore.h ++++ b/server/tests/unittest/fakedatastore.h +@@ -41,6 +41,7 @@ class FakeDataStore: public DataStore + virtual bool setItemsFlags( const PimItem::List &items, + const QVector &flags, + bool *flagsChanged = 0, ++ const Collection &col = Collection(), + bool silent = false ); + virtual bool appendItemsFlags( const PimItem::List &items, + const QVector &flags, +@@ -51,6 +52,7 @@ class FakeDataStore: public DataStore + virtual bool removeItemsFlags( const PimItem::List &items, + const QVector &flags, + bool *flagsChanged = 0, ++ const Collection &col = Collection(), + bool silent = false ); + + virtual bool setItemsTags( const PimItem::List &items, +-- +2.1.0 + diff --git a/0023-Always-create-a-new-PartType-when-it-does-not-exist.patch b/0023-Always-create-a-new-PartType-when-it-does-not-exist.patch new file mode 100644 index 0000000..ba6a473 --- /dev/null +++ b/0023-Always-create-a-new-PartType-when-it-does-not-exist.patch @@ -0,0 +1,105 @@ +From 1ce732668b2b3e4d735665bd60e1a18f139b1de2 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Dan=20Vr=C3=A1til?= +Date: Fri, 5 Dec 2014 18:49:15 +0100 +Subject: [PATCH 23/30] Always create a new PartType when it does not exist + +Fixes a regression introduced in previous commit that caused Part operations +to fail when specified PartType did not exist in Akonadi storage yet. +--- + server/src/handler/append.cpp | 4 ++-- + server/src/storage/datastore.cpp | 2 +- + server/src/storage/parttypehelper.cpp | 16 +++++++++++++++- + server/src/storage/parttypehelper.h | 8 ++++++++ + 4 files changed, 26 insertions(+), 4 deletions(-) + +diff --git a/server/src/handler/append.cpp b/server/src/handler/append.cpp +index b594e27..15fb9ea 100644 +--- a/server/src/handler/append.cpp ++++ b/server/src/handler/append.cpp +@@ -134,7 +134,7 @@ bool Append::commit() + + // wrap data into a part + Part part; +- part.setPartType( PartType::retrieveByFQName( QLatin1String("PLD"), QLatin1String("RFC822") ) ); ++ part.setPartType( PartTypeHelper::fromFqName( QLatin1String("PLD"), QLatin1String("RFC822") ) ); + part.setData( m_data ); + part.setPimItemId( item.id() ); + part.setDatasize( dataSize ); +@@ -148,7 +148,7 @@ bool Append::commit() + //akDebug() << "Append handler: doPreprocessing is" << doPreprocessing; + if ( doPreprocessing ) { + Part hiddenAttribute; +- hiddenAttribute.setPartType( PartType::retrieveByFQName( QLatin1String("ATR"), QLatin1String("HIDDEN") ) ); ++ hiddenAttribute.setPartType( PartTypeHelper::fromFqName( QLatin1String("ATR"), QLatin1String("HIDDEN") ) ); + hiddenAttribute.setData( QByteArray() ); + hiddenAttribute.setPimItemId( item.id() ); + hiddenAttribute.setDatasize( 0 ); +diff --git a/server/src/storage/datastore.cpp b/server/src/storage/datastore.cpp +index 0983d84..c9fa0c3 100644 +--- a/server/src/storage/datastore.cpp ++++ b/server/src/storage/datastore.cpp +@@ -1027,7 +1027,7 @@ bool DataStore::unhideAllPimItems() + + try { + return PartHelper::remove( Part::partTypeIdFullColumnName(), +- PartType::retrieveByFQName( QLatin1String("ATR"), QLatin1String("HIDDEN") ).id() ); ++ PartTypeHelper::fromFqName( QLatin1String("ATR"), QLatin1String("HIDDEN") ).id() ); + } catch ( ... ) {} // we can live with this failing + + return false; +diff --git a/server/src/storage/parttypehelper.cpp b/server/src/storage/parttypehelper.cpp +index 7654108..bcff9c6 100644 +--- a/server/src/storage/parttypehelper.cpp ++++ b/server/src/storage/parttypehelper.cpp +@@ -37,7 +37,8 @@ QPair< QString, QString > PartTypeHelper::parseFqName(const QString& fqName) + PartType PartTypeHelper::fromFqName(const QString& fqName) + { + const QPair p = parseFqName( fqName ); +- return PartType::retrieveByFQName(p.first, p.second); ++ return fromFqName(p.first, p.second); ++ + } + + PartType PartTypeHelper::fromFqName(const QByteArray& fqName) +@@ -45,6 +46,19 @@ PartType PartTypeHelper::fromFqName(const QByteArray& fqName) + return fromFqName( QLatin1String(fqName) ); + } + ++PartType PartTypeHelper::fromFqName(const QString& ns, const QString& name) ++{ ++ PartType partType = PartType::retrieveByFQName(ns, name); ++ if (!partType.isValid()) { ++ PartType pt(name, ns); ++ if (!pt.insert()) { ++ throw PartTypeException( "Failed to append part type" ); ++ } ++ partType = pt; ++ } ++ return partType; ++} ++ + Query::Condition PartTypeHelper::conditionFromFqName(const QString& fqName) + { + const QPair p = parseFqName( fqName ); +diff --git a/server/src/storage/parttypehelper.h b/server/src/storage/parttypehelper.h +index 4c4f42f..6d3cf74 100644 +--- a/server/src/storage/parttypehelper.h ++++ b/server/src/storage/parttypehelper.h +@@ -48,6 +48,14 @@ namespace PartTypeHelper + PartType fromFqName( const QByteArray &fqName ); + + /** ++ * Retrieve (or create) PartType for the given namespace and name ++ * @param ns Namespace ++ * @param name Name ++ * @throws PartTypeException ++ */ ++ PartType fromFqName( const QString &ns, const QString &name ); ++ ++ /** + * Returns a query condition that matches the given part. + * @param fqName fully-qualified part type name + * @throws PartTypeException +-- +2.1.0 + diff --git a/0024-Fix-compilation-with-strict-iterators.patch b/0024-Fix-compilation-with-strict-iterators.patch new file mode 100644 index 0000000..6eb77f8 --- /dev/null +++ b/0024-Fix-compilation-with-strict-iterators.patch @@ -0,0 +1,25 @@ +From fcae659e9be22b00b0efe52f19a89b8fce26a925 Mon Sep 17 00:00:00 2001 +From: David Faure +Date: Sat, 6 Dec 2014 11:35:16 +0100 +Subject: [PATCH 24/30] Fix compilation with strict iterators + +--- + server/src/storage/collectionstatistics.cpp | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/server/src/storage/collectionstatistics.cpp b/server/src/storage/collectionstatistics.cpp +index 85ee449..b2c6915 100644 +--- a/server/src/storage/collectionstatistics.cpp ++++ b/server/src/storage/collectionstatistics.cpp +@@ -52,7 +52,7 @@ const CollectionStatistics::Statistics& CollectionStatistics::statistics(const C + { + QMutexLocker lock(&mCacheLock); + auto it = mCache.find(col.id()); +- if (it == mCache.constEnd()) { ++ if (it == mCache.end()) { + it = mCache.insert(col.id(), getCollectionStatistics(col)); + } + return it.value(); +-- +2.1.0 + diff --git a/0025-Avoid-repeated-calls-to-PimItem-flags-and-PimItem-ta.patch b/0025-Avoid-repeated-calls-to-PimItem-flags-and-PimItem-ta.patch new file mode 100644 index 0000000..4678e9c --- /dev/null +++ b/0025-Avoid-repeated-calls-to-PimItem-flags-and-PimItem-ta.patch @@ -0,0 +1,58 @@ +From 55dc6d141a20e2438308214ab60c18e282dd7b43 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Dan=20Vr=C3=A1til?= +Date: Mon, 8 Dec 2014 10:33:51 +0100 +Subject: [PATCH 25/30] Avoid repeated calls to PimItem::flags() and + PimItem::tags() + +The queries results are not cached, so each call to those methods runs an SQL +query. At least in case of flags, this reduced the number of queries to one +query per changed item. +--- + server/src/storage/datastore.cpp | 10 ++++++---- + 1 file changed, 6 insertions(+), 4 deletions(-) + +diff --git a/server/src/storage/datastore.cpp b/server/src/storage/datastore.cpp +index c9fa0c3..035395e 100644 +--- a/server/src/storage/datastore.cpp ++++ b/server/src/storage/datastore.cpp +@@ -220,7 +220,8 @@ bool DataStore::setItemsFlags( const PimItem::List &items, const QVector & + setBoolPtr( flagsChanged, false ); + + Q_FOREACH ( const PimItem &item, items ) { +- Q_FOREACH ( const Flag &flag, item.flags() ) { ++ const Flag::List itemFlags = item.flags(); ++ Q_FOREACH ( const Flag &flag, itemFlags ) { + if ( !flags.contains( flag ) ) { + removedFlags << flag.name().toLatin1(); + Query::Condition cond; +@@ -231,7 +232,7 @@ bool DataStore::setItemsFlags( const PimItem::List &items, const QVector & + } + + Q_FOREACH ( const Flag &flag, flags ) { +- if ( !item.flags().contains( flag ) ) { ++ if ( !itemFlags.contains( flag ) ) { + addedFlags << flag.name().toLatin1(); + insIds << item.id(); + insFlags << flag.id(); +@@ -414,7 +415,8 @@ bool DataStore::setItemsTags( const PimItem::List &items, const Tag::List &tags, + setBoolPtr( tagsChanged, false ); + + Q_FOREACH ( const PimItem &item, items ) { +- Q_FOREACH ( const Tag &tag, item.tags() ) { ++ const Tag::List itemTags = item.tags(); ++ Q_FOREACH ( const Tag &tag, itemTags ) { + if ( !tags.contains( tag ) ) { + // Remove tags from items that had it set + removedTags << tag.id(); +@@ -426,7 +428,7 @@ bool DataStore::setItemsTags( const PimItem::List &items, const Tag::List &tags, + } + + Q_FOREACH ( const Tag &tag, tags ) { +- if ( !item.tags().contains( tag ) ) { ++ if ( !itemTags.contains( tag ) ) { + // Add tags to items that did not have the tag + addedTags << tag.id(); + insIds << item.id(); +-- +2.1.0 + diff --git a/0026-Avoid-recursive-collection-listing-in-SearchHelper.patch b/0026-Avoid-recursive-collection-listing-in-SearchHelper.patch new file mode 100644 index 0000000..e5f6fba --- /dev/null +++ b/0026-Avoid-recursive-collection-listing-in-SearchHelper.patch @@ -0,0 +1,374 @@ +From 059d52845cbbc10e882764f64245c5995af4e741 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Dan=20Vr=C3=A1til?= +Date: Mon, 8 Dec 2014 13:49:27 +0100 +Subject: [PATCH 26/30] Avoid recursive collection listing in SearchHelper + +The recursive listing generates one SQL query per collection, and since search +is invoked rather often (basically whenever you open an email in KMail), we get +lots of unnecessary queries. This new algorithm does one query to fetch all +folders with matching mime types, and only falls back to query the parent chain +when the requested ancestor is not 0 (root), or when the result collection is +not a direct descendant of the requested ancestor. +--- + server/src/handler/search.cpp | 4 +- + server/src/handler/searchhelper.cpp | 111 ++++++++++++++---------- + server/src/handler/searchhelper.h | 2 +- + server/src/search/searchmanager.cpp | 2 +- + server/tests/unittest/CMakeLists.txt | 2 + + server/tests/unittest/searchtest.cpp | 158 +++++++++++++++++++++++++++++++++++ + 6 files changed, 230 insertions(+), 49 deletions(-) + create mode 100644 server/tests/unittest/searchtest.cpp + +diff --git a/server/src/handler/search.cpp b/server/src/handler/search.cpp +index 06d172f..00484ff 100644 +--- a/server/src/handler/search.cpp ++++ b/server/src/handler/search.cpp +@@ -95,9 +95,7 @@ bool Search::parseStream() + } + + if ( recursive ) { +- Q_FOREACH ( qint64 collection, collectionIds ) { +- collections << SearchHelper::listCollectionsRecursive( QVector() << collection, mimeTypes ); +- } ++ collections << SearchHelper::matchSubcollectionsByMimeType( collectionIds, mimeTypes ); + } else { + collections = collectionIds; + } +diff --git a/server/src/handler/searchhelper.cpp b/server/src/handler/searchhelper.cpp +index aa6694d..1a06c0e 100644 +--- a/server/src/handler/searchhelper.cpp ++++ b/server/src/handler/searchhelper.cpp +@@ -20,6 +20,7 @@ + + #include "searchhelper.h" + #include "storage/countquerybuilder.h" ++#include + #include "entities.h" + + #include +@@ -89,55 +90,77 @@ QString SearchHelper::extractMimetype( const QList &junks, int start + } + + +-QVector SearchHelper::listCollectionsRecursive( const QVector &ancestors, const QStringList &mimeTypes ) ++static qint64 parentCollectionId(qint64 collectionId) + { +- QVector recursiveChildren; +- Q_FOREACH ( qint64 ancestor, ancestors ) { +- QVector searchChildren; +- +- { // Free the query before entering recursion to prevent too many opened connections +- +- Query::Condition mimeTypeCondition; +- mimeTypeCondition.addColumnCondition( CollectionMimeTypeRelation::rightFullColumnName(), Query::Equals, MimeType::idFullColumnName() ); +- // Exclude top-level collections and collections that cannot have items! +- mimeTypeCondition.addValueCondition( MimeType::nameFullColumnName(), Query::NotEquals, QLatin1String( "inode/directory" ) ); +- if ( !mimeTypes.isEmpty() ) { +- mimeTypeCondition.addValueCondition( MimeType::nameFullColumnName(), Query::In, mimeTypes ); +- } ++ QueryBuilder qb(Collection::tableName(), QueryBuilder::Select); ++ qb.addColumn(Collection::parentIdColumn()); ++ qb.addValueCondition(Collection::idColumn(), Query::Equals, collectionId); ++ if (!qb.exec()) { ++ return -1; ++ } ++ if (!qb.query().next()) { ++ return -1; ++ } ++ return qb.query().value(0).toLongLong(); ++} + +- CountQueryBuilder qb( Collection::tableName(), MimeType::nameFullColumnName(), CountQueryBuilder::All ); +- qb.addColumn( Collection::idFullColumnName() ); +- qb.addJoin( QueryBuilder::LeftJoin, CollectionMimeTypeRelation::tableName(), CollectionMimeTypeRelation::leftFullColumnName(), Collection::idFullColumnName() ); +- qb.addJoin( QueryBuilder::LeftJoin, MimeType::tableName(), mimeTypeCondition ); +- if ( ancestor == 0 ) { +- qb.addValueCondition( Collection::parentIdFullColumnName(), Query::Is, QVariant() ); +- } else { +- // Also include current ancestor's result, so that we know whether we should search in the ancestor too +- Query::Condition idCond( Query::Or ); +- idCond.addValueCondition( Collection::parentIdFullColumnName(), Query::Equals, ancestor ); +- idCond.addValueCondition( Collection::idFullColumnName(), Query::Equals, ancestor ); +- qb.addCondition( idCond ); +- } +- qb.addValueCondition( Collection::isVirtualFullColumnName(), Query::Equals, false ); +- qb.addGroupColumn( Collection::idFullColumnName() ); +- qb.exec(); +- +- QSqlQuery query = qb.query(); +- while ( query.next() ) { +- const qint64 id = query.value( 1 ).toLongLong(); +- // Don't add ancestor into search children, we are resolving it right now +- if ( id != ancestor ) { +- searchChildren << id; ++ ++QVector SearchHelper::matchSubcollectionsByMimeType(const QVector &ancestors, const QStringList &mimeTypes) ++{ ++ // Get all collections with given mime types ++ QueryBuilder qb(Collection::tableName(), QueryBuilder::Select); ++ qb.setDistinct(true); ++ qb.addColumn(Collection::idFullColumnName()); ++ qb.addColumn(Collection::parentIdFullColumnName()); ++ qb.addJoin(QueryBuilder::LeftJoin, CollectionMimeTypeRelation::tableName(), ++ CollectionMimeTypeRelation::leftFullColumnName(), Collection::idFullColumnName()); ++ qb.addJoin(QueryBuilder::LeftJoin, MimeType::tableName(), ++ CollectionMimeTypeRelation::rightFullColumnName(), MimeType::idFullColumnName()); ++ Query::Condition cond(Query::Or); ++ Q_FOREACH (const QString &mt, mimeTypes) { ++ cond.addValueCondition(MimeType::nameFullColumnName(), Query::Equals, mt); ++ } ++ qb.addCondition(cond); ++ ++ if (!qb.exec()) { ++ qWarning() << "Failed to query search collections"; ++ return QVector(); ++ } ++ ++ QMap /* collectionIds */> candidateCollections; ++ while (qb.query().next()) { ++ candidateCollections[qb.query().value(1).toLongLong()].append(qb.query().value(0).toLongLong()); ++ } ++ ++ // If the ancestors list contains root, then return what we got, since everything ++ // is sub collection of root ++ QVector results; ++ if (ancestors.contains(0)) { ++ Q_FOREACH (const QVector &res, candidateCollections.values()) { ++ results += res; + } +- if ( query.value( 0 ).toInt() > 0 ) { // count( mimeTypeTable.name ) > 0 +- recursiveChildren << id; ++ return results; ++ } ++ ++ // Try to resolve direct descendants ++ Q_FOREACH (qint64 ancestor, ancestors) { ++ const QVector cols = candidateCollections.take(ancestor); ++ if (!cols.isEmpty()) { ++ results += cols; + } +- } + } +- if ( !searchChildren.isEmpty() ) { +- recursiveChildren << listCollectionsRecursive( searchChildren, mimeTypes ); ++ ++ for (auto iter = candidateCollections.begin(); iter != candidateCollections.end(); ++iter) { ++ // Traverse the collection chain up to root ++ qint64 parentId = iter.key(); ++ while (!ancestors.contains(parentId) && parentId > 0) { ++ parentId = parentCollectionId(parentId); ++ } ++ // Ok, we found a requested ancestor in the parent chain ++ if (parentId > 0) { ++ results += iter.value(); ++ } + } +- } + +- return recursiveChildren; ++ return results; + } +diff --git a/server/src/handler/searchhelper.h b/server/src/handler/searchhelper.h +index a64bb61..1595501 100644 +--- a/server/src/handler/searchhelper.h ++++ b/server/src/handler/searchhelper.h +@@ -33,7 +33,7 @@ class SearchHelper + public: + static QList splitLine( const QByteArray &line ); + static QString extractMimetype( const QList &junks, int start ); +- static QVector listCollectionsRecursive( const QVector &ancestors, const QStringList &mimeTypes ); ++ static QVector matchSubcollectionsByMimeType( const QVector &ancestors, const QStringList &mimeTypes ); + }; + + } // namespace Server +diff --git a/server/src/search/searchmanager.cpp b/server/src/search/searchmanager.cpp +index c821aa3..b940fcc 100644 +--- a/server/src/search/searchmanager.cpp ++++ b/server/src/search/searchmanager.cpp +@@ -296,7 +296,7 @@ void SearchManager::updateSearchImpl( const Collection &collection, QWaitConditi + } + + if ( recursive ) { +- queryCollections = SearchHelper::listCollectionsRecursive( queryAncestors, queryMimeTypes ); ++ queryCollections = SearchHelper::matchSubcollectionsByMimeType( queryAncestors, queryMimeTypes ); + } else { + queryCollections = queryAncestors; + } +diff --git a/server/tests/unittest/CMakeLists.txt b/server/tests/unittest/CMakeLists.txt +index b9744d9..acdc180 100644 +--- a/server/tests/unittest/CMakeLists.txt ++++ b/server/tests/unittest/CMakeLists.txt +@@ -77,3 +77,5 @@ add_server_test(listhandlertest.cpp akonadiprivate) + add_server_test(modifyhandlertest.cpp akonadiprivate) + add_server_test(createhandlertest.cpp akonadiprivate) + add_server_test(collectionreferencetest.cpp akonadiprivate) ++ ++add_server_test(searchtest.cpp akonadiprivate) +\ No newline at end of file +diff --git a/server/tests/unittest/searchtest.cpp b/server/tests/unittest/searchtest.cpp +new file mode 100644 +index 0000000..f523b09 +--- /dev/null ++++ b/server/tests/unittest/searchtest.cpp +@@ -0,0 +1,158 @@ ++/* ++ * Copyright (C) 2014 Daniel Vrátil ++ * ++ * This library is free software; you can redistribute it and/or ++ * modify it under the terms of the GNU Lesser General Public ++ * License as published by the Free Software Foundation; either ++ * version 2.1 of the License, or (at your option) any later version. ++ * ++ * This library is distributed in the hope that it will be useful, ++ * but WITHOUT ANY WARRANTY; without even the implied warranty of ++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU ++ * Lesser General Public License for more details. ++ * ++ * You should have received a copy of the GNU Lesser General Public ++ * License along with this library; if not, write to the Free Software ++ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA ++ * ++ */ ++ ++#include "fakeakonadiserver.h" ++#include "searchhelper.h" ++#include "akdebug.h" ++#include "aktest.h" ++ ++#include ++ ++#include ++ ++using namespace Akonadi::Server; ++ ++Q_DECLARE_METATYPE(QList) ++Q_DECLARE_METATYPE(QList) ++ ++ ++class SearchTest : public QObject ++{ ++ Q_OBJECT ++ ++public: ++ SearchTest() ++ : QObject() ++ { ++ try { ++ FakeAkonadiServer::instance()->setPopulateDb(false); ++ FakeAkonadiServer::instance()->init(); ++ } catch (const FakeAkonadiServerException &e) { ++ akError() << "Server exception: " << e.what(); ++ akFatal() << "Fake Akonadi Server failed to start up, aborting test"; ++ } ++ } ++ ++ ~SearchTest() ++ { ++ FakeAkonadiServer::instance()->quit(); ++ } ++ ++ Collection createCollection(const Resource &res, const QString &name, const Collection &parent, const QStringList &mimetypes) ++ { ++ Collection col; ++ col.setName(name); ++ col.setResource(res); ++ col.setParentId(parent.isValid() ? parent.id() : 0); ++ col.insert(); ++ Q_FOREACH (const QString &mimeType, mimetypes) { ++ MimeType mt = MimeType::retrieveByName(mimeType); ++ if (!mt.isValid()) { ++ mt = MimeType(mimeType); ++ mt.insert(); ++ } ++ col.addMimeType(mt); ++ } ++ return col; ++ } ++ ++private Q_SLOTS: ++ void testSearchHelperCollectionListing_data() ++ { ++ /* ++ Fake Resource ++ |- Col 1 (inode/directory) ++ | |- Col 2 (inode/direcotry, application/octet-stream) ++ | | |- Col 3(application/octet-stream) ++ | |- Col 4 (text/plain) ++ |- Col 5 (inode/directory, text/plain) ++ |- Col 6 (inode/directory, application/octet-stream) ++ |- Col 7 (inode/directory, text/plain) ++ |- Col 8 (inode/directory, application/octet-stream) ++ |- Col 9 (unique/mime-type) ++ */ ++ ++ Resource res(QLatin1String("Test Resource"), false); ++ res.insert(); ++ ++ Collection col1 = createCollection(res, QLatin1String("Col 1"), Collection(), ++ QStringList() << QLatin1String("inode/directory")); ++ Collection col2 = createCollection(res, QLatin1String("Col 2"), col1, ++ QStringList() << QLatin1String("inode/directory") ++ << QLatin1String("application/octet-stream")); ++ Collection col3 = createCollection(res, QLatin1String("Col 3"), col2, ++ QStringList() << QLatin1String("application/octet-stream")); ++ Collection col4 = createCollection(res, QLatin1String("Col 4"), col2, ++ QStringList() << QLatin1String("text/plain")); ++ Collection col5 = createCollection(res, QLatin1String("Col 5"), Collection(), ++ QStringList() << QLatin1String("inode/directory") ++ << QLatin1String("text/plain")); ++ Collection col6 = createCollection(res, QLatin1String("Col 6"), col5, ++ QStringList() << QLatin1String("inode/directory") ++ << QLatin1String("application/octet-stream")); ++ Collection col7 = createCollection(res, QLatin1String("Col 7"), col5, ++ QStringList() << QLatin1String("inode/directory") ++ << QLatin1String("text/plain")); ++ Collection col8 = createCollection(res, QLatin1String("Col 8"), col7, ++ QStringList() << QLatin1String("text/directory") ++ << QLatin1String("application/octet-stream")); ++ Collection col9 = createCollection(res, QLatin1String("Col 9"), col8, ++ QStringList() << QLatin1String("unique/mime-type")); ++ ++ QTest::addColumn>("ancestors"); ++ QTest::addColumn("mimetypes"); ++ QTest::addColumn>("expectedResults"); ++ ++ QTest::newRow("") << QVector({ 0 }) ++ << QStringList({ QLatin1String("text/plain") }) ++ << QVector({ col4.id(), col5.id(), col7.id() }); ++ QTest::newRow("") << QVector({ 0 }) ++ << QStringList({ QLatin1String("application/octet-stream") }) ++ << QVector({ col2.id(), col3.id(), col6.id(), col8.id() }); ++ QTest::newRow("") << QVector({ col1.id() }) ++ << QStringList({ QLatin1String("text/plain") }) ++ << QVector({ col4.id() }); ++ QTest::newRow("") << QVector({ col1.id() }) ++ << QStringList({ QLatin1String("unique/mime-type") }) ++ << QVector(); ++ QTest::newRow("") << QVector({ col2.id(), col7.id() }) ++ << QStringList({ QLatin1String("application/octet-stream") }) ++ << QVector({ col3.id(), col8.id() }); ++ } ++ ++ void testSearchHelperCollectionListing() ++ { ++ QFETCH(QVector, ancestors); ++ QFETCH(QStringList, mimetypes); ++ QFETCH(QVector, expectedResults); ++ ++ QVector results = SearchHelper::matchSubcollectionsByMimeType(ancestors, mimetypes); ++ ++ qSort(expectedResults); ++ qSort(results); ++ ++ QCOMPARE(results.size(), expectedResults.size()); ++ QCOMPARE(results, expectedResults); ++ } ++ ++}; ++ ++AKTEST_FAKESERVER_MAIN(SearchTest) ++ ++#include "searchtest.moc" +-- +2.1.0 + diff --git a/0027-Minor-improvements-in-StatisticsCache-as-suggested-b.patch b/0027-Minor-improvements-in-StatisticsCache-as-suggested-b.patch new file mode 100644 index 0000000..faac95b --- /dev/null +++ b/0027-Minor-improvements-in-StatisticsCache-as-suggested-b.patch @@ -0,0 +1,185 @@ +From ac118e12fca25826340b6c8561939be19c4b7170 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Dan=20Vr=C3=A1til?= +Date: Mon, 8 Dec 2014 13:55:58 +0100 +Subject: [PATCH 27/30] Minor improvements in StatisticsCache as suggested by + Millian + +* rename instance() to self() +* first call self() from main thread to avoid having mutex +* use CountQueryBuilder +--- + server/src/akonadi.cpp | 4 +++- + server/src/handler/select.cpp | 2 +- + server/src/handler/status.cpp | 2 +- + server/src/handlerhelper.cpp | 2 +- + server/src/storage/collectionstatistics.cpp | 10 ++-------- + server/src/storage/collectionstatistics.h | 2 +- + server/src/storage/notificationcollector.cpp | 10 +++++----- + 7 files changed, 14 insertions(+), 18 deletions(-) + +diff --git a/server/src/akonadi.cpp b/server/src/akonadi.cpp +index 5369320..faef3a5 100644 +--- a/server/src/akonadi.cpp ++++ b/server/src/akonadi.cpp +@@ -35,6 +35,7 @@ + #include "utils.h" + #include "debuginterface.h" + #include "storage/itemretrievalthread.h" ++#include "storage/collectionstatistics.h" + #include "preprocessormanager.h" + #include "search/searchmanager.h" + #include "search/searchtaskmanagerthread.h" +@@ -169,6 +170,8 @@ bool AkonadiServer::init() + new DebugInterface( this ); + ResourceManager::self(); + ++ CollectionStatistics::self(); ++ + // Initialize the preprocessor manager + PreprocessorManager::init(); + +@@ -194,7 +197,6 @@ bool AkonadiServer::init() + mAgentSearchManagerThread = new SearchTaskManagerThread( this ); + mAgentSearchManagerThread->start(); + +- + const QStringList searchManagers = settings.value( QLatin1String( "Search/Manager" ), + QStringList() << QLatin1String( "Nepomuk" ) + << QLatin1String( "Agent" ) ).toStringList(); +diff --git a/server/src/handler/select.cpp b/server/src/handler/select.cpp +index f1ecc44..a94d971 100644 +--- a/server/src/handler/select.cpp ++++ b/server/src/handler/select.cpp +@@ -97,7 +97,7 @@ bool Select::parseStream() + response.setString( "FLAGS (" + Flag::joinByName( Flag::retrieveAll(), QLatin1String( " " ) ).toLatin1() + ")" ); + Q_EMIT responseAvailable( response ); + +- const CollectionStatistics::Statistics stats = CollectionStatistics::instance()->statistics(col); ++ const CollectionStatistics::Statistics stats = CollectionStatistics::self()->statistics(col); + if ( stats.count == -1 ) { + return failureResponse( "Unable to determine item count" ); + } +diff --git a/server/src/handler/status.cpp b/server/src/handler/status.cpp +index 283532c..5fc9bb1 100644 +--- a/server/src/handler/status.cpp ++++ b/server/src/handler/status.cpp +@@ -63,7 +63,7 @@ bool Status::parseStream() + // Responses: + // REQUIRED untagged responses: STATUS + +- const CollectionStatistics::Statistics &stats = CollectionStatistics::instance()->statistics(col); ++ const CollectionStatistics::Statistics &stats = CollectionStatistics::self()->statistics(col); + if (stats.count == -1) { + return failureResponse( "Failed to query statistics." ); + } +diff --git a/server/src/handlerhelper.cpp b/server/src/handlerhelper.cpp +index 39583ce..a88bc6e 100644 +--- a/server/src/handlerhelper.cpp ++++ b/server/src/handlerhelper.cpp +@@ -166,7 +166,7 @@ QByteArray HandlerHelper::collectionToByteArray( const Collection &col, bool hid + b += " " AKONADI_PARAM_VIRTUAL " " + QByteArray::number( col.isVirtual() ) + ' '; + + if ( includeStatistics ) { +- const CollectionStatistics::Statistics &stats = CollectionStatistics::instance()->statistics(col); ++ const CollectionStatistics::Statistics &stats = CollectionStatistics::self()->statistics(col); + if (stats.count > -1) { + b += AKONADI_ATTRIBUTE_MESSAGES " " + QByteArray::number( stats.count ) + ' '; + b += AKONADI_ATTRIBUTE_UNSEEN " "; +diff --git a/server/src/storage/collectionstatistics.cpp b/server/src/storage/collectionstatistics.cpp +index b2c6915..7307956 100644 +--- a/server/src/storage/collectionstatistics.cpp ++++ b/server/src/storage/collectionstatistics.cpp +@@ -31,14 +31,11 @@ using namespace Akonadi::Server; + + CollectionStatistics *CollectionStatistics::sInstance = 0; + +-CollectionStatistics* CollectionStatistics::instance() ++CollectionStatistics* CollectionStatistics::self() + { +- static QMutex lock; +- lock.lock(); + if (sInstance == 0) { + sInstance = new CollectionStatistics(); + } +- lock.unlock(); + return sInstance; + } + +@@ -60,11 +57,8 @@ const CollectionStatistics::Statistics& CollectionStatistics::statistics(const C + + CollectionStatistics::Statistics CollectionStatistics::getCollectionStatistics(const Collection &col) + { +- QueryBuilder qb(PimItem::tableName()); + // COUNT(DISTINCT PimItemTable.id) +- qb.addAggregation(QString::fromLatin1("DISTINCT %1") +- .arg(PimItem::idFullColumnName()), +- QLatin1String("count")); ++ CountQueryBuilder qb(PimItem::tableName(), PimItem::idFullColumnName(), CountQueryBuilder::Distinct); + // SUM(PimItemTable.size) + qb.addAggregation(PimItem::sizeFullColumnName(), QLatin1String("sum")); + // SUM(CASE WHEN FlagTable.name IN ('\SEEN', '$IGNORED') THEN 1 ELSE 0 END) +diff --git a/server/src/storage/collectionstatistics.h b/server/src/storage/collectionstatistics.h +index 2c0af6a..a0b9f24 100644 +--- a/server/src/storage/collectionstatistics.h ++++ b/server/src/storage/collectionstatistics.h +@@ -50,7 +50,7 @@ public: + qint64 read; + }; + +- static CollectionStatistics* instance(); ++ static CollectionStatistics* self(); + + const Statistics& statistics(const Collection &col); + void invalidateCollection(const Collection &col); +diff --git a/server/src/storage/notificationcollector.cpp b/server/src/storage/notificationcollector.cpp +index dbc7883..7ed255c 100644 +--- a/server/src/storage/notificationcollector.cpp ++++ b/server/src/storage/notificationcollector.cpp +@@ -134,7 +134,7 @@ void NotificationCollector::collectionChanged( const Collection &collection, + if ( AkonadiServer::instance()->intervalChecker() ) { + AkonadiServer::instance()->intervalChecker()->collectionAdded( collection.id() ); + } +- CollectionStatistics::instance()->invalidateCollection(collection); ++ CollectionStatistics::self()->invalidateCollection(collection); + collectionNotification( NotificationMessageV2::Modify, collection, collection.parentId(), -1, resource, changes.toSet() ); + } + +@@ -161,7 +161,7 @@ void NotificationCollector::collectionRemoved( const Collection &collection, + if ( AkonadiServer::instance()->intervalChecker() ) { + AkonadiServer::instance()->intervalChecker()->collectionRemoved( collection.id() ); + } +- CollectionStatistics::instance()->invalidateCollection(collection); ++ CollectionStatistics::self()->invalidateCollection(collection); + + collectionNotification( NotificationMessageV2::Remove, collection, collection.parentId(), -1, resource ); + } +@@ -187,7 +187,7 @@ void NotificationCollector::collectionUnsubscribed( const Collection &collection + if ( AkonadiServer::instance()->intervalChecker() ) { + AkonadiServer::instance()->intervalChecker()->collectionRemoved( collection.id() ); + } +- CollectionStatistics::instance()->invalidateCollection(collection); ++ CollectionStatistics::self()->invalidateCollection(collection); + + collectionNotification( NotificationMessageV2::Unsubscribe, collection, collection.parentId(), -1, resource, QSet() ); + } +@@ -288,7 +288,7 @@ void NotificationCollector::itemNotification( NotificationMessageV2::Operation o + copy.setParentCollection( iter.key() ); + copy.setResource( resource ); + +- CollectionStatistics::instance()->invalidateCollection(Collection::retrieveById(iter.key())); ++ CollectionStatistics::self()->invalidateCollection(Collection::retrieveById(iter.key())); + dispatchNotification( copy ); + } + +@@ -311,7 +311,7 @@ void NotificationCollector::itemNotification( NotificationMessageV2::Operation o + } + msg.setResource( res ); + +- CollectionStatistics::instance()->invalidateCollection(col); ++ CollectionStatistics::self()->invalidateCollection(col); + dispatchNotification( msg ); + } + +-- +2.1.0 + diff --git a/0028-Extend-imapparser-benchmark-and-keep-static-data-aro.patch b/0028-Extend-imapparser-benchmark-and-keep-static-data-aro.patch new file mode 100644 index 0000000..20d26c5 --- /dev/null +++ b/0028-Extend-imapparser-benchmark-and-keep-static-data-aro.patch @@ -0,0 +1,74 @@ +From da5751c7b1589d2ea5800a3cf96dfc93b23b9783 Mon Sep 17 00:00:00 2001 +From: Milian Wolff +Date: Tue, 9 Dec 2014 14:35:04 +0100 +Subject: [PATCH 28/30] Extend imapparser benchmark and keep static data + around. + +This gets rid of some temporary allocations and thus speeds up +the whole process a bit. + +REVIEW: 121406 +--- + libs/imapparser.cpp | 5 +++-- + libs/tests/imapparserbenchmark.cpp | 22 ++++++++++++++++++++++ + 2 files changed, 25 insertions(+), 2 deletions(-) + +diff --git a/libs/imapparser.cpp b/libs/imapparser.cpp +index 9b437e2..6f9f592 100644 +--- a/libs/imapparser.cpp ++++ b/libs/imapparser.cpp +@@ -364,7 +364,8 @@ int ImapParser::parseNumber( const QByteArray &data, qint64 &result, bool *ok, i + QByteArray ImapParser::quote( const QByteArray &data ) + { + if ( data.isEmpty() ) { +- return QByteArray( "\"\"" ); ++ static const QByteArray empty( "\"\"" ); ++ return empty; + } + + const int inputLength = data.length(); +@@ -499,7 +500,7 @@ int ImapParser::parseDateTime( const QByteArray &data, QDateTime &dateTime, int + } + + pos += 3; +- const QByteArray shortMonthNames( "janfebmaraprmayjunjulaugsepoctnovdec" ); ++ static const QByteArray shortMonthNames( "janfebmaraprmayjunjulaugsepoctnovdec" ); + int month = shortMonthNames.indexOf( data.mid( pos, 3 ).toLower() ); + if ( month == -1 ) { + return start; +diff --git a/libs/tests/imapparserbenchmark.cpp b/libs/tests/imapparserbenchmark.cpp +index 17dac66..fd4335c 100644 +--- a/libs/tests/imapparserbenchmark.cpp ++++ b/libs/tests/imapparserbenchmark.cpp +@@ -94,6 +94,28 @@ class ImapParserBenchmark : public QObject + ImapParser::parseParenthesizedList( data, result, 0 ); + } + } ++ ++ void parseNumber() ++ { ++ QByteArray data( "123456" ); ++ qint64 result; ++ bool ok = false; ++ QBENCHMARK { ++ ImapParser::parseNumber( data, result, &ok ); ++ } ++ QVERIFY(ok); ++ QCOMPARE(result, qint64(123456)); ++ } ++ ++ void parseDateTime() ++ { ++ QByteArray data( "28-May-2006 01:03:35 +0000" ); ++ QDateTime result; ++ QBENCHMARK { ++ ImapParser::parseDateTime( data, result ); ++ } ++ QCOMPARE(result.toString( QString::fromUtf8( "dd-MMM-yyyy hh:mm:ss +0000" ) ), QString::fromUtf8( data )); ++ } + }; + + #include "imapparserbenchmark.moc" +-- +2.1.0 + diff --git a/0029-Reduce-the-amount-of-allocations-by-preallocating-a-.patch b/0029-Reduce-the-amount-of-allocations-by-preallocating-a-.patch new file mode 100644 index 0000000..279a966 --- /dev/null +++ b/0029-Reduce-the-amount-of-allocations-by-preallocating-a-.patch @@ -0,0 +1,61 @@ +From 89357c7b0fc5e76091510af504058c036fa1b2f9 Mon Sep 17 00:00:00 2001 +From: Milian Wolff +Date: Wed, 10 Dec 2014 21:04:24 +0100 +Subject: [PATCH 29/30] Reduce the amount of allocations by preallocating a + buffer. + +Sadly, QByteArray cannot be cleared without destroying the buffer. +But we can guesstimate the target size of the buffer and thus +reduce the amount of allocations. + +This also adds a benchmark for ImapParser::parseString. +--- + libs/imapparser.cpp | 1 + + libs/tests/imapparserbenchmark.cpp | 19 +++++++++++++++++++ + 2 files changed, 20 insertions(+) + +diff --git a/libs/imapparser.cpp b/libs/imapparser.cpp +index 6f9f592..f3301e7 100644 +--- a/libs/imapparser.cpp ++++ b/libs/imapparser.cpp +@@ -186,6 +186,7 @@ int ImapParser::parseQuotedString( const QByteArray &data, QByteArray &result, i + // quoted string + if ( data[begin] == '"' ) { + ++begin; ++ result.reserve(qMin(32, data.size() - begin)); + for ( int i = begin; i < data.length(); ++i ) { + const char ch = data.at( i ); + if ( foundSlash ) { +diff --git a/libs/tests/imapparserbenchmark.cpp b/libs/tests/imapparserbenchmark.cpp +index fd4335c..ee861a0 100644 +--- a/libs/tests/imapparserbenchmark.cpp ++++ b/libs/tests/imapparserbenchmark.cpp +@@ -95,6 +95,25 @@ class ImapParserBenchmark : public QObject + } + } + ++ void parseString_data() ++ { ++ QTest::addColumn( "data" ); ++ QTest::newRow("plain") << QByteArray("fooobarasdf something more lalala"); ++ QTest::newRow("quoted") << QByteArray("\"fooobarasdf\" something more lalala"); ++ } ++ ++ void parseString() ++ { ++ QFETCH(QByteArray, data); ++ QByteArray result; ++ qint64 sum = 0; ++ QBENCHMARK { ++ sum += ImapParser::parseString( data, result ); ++ } ++ QVERIFY(!result.isEmpty()); ++ QVERIFY(sum > 0); ++ } ++ + void parseNumber() + { + QByteArray data( "123456" ); +-- +2.1.0 + diff --git a/0030-Preallocate-a-capacity-of-16-for-the-returned-list.patch b/0030-Preallocate-a-capacity-of-16-for-the-returned-list.patch new file mode 100644 index 0000000..bf96092 --- /dev/null +++ b/0030-Preallocate-a-capacity-of-16-for-the-returned-list.patch @@ -0,0 +1,119 @@ +From c733429f4fa9696fb027ddc946e54f6bbb68deaf Mon Sep 17 00:00:00 2001 +From: Milian Wolff +Date: Wed, 10 Dec 2014 21:16:45 +0100 +Subject: [PATCH 30/30] Preallocate a capacity of 16 for the returned list. + +See also 159abaf2f372eaa633db8f69ff6b1edd459998cc in kdepimlibs on +why. I'll quote it here again: + + In my data, most often the final size of fetchResponse is 16. + By reserving that data upfront, we can get rid of 3/4 of the + list allocations, according to the growth-strategy outlined here: + http://qt-project.org/doc/qt-4.8/containers.html#growth-strategies + I.e. before, we'd allocate 4, 8, 12, 16. Now we directly allocate + room for 16. + +Thing is, doing this outside of akonadi has no effect, as QList::clear +destroys the internal buffer. With the added benchmark, I now verified +that this patch here does have a positive effect. +--- + libs/imapparser.cpp | 2 ++ + libs/tests/imapparserbenchmark.cpp | 52 ++++++++++++++++++++++++++------------ + 2 files changed, 38 insertions(+), 16 deletions(-) + +diff --git a/libs/imapparser.cpp b/libs/imapparser.cpp +index f3301e7..f5a7457 100644 +--- a/libs/imapparser.cpp ++++ b/libs/imapparser.cpp +@@ -79,6 +79,8 @@ int parseParenthesizedListHelper( const QByteArray &data, T &result, int start ) + return start; + } + ++ result.reserve(16); ++ + int count = 0; + int sublistBegin = start; + bool insideQuote = false; +diff --git a/libs/tests/imapparserbenchmark.cpp b/libs/tests/imapparserbenchmark.cpp +index ee861a0..7545238 100644 +--- a/libs/tests/imapparserbenchmark.cpp ++++ b/libs/tests/imapparserbenchmark.cpp +@@ -27,6 +27,25 @@ Q_DECLARE_METATYPE( QList ) + class ImapParserBenchmark : public QObject + { + Q_OBJECT ++ private: ++ void geneateParseParenthesizedListData() ++ { ++ QTest::addColumn( "data" ); ++ QTest::newRow( "empty" ) << QByteArray(); ++ QTest::newRow( "unnested" ) << QByteArray("(\"Foo Bar\" NIL \"foobar\" \"test.com\")"); ++ QTest::newRow( "nested" ) << QByteArray("((\"Foo Bar\" NIL \"foobar\" \"test.com\"))"); ++ QTest::newRow( "nested-long" ) << QByteArray("(UID 86 REV 0 MIMETYPE \"message/rfc822\" COLLECTIONID 13 SIZE 6114 FLAGS (\\SEEN)" ++ " ANCESTORS ((13 \"/INBOX\") (12 \"imap://mail@mail.test.com/\") (0 \"\")) PLD:ENVELOPE[1] {396}" ++ " (\"Fri, 04 Jun 2010 09:07:54 +0200\" \"Re: [ADMIN] foobar available again!\"" ++ " ((\"Foo Bar\" NIL \"foobar\" \"test.com\"))" ++ " NIL NIL" ++ " ((\"Asdf Bla Blub\" NIL \"asdf.bla.blub\" \"123test.org\"))" ++ " ((NIL NIL \"muh.kuh\" \"lalala.com\") (\"Konqi KDE\" NIL \"konqi\" \"kde.org\") (NIL NIL \"all\" \"test.com\"))" ++ " NIL \"<201006040905.33367.foo.bar@test.com>\" \"<4C08A64A.9020205@123test.org>\"" ++ " \"<201006040142.56540.muh.kuh@lalala.com> <201006040704.39648.konqi@kde.org> <201006040905.33367.foo.bar@test.com>\"" ++ "))"); ++ } ++ + private Q_SLOTS: + void quote_data() + { +@@ -68,25 +87,12 @@ class ImapParserBenchmark : public QObject + } + } + +- void parseParenthesizedList_data() ++ void parseParenthesizedQVarLengthArray_data() + { +- QTest::addColumn( "data" ); +- QTest::newRow( "empty" ) << QByteArray(); +- QTest::newRow( "unnested" ) << QByteArray("(\"Foo Bar\" NIL \"foobar\" \"test.com\")"); +- QTest::newRow( "nested" ) << QByteArray("((\"Foo Bar\" NIL \"foobar\" \"test.com\"))"); +- QTest::newRow( "nested-long" ) << QByteArray("(UID 86 REV 0 MIMETYPE \"message/rfc822\" COLLECTIONID 13 SIZE 6114 FLAGS (\\SEEN)" +- " ANCESTORS ((13 \"/INBOX\") (12 \"imap://mail@mail.test.com/\") (0 \"\")) PLD:ENVELOPE[1] {396}" +- " (\"Fri, 04 Jun 2010 09:07:54 +0200\" \"Re: [ADMIN] foobar available again!\"" +- " ((\"Foo Bar\" NIL \"foobar\" \"test.com\"))" +- " NIL NIL" +- " ((\"Asdf Bla Blub\" NIL \"asdf.bla.blub\" \"123test.org\"))" +- " ((NIL NIL \"muh.kuh\" \"lalala.com\") (\"Konqi KDE\" NIL \"konqi\" \"kde.org\") (NIL NIL \"all\" \"test.com\"))" +- " NIL \"<201006040905.33367.foo.bar@test.com>\" \"<4C08A64A.9020205@123test.org>\"" +- " \"<201006040142.56540.muh.kuh@lalala.com> <201006040704.39648.konqi@kde.org> <201006040905.33367.foo.bar@test.com>\"" +- "))"); ++ geneateParseParenthesizedListData(); + } + +- void parseParenthesizedList() ++ void parseParenthesizedQVarLengthArray() + { + QFETCH( QByteArray, data ); + QVarLengthArray result; +@@ -95,6 +101,20 @@ class ImapParserBenchmark : public QObject + } + } + ++ void parseParenthesizedQList_data() ++ { ++ geneateParseParenthesizedListData(); ++ } ++ ++ void parseParenthesizedQList() ++ { ++ QFETCH( QByteArray, data ); ++ QList result; ++ QBENCHMARK { ++ ImapParser::parseParenthesizedList( data, result, 0 ); ++ } ++ } ++ + void parseString_data() + { + QTest::addColumn( "data" ); +-- +2.1.0 + diff --git a/akonadi.spec b/akonadi.spec index 841e0d4..b99b605 100644 --- a/akonadi.spec +++ b/akonadi.spec @@ -18,7 +18,7 @@ Summary: PIM Storage Service Name: akonadi Version: 1.13.0 -Release: 7%{?dist} +Release: 8%{?dist} License: LGPLv2+ URL: http://community.kde.org/KDE_PIM/Akonadi @@ -46,7 +46,27 @@ Patch6: 0006-Don-t-crash-when-setmntent-returns-NULL.patch Patch7: 0007-Don-t-call-insert-from-Q_ASSERT-breaks-unit-tests-in.patch Patch8: 0008-Suppress-unused-variable-warnings-in-release-mode.patch Patch9: 0009-Test-whether-compiler-supports-all-required-C-11-fea.patch - +Patch10: 0010-prevent-starting-a-QTimer-with-a-negative-interval.patch +Patch11: 0011-Convert-some-qDebugs-to-akDebugs.patch +Patch12: 0012-Optimize-Reduce-the-amount-of-allocations-required-t.patch +Patch13: 0013-Intern-entity-strings-for-table-and-column-names.patch +Patch14: 0014-No-semicolon-after-Q_DECLARE_METATYPE.patch +Patch15: 0015-Use-QMutexLocker-instead-of-manual-lock-unlock-calls.patch +Patch16: 0016-Use-an-QAtomicInt-instead-of-a-plain-bool-for-Entity.patch +Patch17: 0017-Optimize-Only-do-one-hash-lookup-to-retrieve-value-f.patch +Patch18: 0018-Optimize-Skip-value-condition-on-invalid-flags.patch +Patch19: 0019-Optimize-queries-Do-not-retrieve-known-key-used-in-t.patch +Patch20: 0020-Avoid-ridiculous-amount-of-SQL-queries-by-caching-Pa.patch +Patch21: 0021-Implement-support-for-CASE.WHEN.THEN-SQL-statements-.patch +Patch22: 0022-Implement-cache-for-CollectionStatistics-to-signific.patch +Patch23: 0023-Always-create-a-new-PartType-when-it-does-not-exist.patch +Patch24: 0024-Fix-compilation-with-strict-iterators.patch +Patch25: 0025-Avoid-repeated-calls-to-PimItem-flags-and-PimItem-ta.patch +Patch26: 0026-Avoid-recursive-collection-listing-in-SearchHelper.patch +Patch27: 0027-Minor-improvements-in-StatisticsCache-as-suggested-b.patch +Patch28: 0028-Extend-imapparser-benchmark-and-keep-static-data-aro.patch +Patch29: 0029-Reduce-the-amount-of-allocations-by-preallocating-a-.patch +Patch30: 0030-Preallocate-a-capacity-of-16-for-the-returned-list.patch %define mysql_conf_timestamp 20140709 @@ -210,6 +230,9 @@ fi %changelog +* Sat Jan 31 2015 Rex Dieter 1.13.0-8 +- latest 1.13 branch fixes + * Tue Jan 27 2015 Petr Machata - 1.13.0-7 - Rebuild for boost 1.57.0