ArrayPool<T>/MemoryPool<T> Best Practice

[GF-Huang]

When should they be used? Are there any guidelines?

And the docs said:

Using the ArrayPool class to rent and return buffers (using the Rent and Return methods) can improve performance in situations where arrays are created and destroyed frequently, resulting in significant memory pressure on the garbage collector.

It's not very clear, how frequent is frequent?

Is it premature optimization to use them instead of temporary arrays which created in the all method bodies?
Even refactor all short life object like the follow, is this bad?

class ShortLife : IDisposable {
    private byte[] _buffer;
    
    public ShortLife(...) {
        _buffer = ArrayPool<byte>.Shared.Rent(xxx);
    }
    public void Dispose() {
        ArrayPool<byte>.Shared.Return(_buffer);
    }
}

// then use
using var shortLife = new ShortLife(...);

[davidfowl]

The shared array pool is optimized for synchronous rent and return. I think that's where the short lived advice comes from.

[Clockwork-Muse]

The other thing that should be mentioned here is that if your array is also small (in bytes used - size of type * number of elements), in addition to being short-lived, it may behoove you to look at using stackalloced arrays.

[GF-Huang]

In the async method body, stackalloc seems cumbersome to use and requires a static local function.

[Clockwork-Muse]

And that's just another part of the determination. Although note that if you're in an async method, they may not "live" long enough - since in many cases you use the array as a buffer to an async call, you'd run into problems at that point.

[Akeit0]

I think we need guidelines.

I was optimizing Linq's TakeLast, but I have a different opinion from the reviewer regarding ArrayPool and yield. I would like official guidelines (at least for implementations within the BCL).

Me:
#118490 (comment)

The reason I return the array just before yielding is that not all yielding are done synchronously, and it is possible to hold an array rent from ArrayPool. In this case, it is customary not to use rent arrays.

Reviewer:
#118490 (comment)

Pooled buffers can certainly cross yield boundaries, it's just that you can no longer use stack allocated buffers.

[ichensky]

The use of ArrayPool.Rent and Return can be used, for example, within a loop where an array of varying size needs to be created multiple times. The ArrayPool attempts to re-use memory allocated for prior arrays.

for (int i =0; ... ){
    var buffer = ArrayPool<int>.Shared.Rent(N);
    ...
    ArrayPool.Return(buffer, clearArray: true);
}

