[PictureLoader] Pace requests and run the throttle timers on the worker thread

Previously the whole backed-up queue was drained in a burst as soon as a
request was enqueued, sending up to 10 requests back-to-back and then
immediately re-filling the quota one second later. That hard-bursts a
rate-limited API like Scryfall's (10 requests/second) into a 30 second
lockout.

Introduce a pacing timer that dispatches a single queue entry every
100 ms, so the per-second allowance is used smoothly instead of in spikes,
and keep the quota timer at 1 second. Also fix both timers' thread
affinity: they are QTimer value members and so are not QObject children,
meaning moveToThread() on the worker left them on the main thread while
the slot code started them from the picture thread, which was a no-op that
also warned. They are moved to the worker thread explicitly and started
lazily from there.
This commit is contained in:
Lukas Brübach 2026-09-06 04:51:07 +02:00
parent 61035f8ae0
commit 64b3b7e0b4
2 changed files with 39 additions and 5 deletions

View file

@ -17,8 +17,10 @@
#include <version_string.h>
static constexpr int MAX_REQUESTS_PER_SEC = 10;
static constexpr int MIN_HOST_QUOTA = 1; ///< Floor for the per-host request allowance
static constexpr qint64 QUOTA_RECOVER_MS = 60000; ///< Idle time before a reduced quota starts recovering
static constexpr int MIN_HOST_QUOTA = 1; ///< Floor for the per-host request allowance
static constexpr qint64 QUOTA_RECOVER_MS = 60000; ///< Idle time before a reduced quota starts recovering
static constexpr int DISPATCH_INTERVAL_MS = 100; ///< Pacing between individual network requests
static constexpr qint64 QUOTA_RESET_INTERVAL_MS = 1000; ///< Interval at which the request quota resets
CardPictureLoaderWorker::CardPictureLoaderWorker()
: QObject(nullptr), picDownload(SettingsCache::instance().downloads().getPicDownload()),
@ -60,11 +62,18 @@ CardPictureLoaderWorker::CardPictureLoaderWorker()
pictureLoaderThread->start(QThread::LowPriority);
moveToThread(pictureLoaderThread);
// QTimer value members are not QObject children, so moveToThread on the worker doesn't move
// them. They must live in the worker's thread to be started from the slot code that runs there.
requestTimer.moveToThread(pictureLoaderThread);
dispatchTimer.moveToThread(pictureLoaderThread);
connect(this, &CardPictureLoaderWorker::imageLoadEnqueued, this, &CardPictureLoaderWorker::handleImageLoadEnqueued);
connect(&requestTimer, &QTimer::timeout, this, &CardPictureLoaderWorker::resetRequestQuota);
requestTimer.setInterval(1000);
requestTimer.start();
requestTimer.setInterval(static_cast<int>(QUOTA_RESET_INTERVAL_MS));
connect(&dispatchTimer, &QTimer::timeout, this, &CardPictureLoaderWorker::dispatchQueuedRequest);
dispatchTimer.setInterval(DISPATCH_INTERVAL_MS);
}
CardPictureLoaderWorker::~CardPictureLoaderWorker()
@ -147,8 +156,29 @@ void CardPictureLoaderWorker::resetRequestQuota()
void CardPictureLoaderWorker::processQueuedRequests()
{
while (requestQuota > 0 && processSingleRequest()) {
if (requestLoadQueue.isEmpty()) {
dispatchTimer.stop();
return;
}
// Start lazily from the worker's own thread: QTimer must be started in the thread it lives in.
if (!requestTimer.isActive()) {
requestTimer.start();
}
dispatchTimer.start();
}
void CardPictureLoaderWorker::dispatchQueuedRequest()
{
if (requestLoadQueue.isEmpty() || requestQuota <= 0) {
dispatchTimer.stop();
return;
}
if (processSingleRequest()) {
--requestQuota;
} else {
// No queued host currently has allowance left in this second; wait for the quota reset.
dispatchTimer.stop();
}
}

View file

@ -89,6 +89,9 @@ public slots:
/** @brief Processes all queued requests respecting the request quota. */
void processQueuedRequests();
/** @brief Chooses a request from the queue and starts it, respecting the quota and pacing. */
void dispatchQueuedRequest();
/**
* @brief Processes a single queued request.
* @return true if a request was processed, false if queue is empty.
@ -120,6 +123,7 @@ private:
int requestQuota; ///< Remaining requests allowed per second
QTimer requestTimer; ///< Timer to reset the request quota
QTimer dispatchTimer; ///< Timer pacing individual network requests
QHash<QString, int> hostRequestQuota; ///< Sustained per-host request allowance
QHash<QString, int> hostQuotaRemaining; ///< Per-host allowance left in the current second
QHash<QString, QDateTime> hostLast429; ///< When each host was last rate limited