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