Skip to content

Commit

Permalink
Bypass redundant cache lookup when using "Uncached" interface
Browse files Browse the repository at this point in the history
  • Loading branch information
@daschuer
daschuer committed Sep 22, 2023
1 parent 9ee880a commit d755cea
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 83 deletions.
100 changes: 44 additions & 56 deletions src/library/coverartcache.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ CoverArtCache::CoverArtCache() {
}

//static
void CoverArtCache::requestCover(
void CoverArtCache::requestCoverPrivate(
const QObject* pRequester,
const TrackPointer& pTrack,
const CoverInfo& coverInfo,
Expand All @@ -53,12 +53,16 @@ void CoverArtCache::requestCover(
VERIFY_OR_DEBUG_ASSERT(pCache) {
return;
}
QPixmap pixmap = CoverArtCache::getCachedCover(coverInfo, desiredWidth);
if (!pixmap.isNull()) {
emit pCache->coverFound(pRequester, coverInfo, pixmap);
return;
}
pCache->tryLoadCover(
pRequester,
pTrack,
coverInfo,
desiredWidth,
Loading::Default);
desiredWidth);
}

//static
Expand All @@ -69,7 +73,7 @@ void CoverArtCache::requestTrackCover(
VERIFY_OR_DEBUG_ASSERT(pTrack) {
return;
}
requestCover(
requestCoverPrivate(
pRequester,
pTrack,
pTrack->getCoverInfoWithLocation(),
Expand All @@ -80,7 +84,7 @@ void CoverArtCache::requestTrackCover(
QPixmap CoverArtCache::getCachedCover(
const CoverInfo& coverInfo,
int desiredWidth) {
if (coverInfo.type == CoverInfo::NONE) {
if (!coverInfo.hasImage()) {
return QPixmap();
}
const mixxx::cache_key_t requestedCacheKey = coverInfo.cacheKey();
Expand All @@ -104,8 +108,9 @@ QPixmap CoverArtCache::getCachedCover(
}

// static
void CoverArtCache::requestUncachedCover(
void CoverArtCache::requestUncachedCoverPrivate(
const QObject* pRequester,
const TrackPointer& pTrack,
const CoverInfo& coverInfo,
int desiredWidth) {
CoverArtCache* pCache = CoverArtCache::instance();
Expand All @@ -114,18 +119,31 @@ void CoverArtCache::requestUncachedCover(
}
pCache->tryLoadCover(
pRequester,
TrackPointer(),
pTrack,
coverInfo,
desiredWidth,
Loading::Default);
desiredWidth);
}

// static
void CoverArtCache::requestUncachedCover(
const QObject* pRequester,
const TrackPointer& pTrack,
int desiredWidth) {
VERIFY_OR_DEBUG_ASSERT(pTrack) {
return;
}
requestUncachedCoverPrivate(
pRequester,
pTrack,
pTrack->getCoverInfoWithLocation(),
desiredWidth);
}

QPixmap CoverArtCache::tryLoadCover(
void CoverArtCache::tryLoadCover(
const QObject* pRequester,
const TrackPointer& pTrack,
const CoverInfo& coverInfo,
int desiredWidth,
Loading loading) {
int desiredWidth) {
if (kLogger.traceEnabled()) {
kLogger.trace()
<< "requestCover"
Expand All @@ -136,20 +154,17 @@ QPixmap CoverArtCache::tryLoadCover(
DEBUG_ASSERT(!pTrack ||
pTrack->getLocation() == coverInfo.trackLocation);

const auto requestedCacheKey = coverInfo.cacheKey();

if (coverInfo.type == CoverInfo::NONE) {
if (loading == Loading::Default) {
emit coverFound(pRequester, coverInfo, QPixmap());
}
return QPixmap();
if (!coverInfo.hasImage()) {
emit coverFound(pRequester, coverInfo, QPixmap());
return;
}

const auto requestedCacheKey = coverInfo.cacheKey();
// keep a list of trackIds for which a future is currently running
// to avoid loading the same picture again while we are loading it
QPair<const QObject*, mixxx::cache_key_t> requestId = qMakePair(pRequester, requestedCacheKey);
if (m_runningRequests.contains(requestId)) {
return QPixmap();
return;
}

// If this request comes from CoverDelegate (table view), it'll want to get
Expand All @@ -159,27 +174,6 @@ QPixmap CoverArtCache::tryLoadCover(
// performance issues).
QString cacheKey = pixmapCacheKey(requestedCacheKey, desiredWidth);

QPixmap pixmap;
if (QPixmapCache::find(cacheKey, &pixmap)) {
if (kLogger.traceEnabled()) {
kLogger.trace()
<< "requestCover cache hit"
<< coverInfo
<< loading;
}
if (loading == Loading::Default) {
emit coverFound(pRequester, coverInfo, pixmap);
}
return pixmap;
}

if (loading == Loading::CachedOnly) {
if (kLogger.traceEnabled()) {
kLogger.trace() << "requestCover cache miss";
}
return QPixmap();
}

if (kLogger.traceEnabled()) {
kLogger.trace()
<< "requestCover starting future for"
Expand All @@ -193,37 +187,33 @@ QPixmap CoverArtCache::tryLoadCover(
pRequester,
pTrack,
coverInfo,
desiredWidth,
loading == Loading::Default);
desiredWidth);
connect(watcher,
&QFutureWatcher<FutureResult>::finished,
this,
&CoverArtCache::coverLoaded);
watcher->setFuture(future);
return QPixmap();
return;
}

//static
CoverArtCache::FutureResult CoverArtCache::loadCover(
const QObject* pRequester,
TrackPointer pTrack,
CoverInfo coverInfo,
int desiredWidth,
bool signalWhenDone) {
int desiredWidth) {
if (kLogger.traceEnabled()) {
kLogger.trace()
<< "loadCover"
<< coverInfo
<< desiredWidth
<< signalWhenDone;
<< desiredWidth;
}
DEBUG_ASSERT(!pTrack ||
pTrack->getLocation() == coverInfo.trackLocation);

auto res = FutureResult(
pRequester,
coverInfo.cacheKey(),
signalWhenDone);
coverInfo.cacheKey());

CoverInfo::LoadedImage loadedImage = coverInfo.loadImage(pTrack);
if (!loadedImage.image.isNull()) {
Expand Down Expand Up @@ -317,10 +307,8 @@ void CoverArtCache::coverLoaded() {

m_runningRequests.remove(qMakePair(res.pRequester, res.requestedCacheKey));

if (res.signalWhenDone) {
emit coverFound(
res.pRequester,
std::move(res.coverArt),
pixmap);
}
emit coverFound(
res.pRequester,
std::move(res.coverArt),
pixmap);
}
46 changes: 21 additions & 25 deletions src/library/coverartcache.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class CoverArtCache : public QObject, public Singleton<CoverArtCache> {
const QObject* pRequester,
const CoverInfo& coverInfo,
int desiredWidth = 0) {
requestCover(pRequester, TrackPointer(), coverInfo, desiredWidth);
requestCoverPrivate(pRequester, TrackPointer(), coverInfo, desiredWidth);
}

static void requestTrackCover(
Expand All @@ -32,33 +32,30 @@ class CoverArtCache : public QObject, public Singleton<CoverArtCache> {
static void requestUncachedCover(
const QObject* pRequester,
const CoverInfo& coverInfo,
int desiredWidth);
int desiredWidth) {
requestUncachedCoverPrivate(pRequester, TrackPointer(), coverInfo, desiredWidth);
}

enum class Loading {
CachedOnly,
NoSignal,
Default, // signal when done
};
static void requestUncachedCover(
const QObject* pRequester,
const TrackPointer& pTrack,
int desiredWidth = 0);

// Only public for testing
struct FutureResult {
FutureResult()
: pRequester(nullptr),
requestedCacheKey(CoverImageUtils::defaultCacheKey()),
signalWhenDone(false) {
requestedCacheKey(CoverImageUtils::defaultCacheKey()) {
}
FutureResult(
const QObject* pRequestorArg,
mixxx::cache_key_t requestedCacheKeyArg,
bool signalWhenDoneArg)
mixxx::cache_key_t requestedCacheKeyArg)
: pRequester(pRequestorArg),
requestedCacheKey(requestedCacheKeyArg),
signalWhenDone(signalWhenDoneArg) {
requestedCacheKey(requestedCacheKeyArg) {
}

const QObject* pRequester;
mixxx::cache_key_t requestedCacheKey;
bool signalWhenDone;

CoverArt coverArt;
};
Expand All @@ -68,8 +65,7 @@ class CoverArtCache : public QObject, public Singleton<CoverArtCache> {
const QObject* pRequester,
TrackPointer pTrack,
CoverInfo coverInfo,
int desiredWidth,
bool emitSignals);
int desiredWidth);

private slots:
// Called when loadCover is complete in the main thread.
Expand All @@ -87,23 +83,23 @@ class CoverArtCache : public QObject, public Singleton<CoverArtCache> {
friend class Singleton<CoverArtCache>;

private:
static void requestCover(
static void requestCoverPrivate(
const QObject* pRequester,
const TrackPointer& /*optional*/ pTrack,
const CoverInfo& coverInfo,
int desiredWidth = 0);

static void requestUncachedCoverPrivate(
const QObject* pRequester,
const TrackPointer& /*optional*/ pTrack,
const CoverInfo& coverInfo,
int desiredWidth = 0);

QPixmap tryLoadCover(
void tryLoadCover(
const QObject* pRequester,
const TrackPointer& pTrack,
const CoverInfo& info,
int desiredWidth,
Loading loading);
int desiredWidth);

QSet<QPair<const QObject*, mixxx::cache_key_t>> m_runningRequests;
};

inline
QDebug operator<<(QDebug dbg, CoverArtCache::Loading loading) {
return dbg << static_cast<int>(loading);
}
4 changes: 2 additions & 2 deletions src/test/coverartcache_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class CoverArtCacheTest : public LibraryTest, public CoverArtCache {
info.trackLocation = trackLocation;

CoverArtCache::FutureResult res;
res = CoverArtCache::loadCover(nullptr, TrackPointer(), info, 0, false);
res = CoverArtCache::loadCover(nullptr, TrackPointer(), info, 0);
EXPECT_EQ(img, res.coverArt.loadedImage.image);
EXPECT_EQ(CoverImageUtils::calculateDigest(img), res.coverArt.imageDigest());
EXPECT_TRUE(res.coverArt.coverLocation.isNull());
Expand All @@ -48,7 +48,7 @@ class CoverArtCacheTest : public LibraryTest, public CoverArtCache {
info.trackLocation = trackLocation;

CoverArtCache::FutureResult res;
res = CoverArtCache::loadCover(nullptr, TrackPointer(), info, 0, false);
res = CoverArtCache::loadCover(nullptr, TrackPointer(), info, 0);
EXPECT_EQ(img, res.coverArt.loadedImage.image);
EXPECT_EQ(CoverImageUtils::calculateDigest(img), res.coverArt.imageDigest());
EXPECT_QSTRING_EQ(info.coverLocation, res.coverArt.coverLocation);
Expand Down

0 comments on commit d755cea

Please sign in to comment.