General discussion about memory management

[sebastienros]

Just want to start a thread about how memory is best used in the library.

By doing a recent dump of Orchard Core on a page with a single image resized, I noticed the two ArrayPool<char> pools that are used in IS. One was 48MB AFAIR. I don't think these would grow higher with higher load, but maybe a solution could be found to use more granular array sizes, even if it can pool more arrays. Another approach that could be optimized is to have these pools shrink overtime since they should not be used once the images are actually resized, which I could prove by restarting the app, and querying the images would then not incur any allocations on these pools.

Where it's defined:

ImageSharp/src/ImageSharp/Configuration.cs

Line 120 in a1784a6

public MemoryAllocator MemoryAllocator { get; set; } = ArrayPoolMemoryAllocator.CreateDefault();

The two pools:

ImageSharp/src/ImageSharp/Memory/Allocators/ArrayPoolMemoryAllocator.cs

Lines 19 to 27 in a1784a6

	/// <summary>
	/// The <see cref="ArrayPool{T}"/> for small-to-medium buffers which is not kept clean.
	/// </summary>
	private ArrayPool<byte> normalArrayPool;
	/// <summary>
	/// The <see cref="ArrayPool{T}"/> for huge buffers, which is not kept clean.
	/// </summary>
	private ArrayPool<byte> largeArrayPool;

The MemoryAllocator seems to be configurable. I am wondering if that is the ultimate answer, and that I could just set these limits myself.

[antonfirsov]

Briefly: ImageSharp's defaults are bad for ImageSharp.Web and Orchard.

Those defaults have been determined in #475, by an ad-hoc, in-process load-testing method which had the assumption, that there won't be any caching on top of the library. This makes sense thinking of ImageSharp as a library on it's own, but seems to be counter-productive, when it's used together with ImageSharp.Web. If I get it right, in most web applications the variety of image sizes is low, and most images get cached immediately by the middleware, so the value of memory/array pooling seems to be much lower.

If we are lucky, something like ArrayPoolMemoryAllocator.CreateWithModeratePooling will do the job for Orchard, otherwise I recommend to define some benchmarks with Crank, and try finding the optimal defaults for ImageSharp.Web.

EDIT: I would actually try keeping CreateDefault()'s array sizes but lowering the bucket counts.

Another approach that could be optimized is to have these pools shrink overtime since they should not be used once the images are actually resized

For this, we can try to call MemoryAllocator.ReleaseRetainedResources periodically, but that seems to be messy.

---

