Skip to content

Commit 2b4569d

Browse files
committed
refactor: remove mutex lock and use async processing for image provider
The image provider has been refactored to eliminate mutex locks and implement fully asynchronous processing. The main changes include: 1. Removed QMutex and QMutexLocker usage entirely, replacing thread synchronization with thread-safe Qt signals/slots 2. Restructured CacheImageResponse to be a pure response container without loading logic 3. Created ImageTask as a QRunnable that loads images asynchronously on QThreadPool::globalInstance() 4. Implemented a pending response tracking system using QMultiMap to handle multiple requests for the same image 5. Added proper thread affinity management with moveToThread() for responses 6. Simplified cache key generation with a dedicated makeCacheKey() method The refactoring addresses performance issues by removing contention on mutex locks during image loading. Multiple requests for the same image now share the same loading task instead of creating duplicate tasks. The response objects are properly managed with QPointer to handle potential deletion during asynchronous operations. Influence: 1. Test image loading with different sizes and formats 2. Verify concurrent requests for the same image don't create duplicate loading tasks 3. Test image provider with invalid image paths to ensure proper error handling 4. Verify memory management and proper cleanup of response objects 5. Test performance under high load with multiple simultaneous image requests 6. Ensure thread safety without race conditions in the new async architecture refactor: 移除图标处理中的互斥锁,使用异步处理 图像提供器已重构,消除了互斥锁并实现了完全异步处理。主要变更包括: 1. 完全移除了QMutex和QMutexLocker的使用,用线程安全的Qt信号/槽替代线程 同步 2. 重构CacheImageResponse为纯响应容器,不包含加载逻辑 3. 创建ImageTask作为QRunnable,在QThreadPool::globalInstance()上异步加载 图像 4. 实现了使用QMultiMap的待处理响应跟踪系统,处理同一图像的多个请求 5. 通过moveToThread()为响应添加了适当的线程亲和性管理 6. 使用专用的makeCacheKey()方法简化了缓存键生成 此次重构解决了图像加载期间互斥锁争用导致的性能问题。现在对同一图像的多个 请求共享相同的加载任务,而不是创建重复任务。响应对象通过QPointer进行适当 管理,以处理异步操作期间可能的删除操作。 Influence: 1. 测试不同尺寸和格式的图像加载 2. 验证对同一图像的并发请求不会创建重复的加载任务 3. 测试图像提供器使用无效图像路径时的错误处理 4. 验证内存管理和响应对象的正确清理 5. 测试高负载下多个同时图像请求的性能 6. 确保新异步架构中的线程安全,避免竞态条件
1 parent 05be706 commit 2b4569d

2 files changed

Lines changed: 154 additions & 116 deletions

File tree

src/dde-control-center/plugin/dccimageprovider.cpp

Lines changed: 140 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -3,169 +3,203 @@
33
// SPDX-License-Identifier: GPL-3.0-or-later
44
#include "dccimageprovider.h"
55

6-
#include <QMutexLocker>
76
#include <QMetaObject>
7+
#include <QThread>
8+
#include <QThreadPool>
9+
#include <QUrl>
810

