From 56260ed72785a772b1b4b018d5eff23e3b262ca4 Mon Sep 17 00:00:00 2001 From: emeric Date: Mon, 17 Jun 2019 13:43:29 +0200 Subject: [PATCH] Fixed regression on lookup performance (introduced when added track/artist links) --- src/api/subsonic/SubsonicResource.cpp | 2 +- src/database/Artist.cpp | 12 +++++ src/database/Artist.hpp | 1 + src/database/DatabaseHandler.cpp | 16 +++++-- test/database/DatabaseTest.cpp | 64 +++++++++++++++++++++++++++ 5 files changed, 90 insertions(+), 5 deletions(-) diff --git a/src/api/subsonic/SubsonicResource.cpp b/src/api/subsonic/SubsonicResource.cpp index 6ce41398..ad9a9511 100644 --- a/src/api/subsonic/SubsonicResource.cpp +++ b/src/api/subsonic/SubsonicResource.cpp @@ -617,7 +617,7 @@ artistToResponseNode(const Database::User::pointer& user, const Database::Artist artistNode.setAttribute("name", artist->getName()); if (id3) - artistNode.setAttribute("albumCount", std::to_string(artist->getReleases().size())); + artistNode.setAttribute("albumCount", std::to_string(artist->getReleaseCount())); if (user->hasStarredArtist(artist)) artistNode.setAttribute("starred", reportedStarredDate); diff --git a/src/database/Artist.cpp b/src/database/Artist.cpp index 2bc7d7ad..828cb37f 100644 --- a/src/database/Artist.cpp +++ b/src/database/Artist.cpp @@ -213,6 +213,18 @@ Artist::getReleases(const std::set& clusterIds) const return std::vector>(res.begin(), res.end()); } +std::size_t +Artist::getReleaseCount() const +{ + assert(self()); + assert(IdIsValid(self()->id())); + assert(session()); + + int res = session()->query("SELECT COUNT(DISTINCT r.id) FROM release r INNER JOIN artist a ON a.id = t_a_l.artist_id INNER JOIN track_artist_link t_a_l ON t_a_l.track_id = t.id INNER JOIN track t ON t.release_id = r.id") + .where("a.id = ?").bind(self()->id()); + return res; +} + std::vector> Artist::getTracks(boost::optional linkType) const { diff --git a/src/database/Artist.hpp b/src/database/Artist.hpp index 88eeed68..92934cd5 100644 --- a/src/database/Artist.hpp +++ b/src/database/Artist.hpp @@ -70,6 +70,7 @@ class Artist : public Wt::Dbo::Dbo const std::string& getMBID(void) const { return _MBID; } std::vector> getReleases(const std::set& clusterIds = std::set()) const; + std::size_t getReleaseCount() const; std::vector> getTracks(boost::optional linkType = {}) const; std::vector> getTracksWithRelease(boost::optional linkType = {}) const; std::vector> getRandomTracks(boost::optional count) const; diff --git a/src/database/DatabaseHandler.cpp b/src/database/DatabaseHandler.cpp index d82ae2d9..62beaf5c 100644 --- a/src/database/DatabaseHandler.cpp +++ b/src/database/DatabaseHandler.cpp @@ -230,15 +230,23 @@ Handler::Handler(Wt::Dbo::SqlConnectionPool& connectionPool) Wt::Dbo::Transaction transaction {_session}; // Indexes - _session.execute("CREATE INDEX IF NOT EXISTS track_path_idx ON track(file_path)"); - _session.execute("CREATE INDEX IF NOT EXISTS track_name_idx ON track(name)"); _session.execute("CREATE INDEX IF NOT EXISTS artist_name_idx ON artist(name)"); - _session.execute("CREATE INDEX IF NOT EXISTS release_name_idx ON release(name)"); - _session.execute("CREATE INDEX IF NOT EXISTS track_release_idx ON track(release_id)"); + _session.execute("CREATE INDEX IF NOT EXISTS artist_mbid_idx ON artist(mbid)"); _session.execute("CREATE INDEX IF NOT EXISTS cluster_name_idx ON cluster(name)"); _session.execute("CREATE INDEX IF NOT EXISTS cluster_type_name_idx ON cluster_type(name)"); + _session.execute("CREATE INDEX IF NOT EXISTS release_name_idx ON release(name)"); + _session.execute("CREATE INDEX IF NOT EXISTS release_mbid_idx ON release(mbid)"); + _session.execute("CREATE INDEX IF NOT EXISTS track_path_idx ON track(file_path)"); + _session.execute("CREATE INDEX IF NOT EXISTS track_name_idx ON track(name)"); + _session.execute("CREATE INDEX IF NOT EXISTS track_mbid_idx ON track(mbid)"); + _session.execute("CREATE INDEX IF NOT EXISTS track_release_idx ON track(release_id)"); _session.execute("CREATE INDEX IF NOT EXISTS tracklist_name_idx ON tracklist(name)"); + _session.execute("CREATE INDEX IF NOT EXISTS tracklist_user_idx ON tracklist(user_id)"); _session.execute("CREATE INDEX IF NOT EXISTS track_features_track_idx ON track_features(track_id)"); + _session.execute("CREATE INDEX IF NOT EXISTS track_artist_link_artist_idx ON track_artist_link(artist_id)"); + _session.execute("CREATE INDEX IF NOT EXISTS track_artist_link_name_idx ON track_artist_link(name)"); + _session.execute("CREATE INDEX IF NOT EXISTS track_artist_link_track_idx ON track_artist_link(track_id)"); + _session.execute("CREATE INDEX IF NOT EXISTS track_artist_link_type_idx ON track_artist_link(type)"); } _users = new UserDatabase(_session); diff --git a/test/database/DatabaseTest.cpp b/test/database/DatabaseTest.cpp index 3ec1989c..7f1eb425 100644 --- a/test/database/DatabaseTest.cpp +++ b/test/database/DatabaseTest.cpp @@ -237,6 +237,8 @@ testSingleTrackSingleArtist(Wt::Dbo::Session& session) auto artist {artists.front()}; CHECK(artist.id() == artistId); + CHECK(artist->getReleaseCount() == 0); + CHECK(track->getArtistLinks().size() == 1); auto artistLink {track->getArtistLinks().front()}; CHECK(artistLink->getTrack().id() == trackId); @@ -777,6 +779,65 @@ testMultiTracksSingleArtistMultiClusters(Wt::Dbo::Session& session) } } +static +void +testMultiTracksSingleArtistSingleRelease(Wt::Dbo::Session& session) +{ + const std::size_t nbTracks {10}; + IdType artistId {}; + IdType releaseId {}; + { + Wt::Dbo::Transaction transaction {session}; + + auto artist {Artist::create(session, "MyArtist")}; + auto release {Release::create(session, "MyRelease")}; + + for (std::size_t i {}; i < nbTracks; ++i) + { + auto track {Track::create(session, "MyTrackFile")}; + TrackArtistLink::create(session, track, artist, TrackArtistLink::Type::Artist); + track.modify()->setRelease(release); + } + + session.flush(); + artistId = artist.id(); + releaseId = release.id(); + } + + { + Wt::Dbo::Transaction transaction {session}; + CHECK(Release::getAllOrphans(session).empty()); + CHECK(Artist::getAllOrphans(session).empty()); + } + + { + Wt::Dbo::Transaction transaction {session}; + + auto artist {Artist::getById(session, artistId)}; + CHECK(artist); + CHECK(artist->getReleaseCount() == 1); + CHECK(artist->getReleases().size() == 1); + CHECK(artist->getReleases().front().id() == releaseId); + + auto release {Release::getById(session, releaseId)}; + CHECK(release); + CHECK(release->getTracks().size() == nbTracks); + } + + { + Wt::Dbo::Transaction transaction {session}; + + std::vector tracks {Track::getAll(session)}; + for (auto& track : tracks) + track.remove(); + + auto artist {Artist::getById(session, artistId)}; + auto release {Release::getById(session, releaseId)}; + artist.remove(); + release.remove(); + } +} + static void testSingleTrackSingleReleaseSingleArtist(Wt::Dbo::Session& session) @@ -809,6 +870,8 @@ testSingleTrackSingleReleaseSingleArtist(Wt::Dbo::Session& session) CHECK(releases.size() == 1); CHECK(releases.front().id() == releaseId); + CHECK(artist->getReleaseCount() == 1); + auto release {Release::getById(session, releaseId)}; CHECK(release); auto artists {release->getArtists()}; @@ -1187,6 +1250,7 @@ int main(int argc, char* argv[]) RUN_TEST(testSingleTrackSingleArtistMultiClusters); RUN_TEST(testSingleTrackSingleArtistMultiRolesMultiClusters); RUN_TEST(testMultiTracksSingleArtistMultiClusters); + RUN_TEST(testMultiTracksSingleArtistSingleRelease); RUN_TEST(testSingleTrackSingleReleaseSingleArtist);