[PictureLoader] Keep the download log from going quadratic (#7766)

* [PictureLoader] Keep the download log from going quadratic

A fast scrollbar sweep over the visual card database requests a picture for
every cell it paints, so one gesture can ask for thousands of downloads. The
status bar built a seven-widget log entry for each of them and answered every
completion with findChildren() over the whole, unbounded log, rebuilding
url.toString() per child: O(n) per completion, O(n^2) per burst.

Profiling a sweep on master put 47% of all cycles on the GUI thread, with
qt_qFindChildren_helper at 12.4% of the process (a quarter of that thread) and
QWidgetPrivate::reparentFocusChildren at 4.1%. LagMonitor reported stalls of
3-18 s, growing with the backlog.

Keep at most MAX_LOG_ENTRIES entries and index them by URL, so a completion
and the 1 Hz cleanup only visit the matching entries. The progress bar follows
the entries that remain: dropping a finished entry moves both counters back,
dropping a pending one only its maximum. The log is a diagnostic view, not a
queue - the downloads themselves are unaffected.

* fixup! [PictureLoader] Keep the download log from going quadratic

Make the URL index a QMultiHash and say why: a deferred retry re-queues the same URL while its first log entry is still pending, so one URL can back several live entries.

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>

cockatrice/src/interface/card_picture_loader/card_picture_loader_status_bar.cpp

@@ -11,6 +11,7 @@
 #include <QSharedPointer>
 #include <QTimer>
 #include <QUrl>
+#include <QtGlobal>
 
 class ExactCard;
 
@@ -38,32 +39,94 @@ void CardPictureLoaderStatusBar::cleanOldEntries()
     if (!loadLog || !loadLog->popup) {
         return;
     }
-    for (CardPictureLoaderRequestStatusDisplayWidget *statusDisplayWidget :
+
-         loadLog->popup->findChildren<CardPictureLoaderRequestStatusDisplayWidget *>()) {
+    const QDateTime now = QDateTime::currentDateTime();
+    // Removing an entry mutates the log, so walk a copy of it.
+    const QList<CardPictureLoaderRequestStatusDisplayWidget *> entries = logEntries;
+    for (CardPictureLoaderRequestStatusDisplayWidget *statusDisplayWidget : entries) {
         statusDisplayWidget->queryElapsedSeconds();
-        if (statusDisplayWidget->getFinished() &&
+        if (!statusDisplayWidget->getFinished()) {
-            QDateTime::fromString(statusDisplayWidget->getStartTime()).secsTo(QDateTime::currentDateTime()) > 10) {
+            continue;
-            loadLog->removeSettingsWidget(statusDisplayWidget);
+        }
-            progressBar->setMaximum(progressBar->maximum() - 1);
+        if (QDateTime::fromString(statusDisplayWidget->getStartTime()).secsTo(now) > LOG_ENTRY_LIFETIME_SECS) {
-            progressBar->setValue(progressBar->value() - 1);
+            removeLogEntry(statusDisplayWidget);
         }
     }
 }
 
 void CardPictureLoaderStatusBar::addQueuedImageLoad(const QUrl &url, const ExactCard &card, const QString &setName)
 {
-    loadLog->addSettingsWidget(new CardPictureLoaderRequestStatusDisplayWidget(loadLog, url, card, setName));
+    if (logEntries.size() >= MAX_LOG_ENTRIES) {
+        trimLog();
+    }
+
+    auto *entry = new CardPictureLoaderRequestStatusDisplayWidget(loadLog, url, card, setName);
+    loadLog->addSettingsWidget(entry);
+    logEntries.append(entry);
+    entriesByUrl.insert(entry->getUrl(), entry);
     progressBar->setMaximum(progressBar->maximum() + 1);
 }
 
 void CardPictureLoaderStatusBar::addSuccessfulImageLoad(const QUrl &url)
 {
-    progressBar->setValue(progressBar->value() + 1);
+    const QList<CardPictureLoaderRequestStatusDisplayWidget *> entries = entriesByUrl.values(url.toString());
-    for (CardPictureLoaderRequestStatusDisplayWidget *statusDisplayWidget :
+    if (entries.isEmpty()) {
-         loadLog->popup->findChildren<CardPictureLoaderRequestStatusDisplayWidget *>()) {
+        // The entry may already have been dropped from the log; the download
-        if (statusDisplayWidget->getUrl() == url.toString()) {
+        // itself still succeeded, there is just nothing left to mark.
-            statusDisplayWidget->queryElapsedSeconds();
+        return;
-            statusDisplayWidget->setFinished();
+    }
+
+    int finishedEntries = 0;
+    for (CardPictureLoaderRequestStatusDisplayWidget *statusDisplayWidget : entries) {
+        if (statusDisplayWidget->getFinished()) {
+            continue;
+        }
+        statusDisplayWidget->queryElapsedSeconds();
+        statusDisplayWidget->setFinished();
+        ++finishedEntries;
+    }
+
+    // The bar counts the entries currently in the log, so only entries that are
+    // still listed move it.
+    progressBar->setValue(progressBar->value() + finishedEntries);
+}
+
+void CardPictureLoaderStatusBar::trimLog()
+{
+    if (logEntries.isEmpty()) {
+        return;
+    }
+
+    // Prefer evicting an entry that has already finished; only fall back to an
+    // in-flight request when the whole log is still pending.
+    CardPictureLoaderRequestStatusDisplayWidget *toRemove = nullptr;
+    for (CardPictureLoaderRequestStatusDisplayWidget *entry : logEntries) {
+        if (entry->getFinished()) {
+            toRemove = entry;
+            break;
         }
     }
-}
+
+    removeLogEntry(toRemove != nullptr ? toRemove : logEntries.constFirst());
+}
+
+void CardPictureLoaderStatusBar::removeLogEntry(CardPictureLoaderRequestStatusDisplayWidget *entry)
+{
+    if (entry == nullptr) {
+        return;
+    }
+
+    logEntries.removeOne(entry);
+    entriesByUrl.remove(entry->getUrl(), entry);
+
+    // The progress bar's maximum counts the entries currently in the log and
+    // its value the finished ones, so dropping an entry moves both counters back.
+    // Move the value first: a maximum of 0 turns the bar into a busy indicator and
+    // resets its value.
+    if (entry->getFinished()) {
+        progressBar->setValue(qMax(0, progressBar->value() - 1));
+    }
+    progressBar->setMaximum(qMax(0, progressBar->maximum() - 1));
+
+    loadLog->removeSettingsWidget(entry);
+}

cockatrice/src/interface/card_picture_loader/card_picture_loader_status_bar.h

@@ -1,10 +1,13 @@
 #ifndef PICTURE_LOADER_STATUS_BAR_H
 #define PICTURE_LOADER_STATUS_BAR_H
 
+#include <QList>
+#include <QMultiHash>
 #include <QString>
 #include <QWidget>
 #include <qtmetamacros.h>
 
+class CardPictureLoaderRequestStatusDisplayWidget;
 class ExactCard;
 class QHBoxLayout;
 class QProgressBar;
@@ -21,11 +24,20 @@ class SettingsButtonWidget;
  * - Shows overall progress of image downloads.
  * - Maintains a log of individual requests via a popup (SettingsButtonWidget).
  * - Cleans up finished request entries automatically after a delay.
+ *
+ * The log is a diagnostic view, not a queue: it keeps at most MAX_LOG_ENTRIES
+ * entries and indexes them by URL, so a burst of requests stays O(1) per event
+ * instead of scanning every entry the popup holds.
  */
 class CardPictureLoaderStatusBar : public QWidget
 {
     Q_OBJECT
 public:
+    /** Entries kept in the log; older ones are evicted as new requests arrive. */
+    static constexpr int MAX_LOG_ENTRIES = 50;
+    /** Seconds a finished entry stays in the log before it is cleaned up. */
+    static constexpr int LOG_ENTRY_LIFETIME_SECS = 10;
+
     /**
      * @brief Constructs a status bar with progress bar and log button.
      * @param parent Parent widget
@@ -55,10 +67,28 @@ public slots:
     void cleanOldEntries();
 
 private:
+    /**
+     * @brief Evicts the oldest finished entry (or the oldest entry if none finished)
+     *        until the log fits within MAX_LOG_ENTRIES.
+     */
+    void trimLog();
+
+    /**
+     * @brief Removes one entry from the log, its URL index and the popup, and keeps
+     *        the progress bar counters in step with the entries that remain.
+     * @param entry Entry to remove
+     */
+    void removeLogEntry(CardPictureLoaderRequestStatusDisplayWidget *entry);
+
     QHBoxLayout *layout;           ///< Horizontal layout containing progress bar and log button
     QProgressBar *progressBar;     ///< Progress bar showing overall download progress
     SettingsButtonWidget *loadLog; ///< Popup log showing individual request statuses
     QTimer *cleaner;               ///< Timer for periodically cleaning old log entries
+    QList<CardPictureLoaderRequestStatusDisplayWidget *> logEntries; ///< Logged requests, oldest first
+    // A URL can back several live entries: a request parked for a deferred retry is re-queued while
+    // its first entry is still pending, so a completion must finish every entry for that URL.
+    QMultiHash<QString, CardPictureLoaderRequestStatusDisplayWidget *>
+        entriesByUrl; ///< Log entries indexed by their URL for O(1) completion lookup
 };
 
 #endif // PICTURE_LOADER_STATUS_BAR_H