911
namespace dccV25 {
1012
// 4x of 84x54 => 336x216, used as a high-res base thumbnail size
1113
const QSize THUMBNAIL_ICON_SIZE(336, 216);
1214
// Separator for cache key to include size information
1315
const QString CACHE_KEY_SIZE_SEPARATOR = "::SIZE::";
1416

17+
static QSize resolveSize(const QSize &size)
18+
{
19+
return (size.isValid() && size.width() > 0 && size.height() > 0)
20+
? size
21+
: THUMBNAIL_ICON_SIZE;
22+
}
23+
24+
// ---------------------------------------------------------------------------
25+
// CacheImageResponse – pure QML response container, no loading logic.
26+
// ---------------------------------------------------------------------------
1527
class CacheImageResponse : public QQuickImageResponse
1628
{
29+
Q_OBJECT
1730
public:
18-
CacheImageResponse(const QString &id, const QSize &requestedSize, DccImageProvider *provider)
19-
: QQuickImageResponse()
20-
{
21-
QImage *img = provider->cacheImage(id, requestedSize, this, requestedSize);
22-
if (img) {
23-
// Image is already scaled to the proper size, no need to rescale here.
24-
setImage(*img, QSize());
25-
}
26-
}
31+
CacheImageResponse() = default;
2732

28-
QQuickTextureFactory *textureFactory() const override { return QQuickTextureFactory::textureFactoryForImage(m_image); }
33+
void setImage(const QImage &image) { m_image = image; }
34+
void emitFinished() { Q_EMIT finished(); }
2935

30-
void setImage(const QImage &image, const QSize &requestedSize)
36+
QQuickTextureFactory *textureFactory() const override
3137
{
32-
QMetaObject::invokeMethod(this, [this, image, requestedSize]() {
33-
m_image = requestedSize.isValid() ? image.scaled(requestedSize) : image;
34-
Q_EMIT finished();
35-
}, Qt::QueuedConnection);
38+
return QQuickTextureFactory::textureFactoryForImage(m_image);
3639
}
3740

3841
private:
3942
QImage m_image;
4043
};
4144

42-
class ImageTask : public QRunnable
45+
// ---------------------------------------------------------------------------
46+
// ImageTask – async image loader, runs on thread pool.
47+
// ---------------------------------------------------------------------------
48+
class ImageTask : public QObject, public QRunnable
4349
{
50+
Q_OBJECT
51+
52+
Q_SIGNALS:
53+
void imageLoaded(const QString &cacheKey, const QImage &image);
54+
4455
public:
45-
explicit ImageTask(const QString &cacheKey, const QSize &thumbnailSize, DccImageProvider *provider, CacheImageResponse *response, const QSize &requestedSize)
46-
: QRunnable()
47-
, m_cacheKey(cacheKey)
48-
, m_size(thumbnailSize.isValid() ? thumbnailSize : THUMBNAIL_ICON_SIZE)
56+
ImageTask(const QString &id, const QSize &requestedSize, const QString &cacheKey)
57+
: m_id(id)
4958
, m_requestedSize(requestedSize)
50-
, m_provider(provider)
51-
, m_response(response)
59+
, m_cacheKey(cacheKey)
5260
{
53-
// Extract original image path from cache key
54-
int sepIndex = m_cacheKey.indexOf(CACHE_KEY_SIZE_SEPARATOR);
55-
if (sepIndex > 0) {
56-
m_id = m_cacheKey.left(sepIndex);
57-
} else {
58-
m_id = m_cacheKey;
59-
}
60-
}
61-
62-
protected:
63-
void run() override;
64-
65-
protected:
66-
QString m_id;
67-
QString m_cacheKey; // Cache key including size information
68-
QSize m_size;
69-
QSize m_requestedSize;
70-
QPointer<DccImageProvider> m_provider;
71-
QPointer<CacheImageResponse> m_response;
72-
};
73-
74-
void ImageTask::run()
75-
{
76-
QImage originalImage;
77-
QUrl url(m_id);
78-
QString scheme = url.scheme().toLower();
79-
80-
QString path;
81-
if (scheme == "qrc") {
82-
path = ":" + url.path();
83-
}
84-
else if (scheme == "file" || url.isLocalFile()) {
85-
path = url.toLocalFile();
86-
}
87-
else {
88-
path = url.toString();
61+
setAutoDelete(false);
8962
}
9063

91-
if (originalImage.load(path)) {
92-
QSize targetSize = m_requestedSize.isValid() ? m_requestedSize : m_size;
93-
if (!targetSize.isValid()) {
94-
targetSize = THUMBNAIL_ICON_SIZE;
64+
void run() override
65+
{
66+
QUrl url(m_id);
67+
const QString scheme = url.scheme().toLower();
68+
QString path;
69+
if (scheme == "qrc")
70+
path = ":" + url.path();
71+
else if (scheme == "file" || url.isLocalFile())
72+
path = url.toLocalFile();
73+
else
74+
path = url.toString();
75+
76+
QImage img;
77+
if (!img.load(path)) {
78+
Q_EMIT imageLoaded(m_cacheKey, QImage());
79+
return;
9580
}
9681

97-
QImage img = originalImage.scaled(targetSize,
98-
Qt::KeepAspectRatioByExpanding,
99-
Qt::SmoothTransformation);
100-
if (img.width() > targetSize.width() || img.height() > targetSize.height()) {
101-
const QRect r(QPoint(0, 0), targetSize);
102-
const QRect srcRect(img.rect().center() - r.center(), targetSize);
103-
img = img.copy(srcRect);
82+
img = img.scaled(m_requestedSize, Qt::KeepAspectRatioByExpanding, Qt::SmoothTransformation);
83+
if (img.width() > m_requestedSize.width() || img.height() > m_requestedSize.height()) {
84+
const QRect dest(QPoint(0, 0), m_requestedSize);
85+
const QRect src(img.rect().center() - dest.center(), m_requestedSize);
86+
img = img.copy(src);
10487
}
10588

106-
if (m_provider && m_response) {
107-
QImage *heapImage = new QImage(std::move(img));
108-
if (m_provider->insert(m_cacheKey, heapImage)) {
109-
// Image already at target size, no further scaling needed.
110-
m_response->setImage(*heapImage, QSize());
111-
} else {
112-
delete heapImage;
113-
}
114-
}
89+
Q_EMIT imageLoaded(m_cacheKey, img);
11590
}
116-
}
91+
92+
private:
93+
QString m_id;
94+
QSize m_requestedSize;
95+
QString m_cacheKey;
96+
};
97+
98+
// ---------------------------------------------------------------------------
99+
// DccImageProvider
100+
// ---------------------------------------------------------------------------
117101

118102
DccImageProvider::DccImageProvider()
119103
: QQuickAsyncImageProvider()
120104
, m_cache(80)
121-
, m_threadPool(new QThreadPool(this))
122105
{
123106
}
124107

125108
DccImageProvider::~DccImageProvider()
126109
{
127-
if (m_threadPool) {
128-
m_threadPool->waitForDone();
129-
}
130110
}
131111

132-
QImage *DccImageProvider::cacheImage(const QString &id, const QSize &thumbnailSize)
112+
// static
113+
QString DccImageProvider::makeCacheKey(const QString &id, const QSize &size)
133114
{
134-
return cacheImage(id, thumbnailSize, new CacheImageResponse(id, QSize(), this), QSize());
115+
const QSize s = resolveSize(size);
116+
return QString("%1%2%3x%4").arg(id, CACHE_KEY_SIZE_SEPARATOR).arg(s.width()).arg(s.height());
135117
}
136118

137-
QImage *DccImageProvider::cacheImage(const QString &id, const QSize &thumbnailSize, CacheImageResponse *response, const QSize &requestedSize)
119+
void DccImageProvider::submitTask(const QString &id, const QSize &resolvedSize,
120+
const QString &cacheKey, CacheImageResponse *response)
138121
{
139-
QMutexLocker<QMutex> locker(&m_mutex);
122+
Q_ASSERT(thread() == QThread::currentThread());
140123

141-
// Use size as part of cache key to avoid different size images interfering with each other
142-
QString cacheKey = id;
143-
144-
// Determine the size to use for cache key
145-
QSize cacheSize = requestedSize.isValid() ? requestedSize : thumbnailSize;
146-
if (!cacheSize.isValid() || cacheSize.width() <= 0 || cacheSize.height() <= 0) {
147-
cacheSize = THUMBNAIL_ICON_SIZE;
124+
if (m_pendingResponses.contains(cacheKey)) {
125+
// Task already in flight – just register this response for notification
126+
if (response)
127+
m_pendingResponses.insert(cacheKey, response);
128+
return;
148129
}
149130

150-
// Add size to cache key
151-
cacheKey = QString("%1%2%3x%4").arg(id).arg(CACHE_KEY_SIZE_SEPARATOR).arg(cacheSize.width()).arg(cacheSize.height());
131+
// Insert the response pointer, or an empty QPointer as an in-flight marker.
132+
m_pendingResponses.insert(cacheKey, QPointer<CacheImageResponse>(response));
152133

153-
if (m_cache.contains(cacheKey)) {
154-
return m_cache.object(cacheKey);
155-
}
156-
m_threadPool->start(new ImageTask(cacheKey, thumbnailSize, this, response, requestedSize));
157-
return nullptr;
134+
auto task = new ImageTask(id, resolvedSize, cacheKey);
135+
connect(task, &ImageTask::imageLoaded, this, &DccImageProvider::onImageLoaded, Qt::QueuedConnection);
136+
connect(task, &ImageTask::imageLoaded, task, &QObject::deleteLater, Qt::QueuedConnection);
137+
QThreadPool::globalInstance()->start(task);
138+
}
139+
140+
void DccImageProvider::cacheImage(const QString &id, const QSize &thumbnailSize)
141+
{
142+
const QString cacheKey = makeCacheKey(id, thumbnailSize);
143+
const QSize resolved = resolveSize(thumbnailSize);
144+
145+
const auto work = [this, id, resolved, cacheKey]() {
146+
if (m_cache.object(cacheKey))
147+
return;
148+
submitTask(id, resolved, cacheKey, nullptr);
149+
};
150+
151+
QMetaObject::invokeMethod(this, work);
158152
}
159153

160154
QQuickImageResponse *DccImageProvider::requestImageResponse(const QString &id, const QSize &requestedSize)
161155
{
162-
return new CacheImageResponse(id, requestedSize, this);
156+
const QString cacheKey = makeCacheKey(id, requestedSize);
157+
const QSize resolved = resolveSize(requestedSize);
158+
auto response = new CacheImageResponse();
159+
response->moveToThread(thread());
160+
QPointer<CacheImageResponse> guardedResponse(response);
161+
162+
const auto work = [this, guardedResponse, id, resolved, cacheKey]() {
163+
if (!guardedResponse)
164+
return;
165+
166+
if (const QImage *cached = m_cache.object(cacheKey)) {
167+
// Cache hit: set image and emit finished asynchronously on the provider thread.
168+
guardedResponse->setImage(*cached);
169+
guardedResponse->emitFinished();
170+
} else {
171+
// Cache miss: submit a loading task. The response will be notified when loading is done.
172+
submitTask(id, resolved, cacheKey, guardedResponse);
173+
}
174+
};
175+
176+
QMetaObject::invokeMethod(this, work);
177+
178+
return response;
163179
}
164180

165-
bool DccImageProvider::insert(const QString &id, QImage *img)
181+
void DccImageProvider::onImageLoaded(const QString &cacheKey, const QImage &image)
166182
{
167-
QMutexLocker<QMutex> locker(&m_mutex);
168-
return m_cache.insert(id, img);
183+
Q_ASSERT(thread() == QThread::currentThread());
184+
185+
if (!image.isNull())
186+
m_cache.insert(cacheKey, new QImage(image));
187+
188+
if (!m_pendingResponses.contains(cacheKey))
189+
return;
190+
191+
const auto responses = m_pendingResponses.values(cacheKey);
192+
m_pendingResponses.remove(cacheKey);
193+
194+
for (const auto &resp : responses) {
195+
if (resp) {
196+
if (!image.isNull())
197+
resp->setImage(image);
198+
resp->emitFinished();
199+
}
200+
}
169201
}
170202

171203
} // namespace dccV25
204+
205+
#include "dccimageprovider.moc"
Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
1-
// SPDX-FileCopyrightText: 2025 UnionTech Software Technology Co., Ltd.
1+
// SPDX-FileCopyrightText: 2025 - 2026 UnionTech Software Technology Co., Ltd.
22
//
33
// SPDX-License-Identifier: GPL-3.0-or-later
44
#ifndef DCCIMAGEPROVIDER_H
55
#define DCCIMAGEPROVIDER_H
66
#include <QCache>
7-
#include <QMutex>
7+
#include <QImage>
8+
#include <QMultiMap>
89
#include <QObject>
10+
#include <QPointer>
911
#include <QQuickAsyncImageProvider>
10-
#include <QThreadPool>
1112

1213
namespace dccV25 {
1314
class CacheImageResponse;
@@ -17,18 +18,21 @@ class DccImageProvider : public QQuickAsyncImageProvider
1718
Q_OBJECT
1819
public:
1920
explicit DccImageProvider();
20-
~DccImageProvider();
21-
22-
QImage *cacheImage(const QString &id, const QSize &thumbnailSize);
23-
QImage *cacheImage(const QString &id, const QSize &thumbnailSize, CacheImageResponse *response, const QSize &requestedSize);
24-
bool insert(const QString &id, QImage *img);
21+
~DccImageProvider() override;
2522

23+
void cacheImage(const QString &id, const QSize &thumbnailSize);
2624
QQuickImageResponse *requestImageResponse(const QString &id, const QSize &requestedSize) override;
2725

26+
private Q_SLOTS:
27+
void onImageLoaded(const QString &cacheKey, const QImage &image);
28+
2829
private:
30+
static QString makeCacheKey(const QString &id, const QSize &size);
31+
void submitTask(const QString &id, const QSize &resolvedSize, const QString &cacheKey,
32+
CacheImageResponse *response);
33+
2934
QCache<QString, QImage> m_cache;
30-
QThreadPool *m_threadPool;
31-
QMutex m_mutex;
35+
QMultiMap<QString, QPointer<CacheImageResponse>> m_pendingResponses;
3236
};
3337
} // namespace dccV25
3438
#endif // DCCIMAGEPROVIDER_H

0 commit comments

Comments
 (0)