(Sidenote: Not being familiar with ImageSharp.Web codebase, I got confused about changing the default MemoryAllocator, so opened SixLabors/ImageSharp.Web#152)

[deanmarcussen]

On our production OrchardCore sites we tend to reset the default ImageSharp.Configuration too CreateWithModeratePooling.

This is because we tend to run in Azure, on a low memory / or shared environment, and it becomes a bit resource hungry otherwise, and perhaps something we could introduce by default in OrchardCore.

Particularly, if there are a few shared Orchard Core sites, all with ImageSharp.

For this, we can try to call MemoryAllocator.ReleaseRetainedResources periodically, but that seems to be messy.

This is not something we are doing, and yet it seems relevant. How periodically are we talking? once a minute / once in 5? Unless there is a plan to consider introducing a pool that shrinks, it would seem like something that we could and should run from a background task.

It looks like it is just resetting the pools, so safe to call, at any point during a pending operation?

If I get it right, in most web applications the variety of image sizes is low

Sadly this is often not the case with CMS users, they have a tendency to want to upgrade large files from phones etc, which then get shrunk to tiny files.

We quite regularly run into out of memory exceptions, due to the above, however ImageSharp.Web recovers on the next request, and once it is in cache (or CDN cache), then the issue is resolved.

[antonfirsov]

I'm less critical about the pool releasing approach than I used to be a few days ago.

I see two strategies:

1. Implement periodical releasing of pools in ImageSharp.Web with the help of MemoryAllocator.ReleaseRetainedResources

once a minute / once in 5?

Should depend on the load. The simplest approach is to do it on every Nth request or every Mth megapixels processed. No need for separate thread IMO.

• Pro: easy to implement
• Con: does not benefit the core library, probably no good defaults that work for everyone

2. Track retained memory in ArrayPoolMemoryAllocator, and release pools (fully or partially) it if it reaches a certain pre-configured threshold.

• Pro: benefits the core library. Straightforward to configure ("I don't want the retained memory to grow over Treshold MB-s"). Finding good defaults for ImageSharp.Web seems to be possible.
• Con: non-trivial amount of work and complexity, especially if we want a more sophisticated approach than releasing all the pools after the threshold is reached.

---

In any case, we could definitely use some help from the Orchard community to benchmark the solution and find good defaults.

If I get it right, in most web applications the variety of image sizes is low

Sadly this is often not the case with CMS users, they have a tendency to want to upgrade large files from phones etc, which then get shrunk to tiny files.

Simulating the distribution of the image dimensions is the hardest part of a proper benchmarking solution. I wonder if you have any specific data on this? In #475, I used lognormal distribution with outlier peaks representing common image dimensions. I bet there is a simpler, yet better approach.

[deanmarcussen]

@antonfirsov sorry to be slow coming back on this.

I have anecdotal experience only. So nothing that can help with benchmarking, right now.

What I do have, however is a particular staging site, where I can experiment a little, as I have time with some ideas.
Currently as a wrapper around the ImageSharp.Web processing pipeline, I have some throttling on that particular site, as its image quality is particularly high, and we ran into issues with thumbnailing / carouseling them (even with lazy loading), on first load. (No problem after they are all resized, and in the cache).

The size range seems to be from 1mb -> 10mb+ per source image.

Get's tricky when the landing pages have 30+ images, so it's more than just being able to manage thumbnail generation, but finding a way to get to a point where ImageSharp.Web just never runs out of memory.

What I hope to get into play this week, is a light wrapper around the MemoryAllocator to see if we can't release pools, when the throttled processes are complete, and then can iterate from there, and see if we can find a configurable point, which makes sense.

[antonfirsov]

@deanmarcussen we have now an action plan to address these issues from within the core library, namely: #1597 and #1596.

I'm hoping to have some time to work on this in the next 3-5 weeks. Any chance you can test some private alpha builds with that website? That could serve as a validation for the implemented changes. Can send I send email or twitter DM when the time comes for that work?

[deanmarcussen]

@antonfirsov Great! Just reading, sounds like a much more cunning plan!

Super easy to try out builds against a test version of that site, so whenever you have something to try, just reach out.

[saucecontrol]

@antonfirsov I remember having a chat a while back (on gitter I think?) about the stress tests I ran on the usual set of imaging libraries and I told you I'd share my setup next time I did it. Well, I finally did it! The latest ImageSharp build had major CPU usage improvements, but the memory was still extreme in x64 builds and it still OOMs in x86 builds. Here's the project if you want to take a look: https://github.com/saucecontrol/core-imaging-playground/tree/beeees/MemoryStress

[antonfirsov]

A quick dotMemory "Memory traffic" analysis shows that the larger JpegDecoder allocation is the allocation of the image itself:

This would be eliminated entirely, if we don't need to allocate the large (let's say 5000 x 5000) source image, only the 500 x 500 thumbnail.

