Speed up playlist restoring by moving sqlite query off main thread

The playlist fetching uses QtConcurrent to make the playlist on a
different thread (possibly concurrently for each item).
However, profiling reveals that the slow operation is fetching
the rows from the SQLite database, making this redundant.

Instead move the whole playlist loading, including the database access,
into a single function, and call that function in a different thread via
QtConcurrent::run.

This has the side effect of moving all the concurrent stuff from
PlaylistBackend into the callers.

kstartperf measures:

Before: 7.5s cold
        3.6 s warm

After: ~4.0 s cold
       3.5 s warm
This commit is contained in:
Simeon Bird 2014-12-06 20:41:31 -05:00
parent 16b2ac547c
commit 09e839353e
5 changed files with 48 additions and 43 deletions

View File

@ -1446,7 +1446,7 @@ void Playlist::Save() const {
} }
namespace { namespace {
typedef QFutureWatcher<shared_ptr<PlaylistItem>> PlaylistItemFutureWatcher; typedef QFutureWatcher<QList<PlaylistItemPtr>> PlaylistItemFutureWatcher;
} }
void Playlist::Restore() { void Playlist::Restore() {
@ -1456,7 +1456,8 @@ void Playlist::Restore() {
virtual_items_.clear(); virtual_items_.clear();
library_items_by_id_.clear(); library_items_by_id_.clear();
PlaylistBackend::PlaylistItemFuture future = backend_->GetPlaylistItems(id_); QFuture<QList<PlaylistItemPtr>> future =
QtConcurrent::run(backend_, &PlaylistBackend::GetPlaylistItems, id_);
PlaylistItemFutureWatcher* watcher = new PlaylistItemFutureWatcher(this); PlaylistItemFutureWatcher* watcher = new PlaylistItemFutureWatcher(this);
watcher->setFuture(future); watcher->setFuture(future);
connect(watcher, SIGNAL(finished()), SLOT(ItemsLoaded())); connect(watcher, SIGNAL(finished()), SLOT(ItemsLoaded()));
@ -1467,7 +1468,7 @@ void Playlist::ItemsLoaded() {
static_cast<PlaylistItemFutureWatcher*>(sender()); static_cast<PlaylistItemFutureWatcher*>(sender());
watcher->deleteLater(); watcher->deleteLater();
PlaylistItemList items = watcher->future().results(); PlaylistItemList items = watcher->future().result();
// backend returns empty elements for library items which it couldn't // backend returns empty elements for library items which it couldn't
// match (because they got deleted); we don't need those // match (because they got deleted); we don't need those

View File

@ -24,7 +24,6 @@
#include <QHash> #include <QHash>
#include <QMutexLocker> #include <QMutexLocker>
#include <QSqlQuery> #include <QSqlQuery>
#include <QtConcurrentMap>
#include <QtDebug> #include <QtDebug>
#include "core/application.h" #include "core/application.h"
@ -138,7 +137,7 @@ PlaylistBackend::Playlist PlaylistBackend::GetPlaylist(int id) {
return p; return p;
} }
QList<SqlRow> PlaylistBackend::GetPlaylistRows(int playlist) { QSqlQuery PlaylistBackend::GetPlaylistRows(int playlist) {
QMutexLocker l(db_->Mutex()); QMutexLocker l(db_->Mutex());
QSqlDatabase db(db_->Connect()); QSqlDatabase db(db_->Connect());
@ -162,42 +161,46 @@ QList<SqlRow> PlaylistBackend::GetPlaylistRows(int playlist) {
" LEFT JOIN jamendo.songs AS jamendo_songs" " LEFT JOIN jamendo.songs AS jamendo_songs"
" ON p.library_id = jamendo_songs.ROWID" " ON p.library_id = jamendo_songs.ROWID"
" WHERE p.playlist = :playlist"; " WHERE p.playlist = :playlist";
QSqlQuery q(query, db); QSqlQuery q(db);
// Forward iterations only may be faster
q.setForwardOnly(true);
q.prepare(query);
q.bindValue(":playlist", playlist); q.bindValue(":playlist", playlist);
q.exec(); q.exec();
if (db_->CheckErrors(q)) return QList<SqlRow>();
QList<SqlRow> rows; return q;
}
QList<PlaylistItemPtr> PlaylistBackend::GetPlaylistItems(int playlist) {
QSqlQuery q = GetPlaylistRows(playlist);
// Note that as this only accesses the query, not the db, we don't need the
// mutex.
if (db_->CheckErrors(q)) return QList<PlaylistItemPtr>();
// it's probable that we'll have a few songs associated with the
// same CUE so we're caching results of parsing CUEs
std::shared_ptr<NewSongFromQueryState> state_ptr(new NewSongFromQueryState());
QList<PlaylistItemPtr> playlistitems;
while (q.next()) { while (q.next()) {
rows << SqlRow(q); playlistitems << NewPlaylistItemFromQuery(SqlRow(q), state_ptr);
} }
return playlistitems;
return rows;
} }
QFuture<PlaylistItemPtr> PlaylistBackend::GetPlaylistItems(int playlist) { QList<Song> PlaylistBackend::GetPlaylistSongs(int playlist) {
QMutexLocker l(db_->Mutex()); QSqlQuery q = GetPlaylistRows(playlist);
QList<SqlRow> rows = GetPlaylistRows(playlist); // Note that as this only accesses the query, not the db, we don't need the
// mutex.
if (db_->CheckErrors(q)) return QList<Song>();
// it's probable that we'll have a few songs associated with the // it's probable that we'll have a few songs associated with the
// same CUE so we're caching results of parsing CUEs // same CUE so we're caching results of parsing CUEs
std::shared_ptr<NewSongFromQueryState> state_ptr(new NewSongFromQueryState()); std::shared_ptr<NewSongFromQueryState> state_ptr(new NewSongFromQueryState());
return QtConcurrent::mapped( QList<Song> songs;
rows, std::bind(&PlaylistBackend::NewPlaylistItemFromQuery, this, _1, while (q.next()) {
state_ptr)); songs << NewSongFromQuery(SqlRow(q), state_ptr);
} }
return songs;
QFuture<Song> PlaylistBackend::GetPlaylistSongs(int playlist) {
QMutexLocker l(db_->Mutex());
QList<SqlRow> rows = GetPlaylistRows(playlist);
// it's probable that we'll have a few songs associated with the
// same CUE so we're caching results of parsing CUEs
std::shared_ptr<NewSongFromQueryState> state_ptr(new NewSongFromQueryState());
return QtConcurrent::mapped(
rows, std::bind(&PlaylistBackend::NewSongFromQuery, this, _1, state_ptr));
} }
PlaylistItemPtr PlaylistBackend::NewPlaylistItemFromQuery( PlaylistItemPtr PlaylistBackend::NewPlaylistItemFromQuery(

View File

@ -18,7 +18,6 @@
#ifndef PLAYLISTBACKEND_H #ifndef PLAYLISTBACKEND_H
#define PLAYLISTBACKEND_H #define PLAYLISTBACKEND_H
#include <QFuture>
#include <QHash> #include <QHash>
#include <QList> #include <QList>
#include <QMutex> #include <QMutex>
@ -53,7 +52,6 @@ class PlaylistBackend : public QObject {
QString special_type; QString special_type;
}; };
typedef QList<Playlist> PlaylistList; typedef QList<Playlist> PlaylistList;
typedef QFuture<PlaylistItemPtr> PlaylistItemFuture;
static const int kSongTableJoins; static const int kSongTableJoins;
@ -61,8 +59,9 @@ class PlaylistBackend : public QObject {
PlaylistList GetAllOpenPlaylists(); PlaylistList GetAllOpenPlaylists();
PlaylistList GetAllFavoritePlaylists(); PlaylistList GetAllFavoritePlaylists();
PlaylistBackend::Playlist GetPlaylist(int id); PlaylistBackend::Playlist GetPlaylist(int id);
PlaylistItemFuture GetPlaylistItems(int playlist);
QFuture<Song> GetPlaylistSongs(int playlist); QList<PlaylistItemPtr> GetPlaylistItems(int playlist);
QList<Song> GetPlaylistSongs(int playlist);
void SetPlaylistOrder(const QList<int>& ids); void SetPlaylistOrder(const QList<int>& ids);
void SetPlaylistUiPath(int id, const QString& path); void SetPlaylistUiPath(int id, const QString& path);
@ -87,7 +86,7 @@ class PlaylistBackend : public QObject {
QMutex mutex_; QMutex mutex_;
}; };
QList<SqlRow> GetPlaylistRows(int playlist); QSqlQuery GetPlaylistRows(int playlist);
Song NewSongFromQuery(const SqlRow& row, Song NewSongFromQuery(const SqlRow& row,
std::shared_ptr<NewSongFromQueryState> state); std::shared_ptr<NewSongFromQueryState> state);

View File

@ -35,6 +35,7 @@
#include <QFuture> #include <QFuture>
#include <QFutureWatcher> #include <QFutureWatcher>
#include <QMessageBox> #include <QMessageBox>
#include <QtConcurrentRun>
#include <QtDebug> #include <QtDebug>
using smart_playlists::GeneratorPtr; using smart_playlists::GeneratorPtr;
@ -182,21 +183,22 @@ void PlaylistManager::Save(int id, const QString& filename,
// Playlist is not in the playlist manager: probably save action was // Playlist is not in the playlist manager: probably save action was
// triggered // triggered
// from the left side bar and the playlist isn't loaded. // from the left side bar and the playlist isn't loaded.
QFuture<Song> future = playlist_backend_->GetPlaylistSongs(id); QFuture<QList<Song>> future = QtConcurrent::run(
QFutureWatcher<Song>* watcher = new QFutureWatcher<Song>(this); playlist_backend_, &PlaylistBackend::GetPlaylistSongs, id);
QFutureWatcher<SongList>* watcher = new QFutureWatcher<SongList>(this);
watcher->setFuture(future); watcher->setFuture(future);
NewClosure(watcher, SIGNAL(finished()), this, NewClosure(watcher, SIGNAL(finished()), this,
SLOT(ItemsLoadedForSavePlaylist(QFutureWatcher<Song>*, QString, SLOT(ItemsLoadedForSavePlaylist(QFutureWatcher<SongList>*,
Playlist::Path)), QString, Playlist::Path)),
watcher, filename); watcher, filename);
} }
} }
void PlaylistManager::ItemsLoadedForSavePlaylist(QFutureWatcher<Song>* watcher, void PlaylistManager::ItemsLoadedForSavePlaylist(
const QString& filename, QFutureWatcher<SongList>* watcher, const QString& filename,
Playlist::Path path_type) { Playlist::Path path_type) {
SongList song_list = watcher->future().results(); SongList song_list = watcher->future().result();
parser_->Save(song_list, filename, path_type); parser_->Save(song_list, filename, path_type);
} }

View File

@ -232,7 +232,7 @@ class PlaylistManager : public PlaylistManagerInterface {
void OneOfPlaylistsChanged(); void OneOfPlaylistsChanged();
void UpdateSummaryText(); void UpdateSummaryText();
void SongsDiscovered(const SongList& songs); void SongsDiscovered(const SongList& songs);
void ItemsLoadedForSavePlaylist(QFutureWatcher<Song>* watcher, void ItemsLoadedForSavePlaylist(QFutureWatcher<SongList>* watcher,
const QString& filename, const QString& filename,
Playlist::Path path_type); Playlist::Path path_type);