Implement pluggable memory management

Problem

In some use cases the default ArrayPool behavior does not fit user needs (see Encoding an animated image requires all frames to be loaded into memory, can lead to OutOfMemoryExceptions #222). Users should be able to configure + reset + disable pools.
Some users may want to back Image<T> by memory resources other than managed memory. (Eg. native buffers).

Solution

Implement a pluggable MemoryManager, make the default pooling memory manager configurable.

Update:
feature/memory-manager is the WIP branch for this refactor. It's having #431 merged. If anyone wishes to contribute, please file your PR-s against that branch. The tasks below are being updated according to the progress on that branch.

Tasks for beta-3

Probably 1.0

Passing custom MemoryManager to codecs

Most likely post- 1.0:

Passing custom MemoryManager to Mutate() and Clone() (---> Allow passing custom Configuration instances to processors #650)
Investigate new System.Memory API-s: Memory<T> and
OwnedMemory<T>. Find a way to integrate these classes into ImageSharp.Memory.
Image.Wrap<T>(Memory<T> memory) or similar
Implement an MMFMemoryManager which uses Memory Mapped Files to reduce the pressure on system memory when processing large images.

About processing large images, there's a common issue in .Net which is that very large memory chunks are not moved during a GC, which can result in out of memoty exceptions even when there's enough memory available.

A solution for this I found in the past was to use a low level bitmap primitive that split images into chunks of 64x64 pixels, all wrapped with a generic IBitmap interface. That way, relatively small images, or images that would require very fast procesing (requeted upon image creation) would use an IBitmap that simply wraps an Array. Large images would use instead an IBitmap implementation that stores multiple chunks.

@vpenades

@vpenades the reason the CLR does not move objects larger than ~85kB is the negative performance impact. Objects that big are stored in the LOH. Splitting the images into movable chunks disables that optimization and will negatively influence performance.

A large image still reserves the some amount of memory if split into chunks. Using the power of streams and the filesystem seems to be the better solution here.

@Toxantron

@Toxantron Not everybody is looking for performance; some years ago I was tasked to process images on the size of about 20.000 x 15.000 pixels, which allocated around 1gb RAM, in a single chunk. We also discovered that when compiling for x86, even if theoretically you can allocate 4gb, the Net Framework runtime was actually limited to 2gb only, essentially limiting the load of a single image of such size.

In practice, it was nearly impossible to load such image, because any time we had any object fragmenting the LOH, it was impossible to allocate more than 800 or 900mb in a single chunk, and ended with out of memory exceptions.

We looked for every single image library available at the time (+5 years), and we found all libraries of the time allocated a single chunk.

We ended developing our own custom image library which splitted images in chunks of less than 64kb, to allow the runtime to defragment memory, at the cost of performance.

It is true that time has passed, memory is not that scarce as it was 5 years ago and x64 is commonplace these days... but don't assume people is going to use image libraries the same way, or for the same reasons as you use them. For example, there's lots of areas related to scientific analysis, or design artists working with large form factors, which work with extremely large images.

@vpenades

@vpenades Did you take a look a ImageMagick back then? This can handle extremely large images.

p.s. I am one of the authors.

@vpenades

@vpenades An MMF-based memory manager could achieve the same tradoff (running slower, but using less memory) in a more simple way.

BTW there is a new property GCSettings.LargeObjectCompactionMode since 4.5.1 to address the LOH fragmentation issue.

@vpenades

@vpenades thank you for the clarification.

@antonfirsov I fand that value as well. It comes with performance impacts, but at least we can leave it to the GC then and not implement our own version.

@dlemstra

@dlemstra I think we tried, but if I recall correctly, we had std:boost compiling issues with other third party libraries.

@antonfirsov that's great to know the GC also has the compact large objects feature, I wish I had it a while back.

@Toxantron 😄

Some feedback on this: we currently use ImageSharp for all our image resizing needs in a cloud application and the optimization of speed over memory usage seems to run counter to the idea of horizontal scaling, because of how it holds onto memory. We'd like to provision many containers with a 2 GB memory limit, but currently if we do that, we'd end up with all those containers using 2 GB all of the time. Currently, we run only a few containers limited to 4 GB and they frequently get OOM killed by Docker, even though I already limited image resizing concurrency. This high memory usage seems to be mostly caused by a few outliers in image sizes, most images we process are in the 500 KB range, but some are 10-20 MB, which then consumes 500+ MB of memory just to decode that gets cached indefinitely. Do that concurrently and memory quickly starts to climb to 2-4 GB which never gets released.

From this issue, I can tell you're working on making it configurable and pluggable, but it sounds like a lot of very fundamental work and somewhat long-term. Is there any chance you would accept a more short term quick-fix? Correct me if I'm wrong here, but from what I can tell, most of our memory usage comes from the caching of Block8x8 which is done by the Jpeg decoder, and the memory is provided by the PixelDataPool. If you were to add properties to Configuration that allowed you to set the maxArrayLength / maxArraysPerBucker that get passed to ArrayPool.Create, you could turn off/tweak array pooling for a common scenario.

Something like:

Configuration.Default.Memory.PixelDataPool.MaxArrayLength = 0;
Configuration.Default.Memory.PixelDataPool.MaxArrays = 0;

I understand that this would expose some internal implementation details, but I think (haven't tried it yet, might later) it would also solve some real issues.

@JulianRooze

@JulianRooze I plan to realize my roadmap in February, so it will be part of our 1.0 release or maybe even some beta. Is this late for you?
The only thing I can propose as a quickfix is is to add a temporal configuration API to control the behavior of PixelDataPool for beta-3:

[Obsolete("This is a temporal API, use it for your own risk!")]
public static class PoolConfiguration
{
    [Obsolete("This is a temporal API, use it for your own risk!")]
    public static int MaximumPooledBufferSizeInBytes { get; set; }
}

@JimBobSquarePants what do you think?

Question: I thought the behaviour of ArrayPool was to keep the returned arrays as weak references, so if not used, or the GC needs memory, these arrays would be reclaimed by the GC at some point.

But from what I am reading, does this means the ArrayPool keeps the arrays forever??

@vpenades

@vpenades

[...] if not used, or the GC needs memory, these arrays would be reclaimed by the GC at some point

The GC runs quite frequently, which means that in this case the "pooled" arrays would be GC-d instaneously in most cases. This would go against the whole concept of pooling.

@JimBobSquarePants an other (less dirty) proposal:
By lowering the value of MaximumExpectedImageSize we could ensure that those large outlier images do not eat up the memory of users with limited environments, while still having the pooling mechanism for more common smaller images.

@antonfirsov

@antonfirsov So, from what you're saying, I understand the only proper way to use ImageSharp is in a short lived command line application that starts, does some image processing, and exits soon after?

What about these use cases:

Game App uses ImageSharp to procedurally generate textures at startup, them runs for four hours, without using ImageSharp anymore. (*)
Paint Application, running for hours, where the artists loads and saves images of wildly different image sizes.
Any other long running application that needs image processing, but also uses a lot of memory for its own purposes, for example, scientific analysis.

(*) We wanted to use this technique as a trick to reduce application size on Android/iOS Apps. But, given the limited memory of these devices, it's simply not acceptable to leave an ArrayPool sitting there for the rest of the application's lifecycle... In Androd/iOS, 100mb can be the difference between staying alive or getting killed by the OS.

** It's not like I neglect performance optimizations, I really really appreciate the length of the efforts you're all doing trying to optimize and improve ImageSharp's performance... but, a memory object that, once it's no longer used can't never be reclaimed by the GC is, by all means, a memory leak.

Found this: ArrayPool's author not fond on ArrayPool.Shared for this very same reason.

@vpenades

@vpenades the method you are looking for is Image.LoadPixelData<TPixel>(data, width, height)!

Later, it will be also possible to wrap an existing memory area to be able to temporarily manage it as an "Image". Dispose() will have no effect in the case if memory ownership is external. This feature requires the availability of Memory<T>, so we can have a clean & standard API for that.

@antonfirsov

@antonfirsov Okey, I've tinkered a prototype of the managed image class I would use here.

@antonfirsov

Hello @antonfirsov, apologise for the delay in the reply.

Currently, I'm using a beta
Yes I stopped stress my application
I think I don't want to consume more than 200 MB in single thread mode

@GeorgePlotnikov thanks for the answers!

Can you try if it's any better for you with the latest nightlies? Would be very helpful information for future fine-tuning tips!

@antonfirsov

@antonfirsov, okay will do it and come back with a feedback.

@antonfirsov

@antonfirsov below the result

the process was finished with

An unhandled exception of type 'SixLabors.ImageSharp.Formats.Jpeg.GolangPort.Components.Decoder.EOFException' occurred in System.Private.CoreLib.dll
Reached end of stream before proceeding EOI marker!

Thanks!

That's still quite too much of memory being eaten up :(

The exception that ended your experiment might be a Jpeg bug ... or a totally corrupt "jpeg", that can't be loaded by any library. Can you share the file?

@antonfirsov

@antonfirsov sorry, but not. I fetch files randomly 😄

@antonfirsov

@antonfirsov

Checking the implementation of Buffer<T> IDisposable and Finalizer, I've noticed the finalizer is not disposing everything it should.

The current finalizer implementation here only unpins the GCHandle, but it doesn't free the actual buffer as the Dispose method does.

I am aware there's probably a lof of changes with the upcoming memory managers, so I don't know if this detail is being tracked already.

My point is that an array built from a memory manager must be considered as an unmanaged resource, which means it is eligible to be released in the finalizer, so I guess it would look something like this:

~Buffer()
        {
            this.UnPin();

            if (this.isPoolingOwner)
            {
                PixelDataPool<T>.Return(this.Array);
            }
        }

The problem is that, as far as I know, the GC does not guarantee the order in which managed resources resources are freed, so, it might happen that this.Array has been already freed if it was created by the "NullMemoryManager".

For my use case scenarios, I was considering to rely more on the finalizer, and less on the Dispose(), but I can see a loophole here (that is, to rely on the finalizer to release resources):

With the current implementation, where the finalizer only does UnPin();

For images created with a PooledMemoryManager that requires releasing the resource, the finalizer would never return the memory back to the pool, causing a memory leak.
For images created with a NullMemoryManager, it would work fine.

Now, let's fix the finalizer and add the release of the array:

For images created with PooledMemoryManager, the resource would be returned back to the pool. Since an instance of the array exists in the pool, it is guaranteed the Array object exists within the finalizer execution context.
For images created with NullMemoryManager, it can happen the GC has freed the Array before the Buffer's finalizer is called, so the call to the NullMemoryManager's Return might cause unexpected behaviour, maybe a GC exception.

Anyway, I think the finalizer behaviour needs to be clarified, and in any case, It must free any resources that the GC cannot free on its own.

The only way I can think to easily fix this is the MemoryManagers to have a property that tells if the resources needs to be returned back to the manager, so for the NullMemoryManager, the Buffer would set isPoolingOwner to false, preventing the finalizer to do "weird things" with a potentially disposed object.

@vpenades

@vpenades It's not allowed to touch managed objects in a finalizer! It's only here to free up the unmanaged handles if the developer forgot to call Dispose().

A developer should not rely on finalizers, you should properly manage the disposal of your objects.

@antonfirsov

@antonfirsov Indeed!, that's precisely my point!

Sorry, misunderstood your comment. It's a bug in my code! Gonna be fixed.

I'm almost done, a PR is coming soon.
However .. I'm not yet happy. Based on the feedback in this + other threads, there is some strange behavior I haven't understood yet. 👻 The memory keeps growing for users even when I supposed it should stop doing so.

We need to do a proper load testing investigation before releasing beta-3!

I'm closing this epic, because it's 99% done. Individual issues like #650 should cover the rest.

Navigation Menu

Repository navigation

Implement pluggable memory management #225

Problem

Solution

Tasks for beta-3

Probably 1.0

Most likely post- 1.0:

53 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Implement pluggable memory management #225

Description

Problem

Solution

Tasks for beta-3

Probably 1.0

Most likely post- 1.0:

Activity

vpenades commented on Jun 12, 2017

Toxantron commented on Jun 13, 2017

vpenades commented on Jun 13, 2017

dlemstra commented on Jun 13, 2017

antonfirsov commented on Jun 13, 2017

Toxantron commented on Jun 14, 2017

vpenades commented on Jun 14, 2017

JulianRooze commented on Jan 9, 2018

antonfirsov commented on Jan 9, 2018

vpenades commented on Jan 9, 2018

antonfirsov commented on Jan 9, 2018

vpenades commented on Jan 9, 2018

53 remaining items

antonfirsov commented on Feb 20, 2018

vpenades commented on Feb 20, 2018

0xGeorgii commented on Feb 20, 2018

antonfirsov commented on Feb 20, 2018

0xGeorgii commented on Feb 20, 2018

0xGeorgii commented on Feb 20, 2018

antonfirsov commented on Feb 21, 2018

0xGeorgii commented on Feb 21, 2018

vpenades commented on Feb 22, 2018

antonfirsov commented on Feb 22, 2018

vpenades commented on Feb 22, 2018

antonfirsov commented on Feb 22, 2018

antonfirsov commented on Feb 23, 2018

antonfirsov commented on Aug 7, 2018

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions