From d02a7ca86bd1ced9765031fa7a0c7ba8efefd618 Mon Sep 17 00:00:00 2001 From: emeric Date: Fri, 21 Feb 2020 15:03:42 +0100 Subject: [PATCH] Made the recommendation engine more usable during initial scan/rescans --- src/libs/recommendation/impl/Engine.cpp | 135 ++++++++++++------ src/libs/recommendation/impl/Engine.hpp | 32 ++++- .../impl/clusters/ClustersClassifier.hpp | 2 +- .../impl/features/FeaturesClassifier.cpp | 56 +++++++- .../impl/features/FeaturesClassifier.hpp | 2 +- .../include/recommendation/IClassifier.hpp | 4 +- src/libs/scanner/impl/MediaScanner.cpp | 6 +- src/lms/ui/explore/ArtistView.cpp | 5 - 8 files changed, 177 insertions(+), 65 deletions(-) diff --git a/src/libs/recommendation/impl/Engine.cpp b/src/libs/recommendation/impl/Engine.cpp index 4b4a6b67..072c3d48 100644 --- a/src/libs/recommendation/impl/Engine.cpp +++ b/src/libs/recommendation/impl/Engine.cpp @@ -46,7 +46,7 @@ Engine::start() assert(!_running); _running = true; - requestReload(); + requestReloadInternal(false); _ioService.start(); } @@ -64,12 +64,18 @@ Engine::stop() void Engine::requestReload() +{ + requestReloadInternal(true); +} + +void +Engine::requestReloadInternal(bool databaseChanged) { LMS_LOG(RECOMMENDATION, DEBUG) << "Reload requested..."; - _ioService.post([&]() + _ioService.post([=]() { - reload(); + reload(databaseChanged); }); } @@ -80,9 +86,13 @@ Engine::getSimilarTracksFromTrackList(Database::Session& session, Database::IdTy std::vector res; - for (const auto& [priority, classifier] : _classifiers) + for (const auto& classifierName : _classifierPriorities) { - res = classifier->getSimilarTracksFromTrackList(session, trackListId, maxCount); + auto itClassifier {_classifiers.find(classifierName)}; + if (itClassifier == std::cend(_classifiers)) + continue; + + res = itClassifier->second->getSimilarTracksFromTrackList(session, trackListId, maxCount); if (!res.empty()) break; } @@ -97,11 +107,18 @@ Engine::getSimilarTracks(Database::Session& dbSession, const std::unordered_set< std::vector res; - for (const auto& [priority, classifier] : _classifiers) + for (const auto& classifierName : _classifierPriorities) { - res = classifier->getSimilarTracks(dbSession, trackIds, maxCount); + auto itClassifier {_classifiers.find(classifierName)}; + if (itClassifier == std::cend(_classifiers)) + continue; + + res = itClassifier->second->getSimilarTracks(dbSession, trackIds, maxCount); if (!res.empty()) + { + LMS_LOG(RECOMMENDATION, DEBUG) << "Got " << res.size() << " similar tracks using classifier '" << classifierName << "'"; break; + } } return res; @@ -114,11 +131,18 @@ Engine::getSimilarReleases(Database::Session& dbSession, Database::IdType releas std::vector res; - for (const auto& [priority, classifier] : _classifiers) + for (const auto& classifierName : _classifierPriorities) { - res = classifier->getSimilarReleases(dbSession, releaseId, maxCount); + auto itClassifier {_classifiers.find(classifierName)}; + if (itClassifier == std::cend(_classifiers)) + continue; + + res = itClassifier->second->getSimilarReleases(dbSession, releaseId, maxCount); if (!res.empty()) + { + LMS_LOG(RECOMMENDATION, DEBUG) << "Got " << res.size() << " similar releases using classifier '" << classifierName << "'"; break; + } } return res; @@ -131,18 +155,25 @@ Engine::getSimilarArtists(Database::Session& dbSession, Database::IdType artistI std::vector res; - for (const auto& [priority, classifier] : _classifiers) + for (const auto& classifierName : _classifierPriorities) { - res = classifier->getSimilarArtists(dbSession, artistId, maxCount); + auto itClassifier {_classifiers.find(classifierName)}; + if (itClassifier == std::cend(_classifiers)) + continue; + + res = itClassifier->second->getSimilarArtists(dbSession, artistId, maxCount); if (!res.empty()) + { + LMS_LOG(RECOMMENDATION, DEBUG) << "Got " << res.size() << " similar artists using classifier '" << classifierName << "'"; return res; + } } return res; } void -Engine::reload() +Engine::reload(bool databaseChanged) { using namespace Database; @@ -155,43 +186,29 @@ Engine::reload() return ScanSettings::get(_dbSession)->getRecommendationEngineType(); }()}; - std::map> newClassifiers; - - // TODO RAII this - auto addClassifier = [&](ClassifierPriority prio, std::unique_ptr classifier) - { - try - { - addPendingClassifier(*classifier.get()); - bool res {classifier->init(_dbSession)}; - removePendingClassifier(*classifier.get()); - - if (res) - newClassifiers.emplace(prio, std::move(classifier)); - - return res; - } - catch (LmsException& e) - { - removePendingClassifier(*classifier.get()); - throw; - } - }; + clearClassifiers(); switch (engineType) { case ScanSettings::RecommendationEngineType::Features: - addClassifier(0, createFeaturesClassifier()); // higher priority - [[fallthrough]]; + { + auto clustersClassifier {createClustersClassifier()}; + auto featuresClassifier {createFeaturesClassifier()}; + + setClassifierPriorities({featuresClassifier->getName(), clustersClassifier->getName()}); + + initAndAddClassifier(std::move(clustersClassifier), databaseChanged); // init first since faster + initAndAddClassifier(std::move(featuresClassifier), databaseChanged); + break; + } case ScanSettings::RecommendationEngineType::Clusters: - addClassifier(1, createClustersClassifier()); // lower priority - break; - } + auto clustersClassifier {createClustersClassifier()}; - { - std::unique_lock lock {_classifiersMutex}; - _classifiers.swap(newClassifiers); + setClassifierPriorities({clustersClassifier->getName()}); + + initAndAddClassifier(std::move(clustersClassifier), databaseChanged); + break; } LMS_LOG(RECOMMENDATION, INFO) << "Recommendation engines reloaded!"; @@ -199,6 +216,40 @@ Engine::reload() _sigReloaded.emit(); } +void +Engine::setClassifierPriorities(std::initializer_list classifierPriorities) +{ + std::unique_lock lock {_classifiersMutex}; + + _classifierPriorities.clear(); + std::transform(std::cbegin(classifierPriorities), std::cend(classifierPriorities), std::back_inserter(_classifierPriorities), [](std::string_view name) { return std::string {name}; }); +} + +void +Engine::clearClassifiers() +{ + std::unique_lock lock {_classifiersMutex}; + + _classifiers.clear(); +} + +void +Engine::initAndAddClassifier(std::unique_ptr classifier, bool databaseChanged) +{ + PendingClassifierHandler pendingClassifier {*this, *classifier.get()}; + + LMS_LOG(RECOMMENDATION, INFO) << "Initializing classifier '" << classifier->getName() << "'..."; + bool res {classifier->init(_dbSession, databaseChanged)}; + LMS_LOG(RECOMMENDATION, INFO) << "Initializing classifier '" << classifier->getName() << "': " << (res ? "SUCCESS" : "FAILURE"); + + if (res) + { + std::unique_lock lock {_classifiersMutex}; + + _classifiers.emplace(classifier->getName(), std::move(classifier)); + } +} + void Engine::cancelPendingClassifiers() { diff --git a/src/libs/recommendation/impl/Engine.hpp b/src/libs/recommendation/impl/Engine.hpp index 17208b77..4fab63f1 100644 --- a/src/libs/recommendation/impl/Engine.hpp +++ b/src/libs/recommendation/impl/Engine.hpp @@ -21,6 +21,7 @@ #include #include +#include #include @@ -37,8 +38,6 @@ namespace Recommendation private: - using ClassifierPriority = unsigned; - void start() override; void stop() override; @@ -51,7 +50,31 @@ namespace Recommendation std::vector getSimilarArtists(Database::Session& session, Database::IdType artistId, std::size_t maxCount) override; - void reload(); + void requestReloadInternal(bool databaseChanged); + void reload(bool databaseChanged); + + void setClassifierPriorities(std::initializer_list classifierNames); + void clearClassifiers(); + void initAndAddClassifier(std::unique_ptr classifier, bool databaseChanged); + + class PendingClassifierHandler + { + public: + PendingClassifierHandler(Engine& engine, IClassifier& classifier) : _engine {engine}, _classifier {classifier} + { + _engine.addPendingClassifier(_classifier); + } + + ~PendingClassifierHandler() + { + _engine.removePendingClassifier(_classifier); + } + + private: + Engine& _engine; + IClassifier& _classifier; + }; + void cancelPendingClassifiers(); void addPendingClassifier(IClassifier& classifier); void removePendingClassifier(IClassifier& classifier); @@ -62,7 +85,8 @@ namespace Recommendation Wt::Signal<> _sigReloaded; std::shared_mutex _classifiersMutex; - std::map> _classifiers; + std::map> _classifiers; + std::vector _classifierPriorities; // ordered by priority std::unordered_set _pendingClassifiers; }; diff --git a/src/libs/recommendation/impl/clusters/ClustersClassifier.hpp b/src/libs/recommendation/impl/clusters/ClustersClassifier.hpp index 469a6ee8..4619d5dc 100644 --- a/src/libs/recommendation/impl/clusters/ClustersClassifier.hpp +++ b/src/libs/recommendation/impl/clusters/ClustersClassifier.hpp @@ -38,7 +38,7 @@ namespace Recommendation std::string_view getName() const { return "Clusters"; } - bool init(Database::Session&) override {return true;} + bool init(Database::Session&, bool) override {return true;} void requestCancelInit() override {} std::vector getSimilarTracksFromTrackList(Database::Session& session, Database::IdType tracklistId, std::size_t maxCount) const override; diff --git a/src/libs/recommendation/impl/features/FeaturesClassifier.cpp b/src/libs/recommendation/impl/features/FeaturesClassifier.cpp index f4f4ad90..d3b13d5d 100644 --- a/src/libs/recommendation/impl/features/FeaturesClassifier.cpp +++ b/src/libs/recommendation/impl/features/FeaturesClassifier.cpp @@ -259,21 +259,57 @@ FeaturesClassifier::getSimilarTracksFromTrackList(Database::Session& session, Da } std::vector -FeaturesClassifier::getSimilarTracks(Database::Session&, const std::unordered_set& tracksIds, std::size_t maxCount) const +FeaturesClassifier::getSimilarTracks(Database::Session& session, const std::unordered_set& tracksIds, std::size_t maxCount) const { - return getSimilarObjects(tracksIds, _tracksMap, _trackPositions, maxCount); + std::vector similarTrackIds {getSimilarObjects(tracksIds, _tracksMap, _trackPositions, maxCount)}; + + if (!similarTrackIds.empty()) + { + // Report only existing ids + auto transaction {session.createSharedTransaction()}; + + similarTrackIds.erase(std::remove_if(std::begin(similarTrackIds), std::end(similarTrackIds), + [&](Database::IdType trackId) { return Database::Track::getById(session, trackId) == Database::Track::pointer {}; }), + std::cend(similarTrackIds)); + } + + return similarTrackIds; } std::vector -FeaturesClassifier::getSimilarReleases(Database::Session&, Database::IdType releaseId, std::size_t maxCount) const +FeaturesClassifier::getSimilarReleases(Database::Session& session, Database::IdType releaseId, std::size_t maxCount) const { - return getSimilarObjects({releaseId}, _releasesMap, _releasePositions, maxCount); + std::vector similarReleaseIds {getSimilarObjects({releaseId}, _releasesMap, _releasePositions, maxCount)}; + + if (!similarReleaseIds.empty()) + { + // Report only existing ids + auto transaction {session.createSharedTransaction()}; + + similarReleaseIds.erase(std::remove_if(std::begin(similarReleaseIds), std::end(similarReleaseIds), + [&](Database::IdType releaseId) { return Database::Release::getById(session, releaseId) == Database::Release::pointer {}; }), + std::cend(similarReleaseIds)); + } + + return similarReleaseIds; } std::vector -FeaturesClassifier::getSimilarArtists(Database::Session&, Database::IdType artistId, std::size_t maxCount) const +FeaturesClassifier::getSimilarArtists(Database::Session& session, Database::IdType artistId, std::size_t maxCount) const { - return getSimilarObjects({artistId}, _artistsMap, _artistPositions, maxCount); + std::vector similarArtistIds {getSimilarObjects({artistId}, _artistsMap, _artistPositions, maxCount)}; + + if (!similarArtistIds.empty()) + { + // Report only existing ids + auto transaction {session.createSharedTransaction()}; + + similarArtistIds.erase(std::remove_if(std::begin(similarArtistIds), std::end(similarArtistIds), + [&](Database::IdType artistId) { return Database::Artist::getById(session, artistId) == Database::Artist::pointer {}; }), + std::cend(similarArtistIds)); + } + + return similarArtistIds; } FeaturesClassifierCache @@ -283,8 +319,14 @@ FeaturesClassifier::toCache() const } bool -FeaturesClassifier::init(Database::Session& session) +FeaturesClassifier::init(Database::Session& session, bool databaseChanged) { + if (databaseChanged) + { + LMS_LOG(RECOMMENDATION, DEBUG) << "Database changed: invidating cache"; + FeaturesClassifierCache::invalidate(); + } + std::optional cache {FeaturesClassifierCache::read()}; if (cache) return initFromCache(session, *cache); diff --git a/src/libs/recommendation/impl/features/FeaturesClassifier.hpp b/src/libs/recommendation/impl/features/FeaturesClassifier.hpp index d4856a96..ecf3b947 100644 --- a/src/libs/recommendation/impl/features/FeaturesClassifier.hpp +++ b/src/libs/recommendation/impl/features/FeaturesClassifier.hpp @@ -57,7 +57,7 @@ class FeaturesClassifier : public IClassifier std::string_view getName() const { return "Features"; } - bool init(Database::Session& session) override; + bool init(Database::Session& session, bool databaseChanged) override; void requestCancelInit() override; std::vector getSimilarTracksFromTrackList(Database::Session& session, Database::IdType tracklistId, std::size_t maxCount) const override; diff --git a/src/libs/recommendation/include/recommendation/IClassifier.hpp b/src/libs/recommendation/include/recommendation/IClassifier.hpp index e1c01f2f..44fa47d5 100644 --- a/src/libs/recommendation/include/recommendation/IClassifier.hpp +++ b/src/libs/recommendation/include/recommendation/IClassifier.hpp @@ -19,7 +19,7 @@ #pragma once -#include +#include #include #include @@ -40,7 +40,7 @@ namespace Recommendation virtual std::string_view getName() const = 0; - virtual bool init(Database::Session& session) = 0; + virtual bool init(Database::Session& session, bool databaseChanged) = 0; virtual void requestCancelInit() = 0; virtual std::vector getSimilarTracksFromTrackList(Database::Session& session, Database::IdType tracklistId, std::size_t maxCount) const = 0; diff --git a/src/libs/scanner/impl/MediaScanner.cpp b/src/libs/scanner/impl/MediaScanner.cpp index 9de04c5b..e70b1c43 100644 --- a/src/libs/scanner/impl/MediaScanner.cpp +++ b/src/libs/scanner/impl/MediaScanner.cpp @@ -360,9 +360,7 @@ MediaScanner::countAllFiles(ScanStats& stats) if (!ec && isFileSupported(path, _fileExtensions)) { stats.filesToScan++; - - if (stats.filesToScan % 250 == 0) - notifyInProgressIfNeeded(stats); + notifyInProgressIfNeeded(stats); } return true; @@ -822,6 +820,8 @@ MediaScanner::removeMissingTracks(ScanStats& stats) stats.deletions++; } } + + notifyInProgressIfNeeded(stats); } } diff --git a/src/lms/ui/explore/ArtistView.cpp b/src/lms/ui/explore/ArtistView.cpp index 089f8714..9bcaa9de 100644 --- a/src/lms/ui/explore/ArtistView.cpp +++ b/src/lms/ui/explore/ArtistView.cpp @@ -140,11 +140,6 @@ Artist::refresh() entry->bindWidget("name", LmsApplication::createReleaseAnchor(release)); auto artists {release->getReleaseArtists()}; - LMS_LOG(UI, DEBUG) << "Found " << artists.size() << " release artists"; - - for (auto artist : artists) - LMS_LOG(UI, DEBUG) << "\tArtist = '" << artist->getName() << "'"; - if (artists.empty()) artists = release->getArtists();