(I need to double check about JpegComponent.SpectralBlocks though, it's the second on the list, but I'm surprised it doesn't show more.)

I'm not saying this is easy work, but not impossible. The resize engine needs to be just a bit more modular to be plugged into the decoders, which then can feed the decoded image rows to the resizer's sliding window. If not implemented for a particular decoder, default implementation could be just full decode + resize as it is today.

[antonfirsov]

But the main thing I'm saying is: we need more data.

I want to see how our memory usage compares to other, pipelining and non-pipelining resizers.

[saucecontrol]

@JimBobSquarePants

Well this is my number one priority fixing when I’m back on my feet

Dude, I just saw your twitter status. Hope you're feeling better.

@antonfirsov

do you any fair comparison of our memory behavior to other libraries' that don't have the "decode into target size" feature?

This is an interesting question, although your wording implies that the comparison with the default behavior of these libraries is unfair, which I think is the wrong way to look at it. From an end user's perspective, they just want a job done, and if some tools can do that job more efficiently, that's a desirable feature. If a library lacks that desirable feature, I'd say that's a negative that's completely fair to show.

There are multiple factors that affect potential efficiency here as well. First is the question of decoding the image to less than full size, which you pointed out is part of the Skia API but not used in these benchmarks (not my benchmarks, BTW -- just copied from bleroy's). Both MagicScaler and Vips do that by default, however they decode to a size larger than the target so they can get the advantage of high-quality resampling in the final image, which is important for quality. It would be interesting to see what impact that has on SkiaSharp's numbers, particularly since they changed it recently to do colorspace conversion on image load, which is why they suddenly got a lot slower than ImageSharp on bleroy's benchmark suite.

In the typical case of decoding a JPEG to a smaller size, this doesn't involve resampling the image but rather changing the decode itself so that the frequency domain data is reinterpreted before conversion to spatial data. The simplest form of this would be an 8:1 resize by taking the the DC coefficient as the single value that represents an 8x8 source block and simply using it as the value of a single output pixel. You can find that as well as 4:1 and 2:1 decoding in libjpeg-turbo's implementation. That feature could be a relatively simple addition to ImageSharp's JPEG codec and would definitely be of use in ImageSharp.Web.

The second factor is the question of when pixels are materialized. Again, MagicScaler and Vips operate differently from the others here in that they both stream requests, materializing only as many pixels as are necessary to fill the next pipeline stage. System.Drawing can operate in this manner as well, but there are restrictions around that, one of which is that it doesn't work with either of their HighQualityXXX resampling modes. In all of these streaming libraries, if one wants to treat the image as a bitmap, materialization must be forced. System.Drawing has LockBits to do that. In cases where the Bitmap has already been materialized, LockBits simply returns a pointer to the existing memory, but in cases where the Bitmap represents a promise of pixels, LockBits forces the materialization itself.

It may be possible to change ImageSharp so that it operates on more of a streaming model, where an Image<T> represents a promise of pixels that may or may not exist currently, and a call to TryGetPixelRowSpan would have to be preceded by a LockBits or the like to force materialization of that promise.

Outside those suggestions, I think in looking at the memory graphs from WPA, one can see that the memory usage in x64 processes might be inflated simply because the GC has room to grow the process memory.

That's a fairly typical GC pattern, although it's worth pointing out that even the post-GC dips there are showing allocations of around 2GB, which would explain the OOM in 32-bit -- if its baseline is anything close to 2GB while running, that doesn't leave a lot of breathing room.

One option for addressing that would be some kind of synchronization mechanism around large buffer acquisition, which I believe is how System.Drawing works. You can see from its graphs that CPU use is uneven, which I think comes from its attempt to limit memory use when converting to PBGRA format for its HighQualityBicubic interpolator.

And finally, since it seems to be large buffers that are the issue, you might consider using unmanaged memory for buffers over some large threshold, since that would allow you deterministic release of those buffers which would normally go on the LOH and only be collected on a Gen2 GC. By virtue of the fact MagicScaler uses native codecs that do their own unmanaged buffer allocations, I am able to avoid any LOH allocations at all in most scenarios, so the memory graphs there don't have the typical GC shape:

[antonfirsov]

Thanks for the analysis @saucecontrol!

your wording implies that the comparison with the default behavior of these libraries is unfair

Sorry I wasn't clear, I meant that the comparison should include unmanaged memory usage. We've seen dudes complaining on ImageSharp because of the value seen in the Allocated column under BenchMarkDotNet results compared to native libs "allocation". This is definitely not your level, so I should have omitted this wording. That said, I would consider removing the GC columns from your reports, since it's no-info / misleading.

Regarding the analysis, I see now how GC is PIA in for the library. Note that for very large images and buffers there is no array pooling to avoid too much memory being retained. In the Bee Heads album most images are way over our 8 Megapixels threshold, so no surprise we see the GC ruling down everything. This is far from ideal behavior.

Note that this problem is quite different from the one that has been reported originally by CMS users (too much being memory retained by array pools), although an unmanaged MemoryAllocator may ease the pain for both, and implementing "decode to target size" may eventually solve them entirely for the thumbnail making use case.

I'm about to open separate issues to track & discuss those ideas, since the discussion here seems to be overloaded with (related but different) topics.

[saucecontrol]

Ah, it's all clear now. I have been privy to a few of those complaints from the uninformed, and I agree the numbers can be misleading. I think I've done an ok job of explaining that in the text surrounding the BDN results on my readme, but maybe I can do better.

I disagree that the MemoryDiagnoser values are no-info, though, simply because GC allocations come with a hidden cost that is paid only on collection. You can see that in the WPA graphs from the ImageSharp run I pasted above. Each memory drop associated with a GC has a corresponding drop in CPU because .NET's GC is of the stop-the-world variety. Each of those CPU dips would represent higher response times in a web app, UI freezes in a desktop app, etc... so as unattractive as they are, I think they're important data points.

I appreciate your reading through my analysis in my readme, though, and I appreciate the feedback. I do try to approach those things fairly, which can be difficult given the broad differences in the design and purpose of the libraries involved. I'd like to focus on helping you get those numbers down rather than hiding them :)

On that point, I think it's unfortunate that bleroy's benchmarks have been so much of a focus of attention for those of us working in this space. Creating a 150px thumbnail of a 1 megapixel image isn't exactly real world. We'd more likely be creating 1 megapixel 'thumbnails' of 12 megapixel phone camera images, and that's what I was going for with the Bee Heads test. It would be worth changing the output size to something more realistic now that I think about it, but I don't anticipate that would make much difference in the relative results.

One last thing I want to mention because I think it's on topic for this discussion post... I had a quick look at the allocator code you linked, and I see you're creating 'normal' and 'large' pools with values based on the allocator chosen. I'd strongly recommend you swap out your 'normal' pool for ArrayPool<byte>.Shared, regardless of allocator configuration chosen, so that apps aren't maintaining 2 isolated pools of <= 1MiB arrays. The Shared pool also has GC-pressure-based cleanup built in, although I recognize that would eliminate your ability to flush the pool on demand as you can now. Funnily enough, I remembered reading a blog post Adam Sitnik did about ArrayPool in which he pointed out ImageSharp as using the Shared pool exclusively. I had always assumed that was true :)