However, in many cases, when N is constant (or when the exact value doesn't strictly matter), such code can be optimized by moving the buffer to an outer loop or function and just re-using it.

int[] buffer = new int[N];
for (int i =0; ... ){
    ...
}

[martin-steinhoefer]

Thank you for your response.

The example violates ownership constraints (pooled escapes). That would be the case even without the try { ... } finally { ... } construct. That construct is unable to help in such usage scenarios.

I should have asked for a example, where placing the Return into a finally block actually causes a use-after-free problem - I couldn't come up with one.

The guidance mentioned is somewhat misleading to me:
It tells the reader that it is bad practice to use try { ... } finally { ... } constructs because it does not save from the escape problem. Although that is true - it completely misses the point:
In case of ArrayPool<T> the only resource managed is memory - which ich actually managed by GC anyway. So missing to Return a rented array is no problem. So the pattern is not required but using it should not be dis-couraged. In fact, the guidance additionally refers to ObjectPool<T> as a similar case - where following it actually can create problems - if T holds resources other than memory (f.i. SqlConnection). Following the guidance in this case will lead to problems.
The guidance should have been like:
DO keep matching....
DON'T use try/finally pattern because it complicates source and adds no value. Arrays obtained by Rent are perfectly cleaned up by GC.
DO be aware, that this does apply only to arrays obtained by ArrayPool<T>.Shared.Rent (and does not apply to ObjectPool<T>)

[EgorBo]

@martin-steinhoefer

DON'T use try/finally pattern because it complicates source and adds no value.

The example above perfectly shows why try-finally may cause use-after-free where just lack of try-finally doesn't have an issue, it's definitely not just "adds no value.". It's indeed a general ownership/lifetime issue that cannot be solved today in modern C#, so at very least we don't recommend certain patterns

[martin-steinhoefer]

It's the very same as with ownership of IDisposable instances. If the instance escapes - it MUST NOT be released (returned rsp. disposed).

Usage of 'Return' (or 'Dispose') is contra-indicated if the instance escapes. That is, what the guideline should state - clearly.
And usage of the pattern in a valid scenario adds little value - as abandoned buffers are perfectly fine GC-ed. That is, what the guideline should state.
And the guideline should not refer to ObjectPool<T> as completely different thoughts apply.

[EgorBo]

it MUST NOT be released (returned rsp. disposed).

That is not a correct guideline I think, some types might be perfectly protected from use-after-free (by checking isDisposed internally) and can lead to a bad behavior if not disposed at all.

And the guideline should not refer to ObjectPool as completely different thoughts apply.

No, same rules apply to ObjectPool:

var pool = ObjectPool.Create<StringBuilder>();
var obj = pool.Get();
pool.Return(obj);
var obj2 = pool.Get();

obj.Append("hello"); // use-after-free
Console.WriteLine(obj2.ToString()); // prints "hello"

[martin-steinhoefer]

The fact, that both ArrayPool<T> and ObjectPool<T> are prone to use-after-free bugs is undisputed.

In your ObjectPool<T> example you missed the fact, that T itself can be a resource (IDisposable) and missing to dispose it can lead to resource leak. Contrary to ArrayPool<T> which gets you merely memory (the array contains nothing - you put your objects into it) on ObjectPool<T> you get an instance - which you get ownership of - so you are obliged to release it. You can take care of your responsibility by either invoking Return or Dispose - but you are obliged to do either (if the instance does not escape - otherwise the caller becomes the owner and responsible for invoking Dispose). You are freed from this only if T itself does not need releasing (contrary to ArrayPool<T>). If you did take care of your responsibility, accessing the instance afterwards may result in a use-after-free bug.

Why is the the rule, that if the instance escapes then it must not be released, a bad guidance? I did not say, that those must not be released (disposed) at all.
Ownership transfers to the caller / receiver which then becomes responsible for releasing the object - in case of ArrayPool<T> that is easy - just let go of the reference. In case of IDisposable you become responsible for invoking Dispose. However, releasing the resource early if the instance escapes is likely an error in both cases. Consider the usual IDisposable implementation - when diposed most if the members throw ObjectDisposedException - so releasing (disposing) an escaped instance will likely lead to throwing (luckily we usually get an exception here - contrary to the hard to detect ArrayPool<T> case).
Just because you can imagine types, that are perfectly protected against use-after-free - most of them aren't. And a guideline should target the common case - while not hindering the special one.

Perhaps the guideline should be more detailed and explain the background (maybe using examples).

[martin-steinhoefer]

When should they be used? Are there any guidelines?

And the docs said:

Using the ArrayPool class to rent and return buffers (using the Rent and Return methods) can improve performance in situations where arrays are created and destroyed frequently, resulting in significant memory pressure on the garbage collector.

It's not very clear, how frequent is frequent?

Is it premature optimization to use them instead of temporary arrays which created in the all method bodies? Even refactor all short life object like the follow, is this bad?

class ShortLife : IDisposable {
    private byte[] _buffer;
    
    public ShortLife(...) {
        _buffer = ArrayPool<byte>.Shared.Rent(xxx);
    }
    public void Dispose() {
        ArrayPool<byte>.Shared.Return(_buffer);
    }
}

// then use
using var shortLife = new ShortLife(...);

That is actually trading creation of an IDisposable for creation of an array instance. Only in case the array to be rented is large enough that will pay off. Usage of pooled arrays seems not to benefit from wrapping them up in IDisposable (pooled objects may be a different story): - You want the array - the IDisposable is not an array (and making it mimicking one is cumbersome).