diff --git a/CHANGELOG.md b/CHANGELOG.md index 52b87d295..1e6e6f9a1 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/src/controllers/moderationactions/ModerationAction.cpp b/src/controllers/moderationactions/ModerationAction.cpp index a24ce9e85..90b3e1de4 100644 --- a/src/controllers/moderationactions/ModerationAction.cpp +++ b/src/controllers/moderationactions/ModerationAction.cpp @@ -141,9 +141,11 @@ const boost::optional &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_; diff --git a/src/messages/Image.cpp b/src/messages/Image.cpp index fda0637f6..95abb3bcd 100644 --- a/src/messages/Image.cpp +++ b/src/messages/Image.cpp @@ -6,7 +6,9 @@ #include #include #include +#include #include +#include #include #include "Application.hpp" @@ -23,7 +25,10 @@ #include "util/DebugCount.hpp" #include "util/PostToThread.hpp" -#include +// 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> &frames) - : items_(frames) + Frames::Frames(QVector> &&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> readFrames(QImageReader &reader, const Url &url) { QVector> 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{image, duration}); + frames.push_back(Frame{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> &parsed, Assign assign) { + static std::queue>>> queued; + static std::mutex mutex; + static std::atomic_bool loadedEventQueued{false}; + return [parsed, assign] { // convert to pixmap - auto frames = QVector>(); + QVector> frames; + frames.reserve(parsed.size()); std::transform(parsed.begin(), parsed.end(), std::back_inserter(frames), [](auto &frame) { return Frame{ @@ -211,15 +251,9 @@ namespace detail { }); // put into stack - static std::queue>>> - queued; - static std::mutex mutex; - std::lock_guard 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; + static std::unordered_map, boost::hash> + cache; + static std::mutex mutex; - result->setPixmap(pixmap); + std::lock_guard 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(newImage)}); + + return newImage; } ImagePtr Image::getEmpty() @@ -335,6 +395,11 @@ boost::optional 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(this)->shouldLoad_ = false; - const_cast(this)->actuallyLoad(); + Image *this2 = const_cast(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(frames); + { + shared->frames_ = + std::make_unique(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( + IMAGE_POOL_CLEANUP_INTERVAL)); +} + +ImageExpirationPool &ImageExpirationPool::instance() +{ + static ImageExpirationPool instance; + return instance; +} + +void ImageExpirationPool::addImagePtr(ImagePtr imgPtr) +{ + std::lock_guard lock(this->mutex_); + this->allImages_.emplace(imgPtr.get(), std::weak_ptr(imgPtr)); +} + +void ImageExpirationPool::removeImagePtr(Image *rawPtr) +{ + std::lock_guard lock(this->mutex_); + this->allImages_.erase(rawPtr); +} + +void ImageExpirationPool::freeOld() +{ + std::lock_guard 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 diff --git a/src/messages/Image.hpp b/src/messages/Image.hpp index a4ad674fe..d47532acc 100644 --- a/src/messages/Image.hpp +++ b/src/messages/Image.hpp @@ -3,11 +3,14 @@ #include #include #include +#include #include #include #include #include #include +#include +#include #include #include #include @@ -26,9 +29,11 @@ namespace detail { { public: Frames(); - Frames(const QVector> &frames); + Frames(QVector> &&frames); ~Frames(); + void clear(); + bool empty() const; bool animated() const; void advance(); boost::optional 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 lastUsed_; + bool shouldLoad_{false}; + + // gui thread only std::unique_ptr 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> allImages_; + std::mutex mutex_; +}; + } // namespace chatterino diff --git a/src/messages/MessageBuilder.cpp b/src/messages/MessageBuilder.cpp index d76b8b4a1..18f2b9102 100644 --- a/src/messages/MessageBuilder.cpp +++ b/src/messages/MessageBuilder.cpp @@ -31,7 +31,8 @@ MessagePtr makeSystemMessage(const QString &text, const QTime &time) EmotePtr makeAutoModBadge() { return std::make_shared(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"}}); } diff --git a/src/providers/twitch/TwitchMessageBuilder.cpp b/src/providers/twitch/TwitchMessageBuilder.cpp index b2eb7488d..38572b6e2 100644 --- a/src/providers/twitch/TwitchMessageBuilder.cpp +++ b/src/providers/twitch/TwitchMessageBuilder.cpp @@ -344,17 +344,17 @@ MessagePtr TwitchMessageBuilder::build() if (this->thread_) { auto &img = getResources().buttons.replyThreadDark; - this->emplace(Image::fromPixmap(img, 0.15), 2, - Qt::gray, - MessageElementFlag::ReplyButton) + this->emplace( + Image::fromResourcePixmap(img, 0.15), 2, Qt::gray, + MessageElementFlag::ReplyButton) ->setLink({Link::ViewThread, this->thread_->rootId()}); } else { auto &img = getResources().buttons.replyDark; - this->emplace(Image::fromPixmap(img, 0.15), 2, - Qt::gray, - MessageElementFlag::ReplyButton) + this->emplace( + Image::fromResourcePixmap(img, 0.15), 2, Qt::gray, + MessageElementFlag::ReplyButton) ->setLink({Link::ReplyToMessage, this->message().id}); } diff --git a/src/widgets/helper/ChannelView.cpp b/src/widgets/helper/ChannelView.cpp index a7d3ad4ad..837df651a 100644 --- a/src/widgets/helper/ChannelView.cpp +++ b/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));