High memory usage #93
Comments
|
Hello! |
|
I think the resizing process itself can be done a bit more efficiently (mostly by not doing it). In my case most of the covers are 500x500 px, so resizing them to ~512x512 px not only wastes CPU cycles, but also reduces the image quality, due to recompression. Smaller covers resized up also won't provide more detail. The only case when resizing might be beneficial is if a cover is 1000x1000 px or bigger. |
|
Ok there seems to be strange things happening: even when reducing LMS's cover cache entry size to 1, memory usage keep growing. Edit: I gave a try storing compressed images in the cache, memory still increases but much slower. Still not sure why it keeps increasing though. |
|
Ok, I have reworked this thing, here is what I got now when scrolling my collection: Actually, the browser gathers several cover files in parallel requests, and that leads to different peaks. We may add a config entry for that in order to tweak the number of threads dispatching requests (would be typically quite small or even 1 on mono core systems). |
|
This looks very good! Having short-timed and much smaller peaks reduces this to minor issue, compared to the current situation (I have one instance which at this moment is consuming 960MB). |
|
@wolfpld I have made a new release with some improvements. Tell me how it goes and if it needs further investigations! |
|
This is what I get now after scrolling through all the albums: Previously resident set size was 960 MB, as mentioned earlier. Current usage tbh still seems a bit excessive. I also don't see the variation like on the massif graph you have pasted. Memory usage climbs up and just stays there (going through the whole album list again doesn't increase it). Configuration values were not changed from default, i.e.: |
|
By the way, do you use the docker image or the debian package? |
|
Thanks for testing! Edit: for info, here is what I get on the demo server (RPi3B+, debian package) And for my collection (RPi4Go, using docker image) I guess the memory usage is really lower on docker due to the fact the dependent libraries are compiled with minimal features builtin (and particularly openMP is disabled for graphicsmagick + using QuantumDepth=8) |
|
Well now, this is getting more interesting. My server has 2 CPU cores available. Currently its memory usage, as reported by My dev machine has 24 CPU cores. When LMS is started there, it consumes 794 MB, and spins up a whopping 247 threads. Massif doesn't seem to be too helpful here: Anyway, here's full ms_print output: out.txt. There certainly are some hints of OpenMP being used: |
|
Ouch :/ |
|
Doesn't seem to. |
|
With the following proof-of-concept using stb libraries (https://github.com/nothings/stb) I can get LMS to much lower memory usage: And 128 MB reported by diff --git a/src/libs/av/impl/AvInfo.cpp b/src/libs/av/impl/AvInfo.cpp
index 0fdb827..7f503b3 100644
--- a/src/libs/av/impl/AvInfo.cpp
+++ b/src/libs/av/impl/AvInfo.cpp
@@ -223,7 +223,9 @@ MediaFile::visitAttachedPictures(std::function<void(const Picture&)> func) const
const AVPacket& pkt {avstream->attached_pic};
- picture.data = reinterpret_cast<const std::byte*>(pkt.data);
+ auto data = std::shared_ptr<std::byte[]>(new std::byte[pkt.size]);
+ memcpy( data.get(), pkt.data, pkt.size );
+ picture.data = data;
picture.dataSize = pkt.size;
func(picture);
diff --git a/src/libs/av/include/av/AvInfo.hpp b/src/libs/av/include/av/AvInfo.hpp
index 60fe084..12e7f76 100644
--- a/src/libs/av/include/av/AvInfo.hpp
+++ b/src/libs/av/include/av/AvInfo.hpp
@@ -28,6 +28,7 @@
#include <optional>
#include <string>
#include <vector>
+#include <memory>
#include "AvTypes.hpp"
@@ -41,7 +42,7 @@ void AvInit();
struct Picture
{
std::string mimeType;
- const std::byte* data {};
+ std::shared_ptr<std::byte[]> data;
std::size_t dataSize;
};
diff --git a/src/libs/cover/impl/CoverArtGrabber.hpp b/src/libs/cover/impl/CoverArtGrabber.hpp
index 021d3e2..f540504 100644
--- a/src/libs/cover/impl/CoverArtGrabber.hpp
+++ b/src/libs/cover/impl/CoverArtGrabber.hpp
@@ -26,6 +26,7 @@
#include <string_view>
#include <unordered_map>
#include <vector>
+#include <map>
#include "cover/ICoverArtGrabber.hpp"
#include "database/Types.hpp"
diff --git a/src/libs/cover/impl/Image.cpp b/src/libs/cover/impl/Image.cpp
index c628b9f..79196b5 100644
--- a/src/libs/cover/impl/Image.cpp
+++ b/src/libs/cover/impl/Image.cpp
@@ -21,8 +21,15 @@
#include <atomic>
#include <fstream>
+#include <string.h>
-#include <magick/resource.h>
+#define STB_IMAGE_IMPLEMENTATION
+#define STB_IMAGE_RESIZE_IMPLEMENTATION
+#define STB_IMAGE_WRITE_IMPLEMENTATION
+
+#include "stb_image.h"
+#include "stb_image_resize.h"
+#include "stb_image_write.h"
#include "utils/Logger.hpp"
@@ -30,83 +37,41 @@ namespace CoverArt {
void
init(const std::filesystem::path& path)
-{
- Magick::InitializeMagick(path.string().c_str());
-
- if (!MagickLib::SetMagickResourceLimit(MagickLib::ThreadsResource, 1))
- LMS_LOG(COVER, ERROR) << "Cannot set Magick thread resource limit to 1!";
-
- if (!MagickLib::SetMagickResourceLimit(MagickLib::DiskResource, 0))
- LMS_LOG(COVER, ERROR) << "Cannot set Magick disk resource limit to 0!";
-
- LMS_LOG(COVER, INFO) << "Magick threads resource limit = " << GetMagickResourceLimit(MagickLib::ThreadsResource);
- LMS_LOG(COVER, INFO) << "Magick Disk resource limit = " << GetMagickResourceLimit(MagickLib::DiskResource);
-}
-
-EncodedImage::EncodedImage(const std::byte* data, std::size_t dataSize)
-: _blob {data, dataSize}
{
}
-EncodedImage::EncodedImage(Magick::Blob blob)
-: _blob {blob}
+EncodedImage::EncodedImage(const std::shared_ptr<std::byte[]>& data, std::size_t dataSize)
+: m_data( data ), m_dataSize( dataSize )
{
}
const std::byte*
EncodedImage::getData() const
{
- return reinterpret_cast<const std::byte*>(_blob.data());
+ return m_data.get();
}
std::size_t
EncodedImage::getDataSize() const
{
- return _blob.length();
+ return m_dataSize;
}
RawImage::RawImage(const std::filesystem::path& p)
{
- try
- {
- _image.read(p.string().c_str());
- }
- catch (Magick::WarningCoder& e)
- {
- LMS_LOG(COVER, WARNING) << "Caught Magick WarningCoder while loading image '" << p.string() << "': " << e.what();
- }
- catch (Magick::Warning& e)
- {
- LMS_LOG(COVER, WARNING) << "Caught Magick warning while loading raw image '" << p.string() << "': " << e.what();
- throw ImageException {std::string {"Magick read warning: "} + e.what()};
- }
- catch (Magick::Exception& e)
- {
- LMS_LOG(COVER, ERROR) << "Caught Magick exception while loading raw image '" << p.string() << "': " << e.what();
- throw ImageException {std::string {"Magick read error: "} + e.what()};
- }
+ int n;
+ m_data = (std::byte*)stbi_load( p.string().c_str(), &m_width, &m_height, &n, 3 );
}
RawImage::RawImage(const EncodedImage& encodedImage)
{
- try
- {
- _image.read(encodedImage._blob);
- }
- catch (Magick::WarningCoder& e)
- {
- LMS_LOG(COVER, WARNING) << "Caught Magick WarningCoder while loading raw image: " << e.what();
- }
- catch (Magick::Warning& e)
- {
- LMS_LOG(COVER, WARNING) << "Caught Magick warning while loading raw image: " << e.what();
- throw ImageException {std::string {"Magick read warning: "} + e.what()};
- }
- catch (Magick::Exception& e)
- {
- LMS_LOG(COVER, ERROR) << "Caught Magick exception while loading raw image: " << e.what();
- throw ImageException {std::string {"Magick read error: "} + e.what()};
- }
+ int n;
+ m_data = (std::byte*)stbi_load_from_memory( (const stbi_uc*)encodedImage.getData(), encodedImage.m_dataSize, &m_width, &m_height, &n, 3 );
+}
+
+RawImage::~RawImage()
+{
+ free( m_data );
}
void
@@ -115,36 +80,46 @@ RawImage::scale(std::size_t width)
if (width == 0)
throw ImageException {"Bad width = 0"};
- try
+ size_t height;
+ if( m_width == m_height )
{
- _image.resize(Magick::Geometry {static_cast<unsigned int>(width), static_cast<unsigned int>(width)});
+ height = width;
}
- catch (Magick::Exception& e)
+ else if( m_width > m_height )
{
- LMS_LOG(COVER, ERROR) << "Caught Magick exception during scale: " << e.what();
- throw ImageException {std::string {"Magick resize error: "} + e.what()};
+ height = (size_t)((float)width/m_width*m_height);
}
+ else
+ {
+ height = width;
+ width = (size_t)((float)height/m_height*m_width);
+ }
+
+ auto rd = (std::byte*)malloc( width*height*3 );
+ stbir_resize_uint8_srgb( (const unsigned char*)m_data, m_width, m_height, 0, (unsigned char*)rd, width, height, 0, 3, STBIR_ALPHA_CHANNEL_NONE, 0 );
+ free( m_data );
+ m_data = rd;
+ m_width = width;
+ m_height = height;
+}
+
+void writeCb( void* ctx, void* data, int size )
+{
+ auto& vec = *(std::vector<char>*)ctx;
+ const auto sz = vec.size();
+ vec.resize( sz + size );
+ memcpy( vec.data() + sz, data, size );
}
EncodedImage
RawImage::encode() const
{
- try
- {
- Magick::Image outputImage {_image};
-
- outputImage.magick("JPEG");
-
- Magick::Blob blob;
- outputImage.write(&blob);
-
- return EncodedImage {blob};
- }
- catch (Magick::Exception& e)
- {
- LMS_LOG(COVER, ERROR) << "Caught Magick exception while encoding raw image: " << e.what();
- throw ImageException {std::string {"Magick encode error: "} + e.what()};
- }
+ std::vector<char> tmp;
+ stbi_write_jpg_to_func( writeCb, &tmp, m_width, m_height, 3, m_data, 80 );
+ const auto sz = tmp.size();
+ auto buf = std::shared_ptr<std::byte[]>( new std::byte[sz] );
+ memcpy( buf.get(), tmp.data(), sz );
+ return EncodedImage( buf, sz );
}
} // namespace CoverArt
diff --git a/src/libs/cover/impl/Image.hpp b/src/libs/cover/impl/Image.hpp
index d7091e0..0b091fa 100644
--- a/src/libs/cover/impl/Image.hpp
+++ b/src/libs/cover/impl/Image.hpp
@@ -19,11 +19,10 @@
#pragma once
+#include <memory>
#include <filesystem>
#include <vector>
-#include <Magick++.h>
-
#include "utils/Exception.hpp"
namespace CoverArt
@@ -41,16 +40,16 @@ namespace CoverArt
{
public:
EncodedImage() = default;
- EncodedImage(const std::byte* data, std::size_t dataSize);
+ EncodedImage(const std::shared_ptr<std::byte[]>& data, std::size_t dataSize);
const std::byte* getData() const;
std::size_t getDataSize() const;
private:
friend class RawImage;
- EncodedImage(Magick::Blob blob);
- Magick::Blob _blob;
+ std::shared_ptr<std::byte[]> m_data;
+ std::size_t m_dataSize;
};
class RawImage
@@ -58,6 +57,7 @@ namespace CoverArt
public:
RawImage(const std::filesystem::path& p);
RawImage(const EncodedImage& encodedImage);
+ ~RawImage();
// Operations
void scale(std::size_t width);
@@ -66,7 +66,8 @@ namespace CoverArt
EncodedImage encode() const;
private:
- Magick::Image _image;
+ int m_width, m_height;
+ std::byte* m_data = nullptr;
};
} // namespace CoverArt |
|
Interesting, I didn't know this library and it indeed looks promising! |
|
The stb libraries are commonly used by gamedev people, mainly because they are single-header and have simple API, thus it's easy to quickly make something that is doing its job, without much of a fuss. As such, I would only consider them to get things up and running, and then would look for a more efficient alternative. When thinking about stb performance consider that games load images (~90% of game data) much more often than they save them (a screenshot once in a blue moon, I guess?). Note that the resize function I used is operating in srgb color space, not in linear. This is probably a big perf hit, for a small image quality increase. See http://www.ericbrasseur.org/gamma.html?i=1 for context. |
|
Do you mean it is not wise to switch to stb? What do you mean by "efficient alternative" ? More subformats/corner cases handled? Anyway, I am going to test stb myself, and I am also going to test how FreeImage performs (see https://freeimage.sourceforge.io/). Any thoughts on this lib? |
|
I would definitely start with stb, but then would look for a more optimized (i.e. faster) alternative. This usually means using the reference libraries (or forks containing some improvements). So, in case of ID3 tags: I would use libpng for png support and libjpeg-turbo for jpeg support. This should cover the majority of use cases, and stb can be then used as a fallback for the odd gif, or whatever can be there. Writing the resized jpegs can be handled through libjpeg-turbo. As for resizing itself, I can't say much about that, or which library might be useful here. This is a complex subject and what you'd want to use depends heavily on where you want to lie on the performance-quality axis. Upscaling and downscaling are two different problems to consider here. In case of upsampling on one side of the quality range you have nearest neighbour filtering, and on the opposite one, there's AI upscaling. Then there's processing in linear vs gamma-corrected space. Of course you would make different decisions if you keep the cover cache in memory (as you do now) vs preprocessing them and storing on disk. As I said, this is a complex problem. As mentioned earlier, I would consider if resizing covers is even needed in most cases. It certainly makes sense, if the original image is something like 1500x1500 px and 3 MB, but resizing a 640x640 px 100 KB image doesn't seem necessary. |
|
I understand your point of view. I am not sure I want to handle these formats using dedicated libraries, I would expect higher level libraries (like STB, FreeImage, GM, etc.) to bring a far greater ease of usage and maintainability with a very low resource overhead (ideally unnoticeable). But if the resource overhead is really noticeable, that is indeed another story! So I made some changes thanks to your PoC, and using stb I discovered a badly placed lock at the LMS database session handling level. This lock made images be processed sequentially within a session, and since GraphicsMagicks uses parallel algorithms, I have not noticed it. It is now removed. Indeed GM seems to be noticeably faster than STB, that is quite annoying to waste so much CPU cycles resizing images. I will investigate this. STB: GM: At the moment, no clear winner for me (instruction count vs mem usage), but I am still investigating (FreeImage, tweaks for STB?, ...). Edit: I suspect GM built with quantum depth=8 be very close to stb in memory usage (it is 16 on my system) |
|
I would try replacing |
…e (GraphicsMagick can still be selected). closes #93
|
Ok, now using lib STB image by default. |
|
3.21.0 works very nicely with stb, consuming 88 MB RSS. There is no noticable slowdown compared to previous version. |
|
Thanks for this feedback! 👍 |
|
And thanks for the help! |
You say in the readme that low memory usage is one of the features of LMS. I was a bit disappointed to see that in my case LMS actually has very high memory consumption. Below you can see heap snapshot generated using massif:
Rapid growth of memory usage can be seen when scrolling down the list of albums, as new ones are being loaded. This, combined with the amount of required memory would suggest that the cover art image data is being stored uncompressed in the memory?
The text was updated successfully, but these errors were encountered: