dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.26k stars 4.73k forks source link

[API Proposal]: Let MemoryPool<T>.Shared.Rent support clear the data when return #96396

Open universorum opened 10 months ago

universorum commented 10 months ago

Background and motivation

We already have ArrayPool<T>.Return(T[] array, boolclearArray = false) to require the ArrayPool to clear all data in the array before other callers can reuse it. But MemoryPool does not have a similar API to require MemoryPool to clear data. Now, we need to wrap the array into memory and return it, or write 0 to the array to make the same effect.

In the case of MemoryPool<T>.Shared.Rent, we want to return the memory automatically. So the only thing we can do is to explicitly require it in the MemoryPool.Shared.Rent method.

API Proposal

namespace System.Buffers
{
    public abstract class MemoryPool<T> : IDisposable
    {
        public abstract IMemoryOwner<T> Rent(int minBufferSize, bool clearArray);
    }
}
namespace System.Buffers
{
    internal sealed partial class ArrayMemoryPool<T> : MemoryPool<T>
    {
        public sealed override IMemoryOwner<T> Rent(int minimumBufferSize, bool clearArray)
        {
            return new ArrayMemoryPoolBuffer(minimumBufferSize, clearArray);
        }
    }
}
namespace System.Buffers
{
    internal sealed partial class ArrayMemoryPool<T> : MemoryPool<T>
    {
        private sealed class ArrayMemoryPoolBuffer : IMemoryOwner<T>
        {
            private bool _clearArray;

            public ArrayMemoryPoolBuffer(int size, bool clearArray)
            {
                _clearArray = clearArray;
            }

            public void Dispose()
            {
                ArrayPool<T>.Shared.Return(array, _clearArray);
            }
        }
    }
}

API Usage

using var memory = MemoryPool<byte>.Shared.Rent(128, clearArray: true);

Alternative Designs

No response

Risks

No response

ghost commented 10 months ago

Tagging subscribers to this area: @dotnet/area-system-buffers See info in area-owners.md if you want to be subscribed.

Issue Details
### Background and motivation We already have `ArrayPool.Return(T[] array, boolclearArray = false)` to require the ArrayPool to clear all data in the array before other callers can reuse it. But MemoryPool does not have a similar API to require MemoryPool to clear data. Now, we need to wrap the array into memory and return it, or write 0 to the array to make the same effect. In the case of `MemoryPool.Shared.Rent`, we want to return the memory automatically. So the only thing we can do is to explicitly require it in the MemoryPool.Shared.Rent method. ### API Proposal ```csharp namespace System.Buffers { public abstract class MemoryPool : IDisposable { public abstract IMemoryOwner Rent(int minBufferSize, bool clearArray); } } ``` ```csharp namespace System.Buffers { internal sealed partial class ArrayMemoryPool : MemoryPool { public sealed override IMemoryOwner Rent(int minimumBufferSize, bool clearArray) { return new ArrayMemoryPoolBuffer(minimumBufferSize, clearArray); } } } ``` ```csharp namespace System.Buffers { internal sealed partial class ArrayMemoryPool : MemoryPool { private sealed class ArrayMemoryPoolBuffer : IMemoryOwner { private bool _clearArray; public ArrayMemoryPoolBuffer(int size, bool clearArray) { _clearArray = clearArray; } public void Dispose() { ArrayPool.Shared.Return(array, _clearArray); } } } } ``` ### API Usage ```csharp using var memory = MemoryPool.Shared.Rent(128, clearArray: true); ``` ### Alternative Designs _No response_ ### Risks _No response_
Author: universorum
Assignees: -
Labels: `api-suggestion`, `area-System.Buffers`
Milestone: -
jeffhandley commented 7 months ago

This proposal looks good to me as-is.

@jkotas Can you think of any risks worth citing on this?

jkotas commented 7 months ago