Periodically free memory from unused images (#3915)

Co-authored-by: pajlada <rasmus.karlsson@pajlada.com>

CHANGELOG.md

@@ -28,6 +28,7 @@
 - Minor: Reduced GIF frame window from 30ms to 20ms, causing fewer frame skips in animated emotes. (#3886, #3907)
 - Minor: Warn when parsing an environment variable fails. (#3904)
 - Minor: Load missing messages from Recent Messages API upon reconnecting (#3878, #3932)
+- Minor: Reduced image memory usage when running Chatterino for a long time. (#3915)
 - Minor: Add settings to toggle BTTV/FFZ global/channel emotes (#3935)
 - Minor: Add AutoMod message flag filter. (#3938)
 - Bugfix: Fix crash that can occur when closing and quickly reopening a split, then running a command. (#3852)

src/controllers/moderationactions/ModerationAction.cpp

@@ -141,9 +141,11 @@ const boost::optional<ImagePtr> &ModerationAction::getImage() const
     if (this->imageToLoad_ != 0)
     {
         if (this->imageToLoad_ == 1)
-            this->image_ = Image::fromPixmap(getResources().buttons.ban);
+            this->image_ =
+                Image::fromResourcePixmap(getResources().buttons.ban);
         else if (this->imageToLoad_ == 2)
-            this->image_ = Image::fromPixmap(getResources().buttons.trashCan);
+            this->image_ =
+                Image::fromResourcePixmap(getResources().buttons.trashCan);
     }
 
     return this->image_;

src/messages/Image.cpp

@@ -6,7 +6,9 @@
 #include <QNetworkReply>
 #include <QNetworkRequest>
 #include <QTimer>
+#include <boost/functional/hash.hpp>
 #include <functional>
+#include <queue>
 #include <thread>
 
 #include "Application.hpp"
@@ -23,7 +25,10 @@
 #include "util/DebugCount.hpp"
 #include "util/PostToThread.hpp"
 
-#include <queue>
+// Duration between each check of every Image instance
+const auto IMAGE_POOL_CLEANUP_INTERVAL = std::chrono::minutes(1);
+// Duration since last usage of Image pixmap before expiration of frames
+const auto IMAGE_POOL_IMAGE_LIFETIME = std::chrono::minutes(10);
 
 namespace chatterino {
 namespace detail {
@@ -33,11 +38,15 @@ namespace detail {
         DebugCount::increase("images");
     }
 
-    Frames::Frames(const QVector<Frame<QPixmap>> &frames)
-        : items_(frames)
+    Frames::Frames(QVector<Frame<QPixmap>> &&frames)
+        : items_(std::move(frames))
     {
         assertInGuiThread();
         DebugCount::increase("images");
+        if (!this->empty())
+        {
+            DebugCount::increase("loaded images");
+        }
 
         if (this->animated())
         {
@@ -76,6 +85,10 @@ namespace detail {
     {
         assertInGuiThread();
         DebugCount::decrease("images");
+        if (!this->empty())
+        {
+            DebugCount::decrease("loaded images");
+        }
 
         if (this->animated())
         {
@@ -114,6 +127,25 @@ namespace detail {
         }
     }
 
+    void Frames::clear()
+    {
+        assertInGuiThread();
+        if (!this->empty())
+        {
+            DebugCount::decrease("loaded images");
+        }
+
+        this->items_.clear();
+        this->index_ = 0;
+        this->durationOffset_ = 0;
+        this->gifTimerConnection_.disconnect();
+    }
+
+    bool Frames::empty() const
+    {
+        return this->items_.empty();
+    }
+
     bool Frames::animated() const
     {
         return this->items_.size() > 1;
@@ -137,13 +169,13 @@ namespace detail {
     QVector<Frame<QImage>> readFrames(QImageReader &reader, const Url &url)
     {
         QVector<Frame<QImage>> frames;
+        frames.reserve(reader.imageCount());
 
         QImage image;
         for (int index = 0; index < reader.imageCount(); ++index)
         {
             if (reader.read(&image))
             {
-                QPixmap::fromImage(image);
                 // It seems that browsers have special logic for fast animations.
                 // This implements Chrome and Firefox's behavior which uses
                 // a duration of 100 ms for any frames that specify a duration of <= 10 ms.
@@ -153,7 +185,7 @@ namespace detail {
                 if (duration <= 10)
                     duration = 100;
                 duration = std::max(20, duration);
-                frames.push_back(Frame<QImage>{image, duration});
+                frames.push_back(Frame<QImage>{std::move(image), duration});
             }
         }
 
@@ -178,9 +210,12 @@ namespace detail {
 
         while (!queued.empty())
         {
-            queued.front().first(queued.front().second);
+            auto front = std::move(queued.front());
             queued.pop();
 
+            // Call Assign with the vector of frames
+            front.first(std::move(front.second));
+
             if (++i > 50)
             {
                 QTimer::singleShot(3, [&] {
@@ -200,9 +235,14 @@ namespace detail {
     auto makeConvertCallback(const QVector<Frame<QImage>> &parsed,
                              Assign assign)
     {
+        static std::queue<std::pair<Assign, QVector<Frame<QPixmap>>>> queued;
+        static std::mutex mutex;
+        static std::atomic_bool loadedEventQueued{false};
+
         return [parsed, assign] {
             // convert to pixmap
-            auto frames = QVector<Frame<QPixmap>>();
+            QVector<Frame<QPixmap>> frames;
+            frames.reserve(parsed.size());
             std::transform(parsed.begin(), parsed.end(),
                            std::back_inserter(frames), [](auto &frame) {
                                return Frame<QPixmap>{
@@ -211,15 +251,9 @@ namespace detail {
                            });
 
             // put into stack
-            static std::queue<std::pair<Assign, QVector<Frame<QPixmap>>>>
-                queued;
-            static std::mutex mutex;
-
             std::lock_guard<std::mutex> lock(mutex);
             queued.emplace(assign, frames);
 
-            static std::atomic_bool loadedEventQueued{false};
-
             if (!loadedEventQueued)
             {
                 loadedEventQueued = true;
@@ -235,7 +269,9 @@ namespace detail {
 // IMAGE2
 Image::~Image()
 {
-    if (this->empty_)
+    ImageExpirationPool::instance().removeImagePtr(this);
+
+    if (this->empty_ && !this->frames_)
     {
         // No data in this image, don't bother trying to release it
         // The reason we do this check is that we keep a few (or one) static empty image around that are deconstructed at the end of the programs lifecycle, and we want to prevent the isGuiThread call to be called after the QApplication has been exited
@@ -268,13 +304,37 @@ ImagePtr Image::fromUrl(const Url &url, qreal scale)
     return shared;
 }
 
-ImagePtr Image::fromPixmap(const QPixmap &pixmap, qreal scale)
+ImagePtr Image::fromResourcePixmap(const QPixmap &pixmap, qreal scale)
 {
-    auto result = ImagePtr(new Image(scale));
+    using key_t = std::pair<const QPixmap *, qreal>;
+    static std::unordered_map<key_t, std::weak_ptr<Image>, boost::hash<key_t>>
+        cache;
+    static std::mutex mutex;
 
-    result->setPixmap(pixmap);
+    std::lock_guard<std::mutex> lock(mutex);
 
-    return result;
+    auto it = cache.find({&pixmap, scale});
+    if (it != cache.end())
+    {
+        auto shared = it->second.lock();
+        if (shared)
+        {
+            return shared;
+        }
+        else
+        {
+            cache.erase(it);
+        }
+    }
+
+    auto newImage = ImagePtr(new Image(scale));
+
+    newImage->setPixmap(pixmap);
+
+    // store in cache
+    cache.insert({{&pixmap, scale}, std::weak_ptr<Image>(newImage)});
+
+    return newImage;
 }
 
 ImagePtr Image::getEmpty()
@@ -335,6 +395,11 @@ boost::optional<QPixmap> Image::pixmapOrLoad() const
 {
     assertInGuiThread();
 
+    // Mark the image as just used.
+    // Any time this Image is painted, this method is invoked.
+    // See src/messages/layouts/MessageLayoutElement.cpp ImageLayoutElement::paint, for example.
+    this->lastUsed_ = std::chrono::steady_clock::now();
+
     this->load();
 
     return this->frames_->current();
@@ -346,8 +411,10 @@ void Image::load() const
 
     if (this->shouldLoad_)
     {
-        const_cast<Image *>(this)->shouldLoad_ = false;
-        const_cast<Image *>(this)->actuallyLoad();
+        Image *this2 = const_cast<Image *>(this);
+        this2->shouldLoad_ = false;
+        this2->actuallyLoad();
+        ImageExpirationPool::instance().addImagePtr(this2->shared_from_this());
     }
 }
 
@@ -439,9 +506,12 @@ void Image::actuallyLoad()
 
             auto parsed = detail::readFrames(reader, shared->url());
 
-            postToThread(makeConvertCallback(parsed, [weak](auto frames) {
+            postToThread(makeConvertCallback(parsed, [weak](auto &&frames) {
                 if (auto shared = weak.lock())
-                    shared->frames_ = std::make_unique<detail::Frames>(frames);
+                {
+                    shared->frames_ =
+                        std::make_unique<detail::Frames>(std::move(frames));
+                }
             }));
 
             return Success;
@@ -459,6 +529,13 @@ void Image::actuallyLoad()
         .execute();
 }
 
+void Image::expireFrames()
+{
+    assertInGuiThread();
+    this->frames_->clear();
+    this->shouldLoad_ = true;  // Mark as needing load again
+}
+
 bool Image::operator==(const Image &other) const
 {
     if (this->isEmpty() && other.isEmpty())
@@ -476,4 +553,96 @@ bool Image::operator!=(const Image &other) const
     return !this->operator==(other);
 }
 
+ImageExpirationPool::ImageExpirationPool()
+{
+    QObject::connect(&this->freeTimer_, &QTimer::timeout, [this] {
+        if (isGuiThread())
+        {
+            this->freeOld();
+        }
+        else
+        {
+            postToThread([this] {
+                this->freeOld();
+            });
+        }
+    });
+
+    this->freeTimer_.start(
+        std::chrono::duration_cast<std::chrono::milliseconds>(
+            IMAGE_POOL_CLEANUP_INTERVAL));
+}
+
+ImageExpirationPool &ImageExpirationPool::instance()
+{
+    static ImageExpirationPool instance;
+    return instance;
+}
+
+void ImageExpirationPool::addImagePtr(ImagePtr imgPtr)
+{
+    std::lock_guard<std::mutex> lock(this->mutex_);
+    this->allImages_.emplace(imgPtr.get(), std::weak_ptr<Image>(imgPtr));
+}
+
+void ImageExpirationPool::removeImagePtr(Image *rawPtr)
+{
+    std::lock_guard<std::mutex> lock(this->mutex_);
+    this->allImages_.erase(rawPtr);
+}
+
+void ImageExpirationPool::freeOld()
+{
+    std::lock_guard<std::mutex> lock(this->mutex_);
+
+#ifndef NDEBUG
+    size_t numExpired = 0;
+    size_t eligible = 0;
+#endif
+
+    auto now = std::chrono::steady_clock::now();
+    for (auto it = this->allImages_.begin(); it != this->allImages_.end();)
+    {
+        auto img = it->second.lock();
+        if (!img)
+        {
+            // This can only really happen from a race condition because ~Image
+            // should remove itself from the ImageExpirationPool automatically.
+            it = this->allImages_.erase(it);
+            continue;
+        }
+
+        if (img->frames_->empty())
+        {
+            // No frame data, nothing to do
+            ++it;
+            continue;
+        }
+
+#ifndef NDEBUG
+        ++eligible;
+#endif
+
+        // Check if image has expired and, if so, expire its frame data
+        auto diff = now - img->lastUsed_;
+        if (diff > IMAGE_POOL_IMAGE_LIFETIME)
+        {
+#ifndef NDEBUG
+            ++numExpired;
+#endif
+            img->expireFrames();
+            // erase without mutex locking issue
+            it = this->allImages_.erase(it);
+            continue;
+        }
+
+        ++it;
+    }
+
+#ifndef NDEBUG
+    qCDebug(chatterinoImage) << "freed frame data for" << numExpired << "/"
+                             << eligible << "eligible images";
+#endif
+}
+
 }  // namespace chatterino

src/messages/Image.hpp

@@ -3,11 +3,14 @@
 #include <QPixmap>
 #include <QString>
 #include <QThread>
+#include <QTimer>
 #include <QVector>
 #include <atomic>
 #include <boost/noncopyable.hpp>
 #include <boost/optional.hpp>
 #include <boost/variant.hpp>
+#include <chrono>
+#include <map>
 #include <memory>
 #include <mutex>
 #include <pajlada/signals/signal.hpp>
@@ -26,9 +29,11 @@ namespace detail {
     {
     public:
         Frames();
-        Frames(const QVector<Frame<QPixmap>> &frames);
+        Frames(QVector<Frame<QPixmap>> &&frames);
         ~Frames();
 
+        void clear();
+        bool empty() const;
         bool animated() const;
         void advance();
         boost::optional<QPixmap> current() const;
@@ -56,7 +61,7 @@ public:
     ~Image();
 
     static ImagePtr fromUrl(const Url &url, qreal scale = 1);
-    static ImagePtr fromPixmap(const QPixmap &pixmap, qreal scale = 1);
+    static ImagePtr fromResourcePixmap(const QPixmap &pixmap, qreal scale = 1);
     static ImagePtr getEmpty();
 
     const Url &url() const;
@@ -80,13 +85,46 @@ private:
 
     void setPixmap(const QPixmap &pixmap);
     void actuallyLoad();
+    void expireFrames();
 
     const Url url_{};
     const qreal scale_{1};
     std::atomic_bool empty_{false};
 
-    // gui thread only
+    mutable std::chrono::time_point<std::chrono::steady_clock> lastUsed_;
+
     bool shouldLoad_{false};
+
+    // gui thread only
     std::unique_ptr<detail::Frames> frames_{};
+
+    friend class ImageExpirationPool;
 };
+
+class ImageExpirationPool
+{
+private:
+    friend class Image;
+
+    ImageExpirationPool();
+    static ImageExpirationPool &instance();
+
+    void addImagePtr(ImagePtr imgPtr);
+    void removeImagePtr(Image *rawPtr);
+
+    /**
+     * @brief Frees frame data for all images that ImagePool deems to have expired.
+     * 
+     * Expiration is based on last accessed time of the Image, stored in Image::lastUsed_.
+     * Must be ran in the GUI thread.
+     */
+    void freeOld();
+
+private:
+    // Timer to periodically run freeOld()
+    QTimer freeTimer_;
+    std::map<Image *, std::weak_ptr<Image>> allImages_;
+    std::mutex mutex_;
+};
+
 }  // namespace chatterino

src/messages/MessageBuilder.cpp

@@ -31,7 +31,8 @@ MessagePtr makeSystemMessage(const QString &text, const QTime &time)
 EmotePtr makeAutoModBadge()
 {
     return std::make_shared<Emote>(Emote{
-        EmoteName{}, ImageSet{Image::fromPixmap(getResources().twitch.automod)},
+        EmoteName{},
+        ImageSet{Image::fromResourcePixmap(getResources().twitch.automod)},
         Tooltip{"AutoMod"},
         Url{"https://dashboard.twitch.tv/settings/moderation/automod"}});
 }

src/providers/twitch/TwitchMessageBuilder.cpp

@@ -344,17 +344,17 @@ MessagePtr TwitchMessageBuilder::build()
     if (this->thread_)
     {
         auto &img = getResources().buttons.replyThreadDark;
-        this->emplace<CircularImageElement>(Image::fromPixmap(img, 0.15), 2,
-                                            Qt::gray,
-                                            MessageElementFlag::ReplyButton)
+        this->emplace<CircularImageElement>(
+                Image::fromResourcePixmap(img, 0.15), 2, Qt::gray,
+                MessageElementFlag::ReplyButton)
             ->setLink({Link::ViewThread, this->thread_->rootId()});
     }
     else
     {
         auto &img = getResources().buttons.replyDark;
-        this->emplace<CircularImageElement>(Image::fromPixmap(img, 0.15), 2,
-                                            Qt::gray,
-                                            MessageElementFlag::ReplyButton)
+        this->emplace<CircularImageElement>(
+                Image::fromResourcePixmap(img, 0.15), 2, Qt::gray,
+                MessageElementFlag::ReplyButton)
             ->setLink({Link::ReplyToMessage, this->message().id});
     }
 

src/widgets/helper/ChannelView.cpp

@@ -1567,7 +1567,7 @@ void ChannelView::mouseMoveEvent(QMouseEvent *event)
                     !element->getThumbnail()->url().string.isEmpty();
                 auto thumb =
                     shouldHideThumbnail
-                        ? Image::fromPixmap(getResources().streamerMode)
+                        ? Image::fromResourcePixmap(getResources().streamerMode)
                         : element->getThumbnail();
                 tooltipPreviewImage.setImage(std::move(thumb));