From f51ceb933e5fccad968b19d7079e7c5a8eacbb1a Mon Sep 17 00:00:00 2001 From: emeric Date: Fri, 18 Jul 2025 19:29:08 +0200 Subject: [PATCH] Do not try to optimize when there are too few files, it seems to bring more drawbacks than benefits --- src/libs/database/impl/Session.cpp | 33 ++++------ .../database/include/database/Session.hpp | 4 +- src/libs/services/scanner/CMakeLists.txt | 1 + .../services/scanner/impl/ScannerService.cpp | 17 ++++- .../services/scanner/impl/ScannerService.hpp | 1 + .../impl/ScannerServiceTraceLogger.cpp | 63 +++++++++++++++++++ .../services/scanner/impl/ScannerStats.cpp | 4 +- .../steps/ScanStepAssociateArtistImages.cpp | 2 +- .../steps/ScanStepAssociateExternalLyrics.cpp | 2 +- .../steps/ScanStepAssociatePlayListTracks.cpp | 2 +- .../steps/ScanStepAssociateReleaseImages.cpp | 2 +- .../steps/ScanStepAssociateTrackImages.cpp | 2 +- .../steps/ScanStepCheckForRemovedFiles.cpp | 7 +-- .../steps/ScanStepComputeClusterStats.cpp | 5 +- .../scanner/impl/steps/ScanStepOptimize.cpp | 7 ++- .../include/services/scanner/ScannerStats.hpp | 4 +- src/lms/main.cpp | 6 -- src/lms/ui/LmsApplication.cpp | 2 +- src/lms/ui/admin/ScannerController.cpp | 2 +- src/lms/ui/resource/ArtworkResource.cpp | 2 +- 20 files changed, 116 insertions(+), 52 deletions(-) create mode 100644 src/libs/services/scanner/impl/ScannerServiceTraceLogger.cpp diff --git a/src/libs/database/impl/Session.cpp b/src/libs/database/impl/Session.cpp index fbb5194f..c946c947 100644 --- a/src/libs/database/impl/Session.cpp +++ b/src/libs/database/impl/Session.cpp @@ -346,26 +346,6 @@ namespace lms::db LMS_LOG(DB, INFO, "Vacuum complete!"); } - void Session::refreshTracingLoggerStats() - { - auto* traceLogger{ core::Service::get() }; - if (!traceLogger) - return; - - auto transaction{ createReadTransaction() }; - - traceLogger->setMetadata("db_artist_count", std::to_string(db::Artist::getCount(*this))); - traceLogger->setMetadata("db_cluster_count", std::to_string(db::Cluster::getCount(*this))); - traceLogger->setMetadata("db_cluster_type_count", std::to_string(db::ClusterType::getCount(*this))); - traceLogger->setMetadata("db_starred_artist_count", std::to_string(db::StarredArtist::getCount(*this))); - traceLogger->setMetadata("db_starred_release_count", std::to_string(db::StarredRelease::getCount(*this))); - traceLogger->setMetadata("db_starred_track_count", std::to_string(db::StarredTrack::getCount(*this))); - traceLogger->setMetadata("db_track_bookmark_count", std::to_string(db::TrackBookmark::getCount(*this))); - traceLogger->setMetadata("db_listen_count", std::to_string(db::Listen::getCount(*this))); - traceLogger->setMetadata("db_release_count", std::to_string(db::Release::getCount(*this))); - traceLogger->setMetadata("db_track_count", std::to_string(db::Track::getCount(*this))); - } - void Session::fullAnalyze() { LMS_SCOPED_TRACE_OVERVIEW("Database", "Analyze"); @@ -393,6 +373,19 @@ namespace lms::db }); } + std::size_t Session::getTotalFilesCount() + { + std::size_t res{}; + + res += db::Track::getCount(*this); + res += db::Image::getCount(*this); + res += db::TrackLyrics::getExternalLyricsCount(*this); + res += db::PlayListFile::getCount(*this); + res += db::ArtistInfo::getCount(*this); + + return res; + } + void Session::retrieveEntriesToAnalyze(std::vector& entryList) { auto transaction{ createReadTransaction() }; diff --git a/src/libs/database/include/database/Session.hpp b/src/libs/database/include/database/Session.hpp index c07f43c5..b7e45687 100644 --- a/src/libs/database/include/database/Session.hpp +++ b/src/libs/database/include/database/Session.hpp @@ -52,14 +52,14 @@ namespace lms::db void retrieveEntriesToAnalyze(std::vector& entryList); void analyzeEntry(const std::string& entry); - bool areAllTablesEmpty(); // need to acquire a read transaction + bool areAllTablesEmpty(); // need to acquire a read transaction + std::size_t getTotalFilesCount(); // need to acquire a read transaction void prepareTablesIfNeeded(); // need to run only once at startup bool migrateSchemaIfNeeded(); // returns true if migration was performed void createIndexesIfNeeded(); void vacuumIfNeeded(); void vacuum(); - void refreshTracingLoggerStats(); // returning a ptr here to ease further wrapping using operator-> Wt::Dbo::Session* getDboSession() { return &_session; } diff --git a/src/libs/services/scanner/CMakeLists.txt b/src/libs/services/scanner/CMakeLists.txt index 79122c0b..86cec564 100644 --- a/src/libs/services/scanner/CMakeLists.txt +++ b/src/libs/services/scanner/CMakeLists.txt @@ -27,6 +27,7 @@ add_library(lmsscanner STATIC impl/steps/ScanStepUpdateLibraryFields.cpp impl/FileScanners.cpp impl/ScannerService.cpp + impl/ScannerServiceTraceLogger.cpp impl/ScannerStats.cpp ) diff --git a/src/libs/services/scanner/impl/ScannerService.cpp b/src/libs/services/scanner/impl/ScannerService.cpp index 53d19a17..3cf55aa8 100644 --- a/src/libs/services/scanner/impl/ScannerService.cpp +++ b/src/libs/services/scanner/impl/ScannerService.cpp @@ -166,6 +166,20 @@ namespace lms::scanner LMS_LOG(DBUPDATER, INFO, "Using " << _jobScheduler->getThreadCount() << " thread(s) for jobs"); _jobScheduler->setShouldAbortCallback([this]() { return _abortScan; }); + std::size_t totalFilesCount{}; + { + auto& session{ _db.getTLSSession() }; + auto transaction{ session.createReadTransaction() }; + totalFilesCount = session.getTotalFilesCount(); + } + + // Force optimize in case scanner aborted during a large import, but do this only if there are enough elements in the database + // Otherwise, indexes may be not used and queries may be slower and slower while adding more and more elements in the db + LMS_LOG(DBUPDATER, INFO, "Scanned file count = " << totalFilesCount); + if (totalFilesCount >= 1'000) + _db.getTLSSession().fullAnalyze(); + + refreshTracingLoggerStats(); refreshScanSettings(); start(); @@ -371,7 +385,8 @@ namespace lms::scanner _currentScanStepStats.reset(); // must be sync with _curState } - LMS_LOG(DBUPDATER, INFO, "Scan " << (_abortScan ? "aborted" : "complete") << ". Changes = " << stats.nbChanges() << " (added = " << stats.additions << ", removed = " << stats.deletions << ", updated = " << stats.updates << ", failures = " << stats.failures << "), Not changed = " << stats.skips << ", Scanned = " << stats.scans << " (errors = " << stats.errorsCount << "), features fetched = " << stats.featuresFetched << ", duplicates = " << stats.duplicates.size()); + refreshTracingLoggerStats(); + LMS_LOG(DBUPDATER, INFO, "Scan " << (_abortScan ? "aborted" : "complete") << ". Changes = " << stats.getChangesCount() << " (added = " << stats.additions << ", removed = " << stats.deletions << ", updated = " << stats.updates << ", failures = " << stats.failures << "), Not changed = " << stats.skips << ", Scanned = " << stats.scans << " (errors = " << stats.errorsCount << "), features fetched = " << stats.featuresFetched << ", duplicates = " << stats.duplicates.size()); if (!_abortScan) { diff --git a/src/libs/services/scanner/impl/ScannerService.hpp b/src/libs/services/scanner/impl/ScannerService.hpp index 2ac1481f..88779ae9 100644 --- a/src/libs/services/scanner/impl/ScannerService.hpp +++ b/src/libs/services/scanner/impl/ScannerService.hpp @@ -82,6 +82,7 @@ namespace lms::scanner // Helpers void refreshScanSettings(); + void refreshTracingLoggerStats(); void notifyInProgressIfNeeded(const ScanStepStats& stats); void notifyInProgress(const ScanStepStats& stats); diff --git a/src/libs/services/scanner/impl/ScannerServiceTraceLogger.cpp b/src/libs/services/scanner/impl/ScannerServiceTraceLogger.cpp new file mode 100644 index 00000000..1c25f484 --- /dev/null +++ b/src/libs/services/scanner/impl/ScannerServiceTraceLogger.cpp @@ -0,0 +1,63 @@ +/* + * Copyright (C) 2013 Emeric Poupon + * + * This file is part of LMS. + * + * LMS is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * LMS 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 General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with LMS. If not, see . + */ + +#include "ScannerService.hpp" + +#include "database/objects/Artist.hpp" +#include "database/objects/ArtistInfo.hpp" +#include "database/objects/Cluster.hpp" +#include "database/objects/Image.hpp" +#include "database/objects/Listen.hpp" +#include "database/objects/StarredArtist.hpp" +#include "database/objects/StarredRelease.hpp" +#include "database/objects/StarredTrack.hpp" +#include "database/objects/Track.hpp" +#include "database/objects/TrackArtistLink.hpp" +#include "database/objects/TrackBookmark.hpp" +#include "database/objects/TrackEmbeddedImage.hpp" +#include "database/objects/TrackEmbeddedImageLink.hpp" + +namespace lms::scanner +{ + void ScannerService::refreshTracingLoggerStats() + { + auto* traceLogger{ core::Service::get() }; + if (!traceLogger) + return; + + auto& session{ _db.getTLSSession() }; + auto transaction{ session.createReadTransaction() }; + + traceLogger->setMetadata("db_artist_count", std::to_string(db::Artist::getCount(session))); + traceLogger->setMetadata("db_artist_info_count", std::to_string(db::ArtistInfo::getCount(session))); + traceLogger->setMetadata("db_cluster_count", std::to_string(db::Cluster::getCount(session))); + traceLogger->setMetadata("db_cluster_type_count", std::to_string(db::ClusterType::getCount(session))); + traceLogger->setMetadata("db_image_count", std::to_string(db::Image::getCount(session))); + traceLogger->setMetadata("db_listen_count", std::to_string(db::Listen::getCount(session))); + traceLogger->setMetadata("db_release_count", std::to_string(db::Release::getCount(session))); + traceLogger->setMetadata("db_starred_artist_count", std::to_string(db::StarredArtist::getCount(session))); + traceLogger->setMetadata("db_starred_release_count", std::to_string(db::StarredRelease::getCount(session))); + traceLogger->setMetadata("db_starred_track_count", std::to_string(db::StarredTrack::getCount(session))); + traceLogger->setMetadata("db_track_bookmark_count", std::to_string(db::TrackBookmark::getCount(session))); + traceLogger->setMetadata("db_track_count", std::to_string(db::Track::getCount(session))); + traceLogger->setMetadata("db_track_artist_link_count", std::to_string(db::TrackArtistLink::getCount(session))); + traceLogger->setMetadata("db_track_embedded_image_count", std::to_string(db::TrackEmbeddedImage::getCount(session))); + traceLogger->setMetadata("db_track_embedded_image_link_count", std::to_string(db::TrackEmbeddedImageLink::getCount(session))); + } +} // namespace lms::scanner \ No newline at end of file diff --git a/src/libs/services/scanner/impl/ScannerStats.cpp b/src/libs/services/scanner/impl/ScannerStats.cpp index 37887424..74f15f14 100644 --- a/src/libs/services/scanner/impl/ScannerStats.cpp +++ b/src/libs/services/scanner/impl/ScannerStats.cpp @@ -21,12 +21,12 @@ namespace lms::scanner { - std::size_t ScanStats::nbFiles() const + std::size_t ScanStats::getTotalFileCount() const { return skips + additions + updates + failures; } - std::size_t ScanStats::nbChanges() const + std::size_t ScanStats::getChangesCount() const { return additions + deletions + updates; } diff --git a/src/libs/services/scanner/impl/steps/ScanStepAssociateArtistImages.cpp b/src/libs/services/scanner/impl/steps/ScanStepAssociateArtistImages.cpp index c3200419..c8209f9a 100644 --- a/src/libs/services/scanner/impl/steps/ScanStepAssociateArtistImages.cpp +++ b/src/libs/services/scanner/impl/steps/ScanStepAssociateArtistImages.cpp @@ -321,7 +321,7 @@ namespace lms::scanner bool ScanStepAssociateArtistImages::needProcess(const ScanContext& context) const { - if (context.stats.nbChanges() > 0) + if (context.stats.getChangesCount() > 0) return true; if (getLastScanSettings() && getLastScanSettings()->artistImageFallbackToRelease != _settings.artistImageFallbackToRelease) diff --git a/src/libs/services/scanner/impl/steps/ScanStepAssociateExternalLyrics.cpp b/src/libs/services/scanner/impl/steps/ScanStepAssociateExternalLyrics.cpp index 9fb01f3a..e5038c31 100644 --- a/src/libs/services/scanner/impl/steps/ScanStepAssociateExternalLyrics.cpp +++ b/src/libs/services/scanner/impl/steps/ScanStepAssociateExternalLyrics.cpp @@ -145,7 +145,7 @@ namespace lms::scanner bool ScanStepAssociateExternalLyrics::needProcess(const ScanContext& context) const { - if (context.stats.nbChanges() > 0) + if (context.stats.getChangesCount() > 0) return true; return false; diff --git a/src/libs/services/scanner/impl/steps/ScanStepAssociatePlayListTracks.cpp b/src/libs/services/scanner/impl/steps/ScanStepAssociatePlayListTracks.cpp index 098f6bba..2e1a3cad 100644 --- a/src/libs/services/scanner/impl/steps/ScanStepAssociatePlayListTracks.cpp +++ b/src/libs/services/scanner/impl/steps/ScanStepAssociatePlayListTracks.cpp @@ -251,7 +251,7 @@ namespace lms::scanner bool ScanStepAssociatePlayListTracks::needProcess(const ScanContext& context) const { - if (context.stats.nbChanges() > 0) + if (context.stats.getChangesCount() > 0) return true; if (getLastScanSettings() && getLastScanSettings()->skipSingleReleasePlayLists != _settings.skipSingleReleasePlayLists) diff --git a/src/libs/services/scanner/impl/steps/ScanStepAssociateReleaseImages.cpp b/src/libs/services/scanner/impl/steps/ScanStepAssociateReleaseImages.cpp index f5a589a8..2488a139 100644 --- a/src/libs/services/scanner/impl/steps/ScanStepAssociateReleaseImages.cpp +++ b/src/libs/services/scanner/impl/steps/ScanStepAssociateReleaseImages.cpp @@ -266,7 +266,7 @@ namespace lms::scanner bool ScanStepAssociateReleaseImages::needProcess(const ScanContext& context) const { - return context.stats.nbChanges() > 0; + return context.stats.getChangesCount() > 0; } void ScanStepAssociateReleaseImages::process(ScanContext& context) diff --git a/src/libs/services/scanner/impl/steps/ScanStepAssociateTrackImages.cpp b/src/libs/services/scanner/impl/steps/ScanStepAssociateTrackImages.cpp index d7d7d456..e7871c3b 100644 --- a/src/libs/services/scanner/impl/steps/ScanStepAssociateTrackImages.cpp +++ b/src/libs/services/scanner/impl/steps/ScanStepAssociateTrackImages.cpp @@ -232,7 +232,7 @@ namespace lms::scanner bool ScanStepAssociateTrackImages::needProcess(const ScanContext& context) const { - return context.stats.nbChanges() > 0; + return context.stats.getChangesCount() > 0; } void ScanStepAssociateTrackImages::process(ScanContext& context) diff --git a/src/libs/services/scanner/impl/steps/ScanStepCheckForRemovedFiles.cpp b/src/libs/services/scanner/impl/steps/ScanStepCheckForRemovedFiles.cpp index 718b3c8b..0266249b 100644 --- a/src/libs/services/scanner/impl/steps/ScanStepCheckForRemovedFiles.cpp +++ b/src/libs/services/scanner/impl/steps/ScanStepCheckForRemovedFiles.cpp @@ -194,12 +194,7 @@ namespace lms::scanner { auto transaction{ session.createReadTransaction() }; - context.currentStepStats.totalElems = 0; - context.currentStepStats.totalElems += db::Track::getCount(session); - context.currentStepStats.totalElems += db::Image::getCount(session); - context.currentStepStats.totalElems += db::TrackLyrics::getExternalLyricsCount(session); - context.currentStepStats.totalElems += db::PlayListFile::getCount(session); - context.currentStepStats.totalElems += db::ArtistInfo::getCount(session); + context.currentStepStats.totalElems = session.getTotalFilesCount(); } LMS_LOG(DBUPDATER, DEBUG, context.currentStepStats.totalElems << " files to be checked..."); diff --git a/src/libs/services/scanner/impl/steps/ScanStepComputeClusterStats.cpp b/src/libs/services/scanner/impl/steps/ScanStepComputeClusterStats.cpp index 7d1dd110..3089aa8a 100644 --- a/src/libs/services/scanner/impl/steps/ScanStepComputeClusterStats.cpp +++ b/src/libs/services/scanner/impl/steps/ScanStepComputeClusterStats.cpp @@ -29,10 +29,7 @@ namespace lms::scanner { bool ScanStepComputeClusterStats::needProcess(const ScanContext& context) const { - if (context.stats.nbChanges() > 0) - return true; - - return false; + return context.stats.getChangesCount() > 0; } void ScanStepComputeClusterStats::process(ScanContext& context) diff --git a/src/libs/services/scanner/impl/steps/ScanStepOptimize.cpp b/src/libs/services/scanner/impl/steps/ScanStepOptimize.cpp index 98acc7ea..dfc87d65 100644 --- a/src/libs/services/scanner/impl/steps/ScanStepOptimize.cpp +++ b/src/libs/services/scanner/impl/steps/ScanStepOptimize.cpp @@ -32,8 +32,13 @@ namespace lms::scanner if (context.scanOptions.forceOptimize) return true; - if (context.stats.nbChanges() > (context.stats.nbFiles() / 10)) + // Don't optimize if there are too few files: it may lead to some indexes not being used + // and will drastically slow down the scan process when adding more files later + if (context.stats.getChangesCount() > (context.stats.getTotalFileCount() / 5) + && context.stats.getTotalFileCount() >= 1'000) + { return true; + } return false; } diff --git a/src/libs/services/scanner/include/services/scanner/ScannerStats.hpp b/src/libs/services/scanner/include/services/scanner/ScannerStats.hpp index 3ed335a0..1015cdbb 100644 --- a/src/libs/services/scanner/include/services/scanner/ScannerStats.hpp +++ b/src/libs/services/scanner/include/services/scanner/ScannerStats.hpp @@ -100,7 +100,7 @@ namespace lms::scanner std::size_t errorsCount{}; // maybe bigger than errors.size() if too many errors std::vector duplicates; - std::size_t nbFiles() const; - std::size_t nbChanges() const; + std::size_t getTotalFileCount() const; + std::size_t getChangesCount() const; }; } // namespace lms::scanner diff --git a/src/lms/main.cpp b/src/lms/main.cpp index 09befea6..2c1440d7 100644 --- a/src/lms/main.cpp +++ b/src/lms/main.cpp @@ -333,11 +333,6 @@ namespace lms session.vacuum(); else session.vacuumIfNeeded(); - - // force optimize in case scanner aborted during a large import: - // queries may be too slow to even be able to relaunch a scan using the web interface - session.fullAnalyze(); - database->getTLSSession().refreshTracingLoggerStats(); } ui::LmsApplicationManager appManager; @@ -385,7 +380,6 @@ namespace lms // Flush cover cache even if no changes: // covers may be external files that changed and we don't keep track of them for now (but we should) artworkService->flushCache(); - database->getTLSSession().refreshTracingLoggerStats(); }); core::Service feedbackService{ feedback::createFeedbackService(ioContext, *database) }; diff --git a/src/lms/ui/LmsApplication.cpp b/src/lms/ui/LmsApplication.cpp index e6f6034e..a1e3aa10 100644 --- a/src/lms/ui/LmsApplication.cpp +++ b/src/lms/ui/LmsApplication.cpp @@ -542,7 +542,7 @@ namespace lms::ui notifyMsg(Notification::Type::Info, Wt::WString::tr("Lms.Admin.Database.database"), Wt::WString::tr("Lms.Admin.Database.scan-complete") - .arg(static_cast(stats.nbFiles())) + .arg(static_cast(stats.getTotalFileCount())) .arg(static_cast(stats.additions)) .arg(static_cast(stats.updates)) .arg(static_cast(stats.deletions)) diff --git a/src/lms/ui/admin/ScannerController.cpp b/src/lms/ui/admin/ScannerController.cpp index 5931b8a9..fd45bb0b 100644 --- a/src/lms/ui/admin/ScannerController.cpp +++ b/src/lms/ui/admin/ScannerController.cpp @@ -115,7 +115,7 @@ namespace lms::ui if (status.lastCompleteScanStats) { _lastScanStatus->setText(Wt::WString::tr("Lms.Admin.ScannerController.last-scan-status") - .arg(status.lastCompleteScanStats->nbFiles()) + .arg(status.lastCompleteScanStats->getTotalFileCount()) .arg(durationToString(status.lastCompleteScanStats->startTime, status.lastCompleteScanStats->stopTime)) .arg(status.lastCompleteScanStats->stopTime.date().toString(Wt::WLocale::currentLocale().dateFormat())) .arg(status.lastCompleteScanStats->stopTime.time().toString(Wt::WLocale::currentLocale().timeFormat())) diff --git a/src/lms/ui/resource/ArtworkResource.cpp b/src/lms/ui/resource/ArtworkResource.cpp index cb7a5893..b160bf65 100644 --- a/src/lms/ui/resource/ArtworkResource.cpp +++ b/src/lms/ui/resource/ArtworkResource.cpp @@ -57,7 +57,7 @@ namespace lms::ui ArtworkResource::ArtworkResource() { LmsApp->getScannerEvents().scanComplete.connect(this, [this](const scanner::ScanStats& stats) { - if (stats.nbChanges()) + if (stats.getChangesCount()) setChanged(); }); }