-
Notifications
You must be signed in to change notification settings - Fork 4.9k
Add MemoryPool apis #26563
Add MemoryPool apis #26563
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
// See the LICENSE file in the project root for more information. | ||
|
||
using System.Runtime.InteropServices; | ||
#if !netstandard | ||
using Internal.Runtime.CompilerServices; | ||
#else | ||
using System.Runtime.CompilerServices; | ||
#endif | ||
|
||
namespace System.Buffers | ||
{ | ||
internal sealed partial class ArrayMemoryPool<T> : MemoryPool<T> | ||
{ | ||
private sealed class ArrayMemoryPoolBuffer : OwnedMemory<T> | ||
{ | ||
private T[] _array; | ||
private int _refCount; | ||
|
||
public ArrayMemoryPoolBuffer(int size) | ||
{ | ||
_array = ArrayPool<T>.Shared.Rent(size); | ||
} | ||
|
||
public sealed override int Length => _array.Length; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do properties and methods on a sealed class need to marked as sealed as well? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
No |
||
|
||
public sealed override bool IsDisposed => _array == null; | ||
|
||
protected sealed override bool IsRetained => _refCount > 0; | ||
|
||
public sealed override Span<T> Span | ||
{ | ||
get | ||
{ | ||
if (IsDisposed) | ||
ThrowHelper.ThrowObjectDisposedException_ArrayMemoryPoolBuffer(); | ||
|
||
return _array; | ||
} | ||
} | ||
|
||
protected sealed override void Dispose(bool disposing) | ||
{ | ||
if (_array != null) | ||
{ | ||
ArrayPool<T>.Shared.Return(_array); | ||
_array = null; | ||
} | ||
} | ||
|
||
protected | ||
#if netstandard // TryGetArray is exposed as "protected internal". Normally, the rules of C# dictate we override it as "protected" because the base class is | ||
// in a different assembly. Except in the netstandard config where the base class is in the same assembly. | ||
internal | ||
#endif | ||
sealed override bool TryGetArray(out ArraySegment<T> arraySegment) | ||
{ | ||
if (IsDisposed) | ||
ThrowHelper.ThrowObjectDisposedException_ArrayMemoryPoolBuffer(); | ||
|
||
arraySegment = new ArraySegment<T>(_array); | ||
return true; | ||
} | ||
|
||
public sealed override MemoryHandle Pin(int byteOffset = 0) | ||
{ | ||
unsafe | ||
{ | ||
Retain(); // this checks IsDisposed | ||
|
||
if (byteOffset != 0 && (((uint)byteOffset) - 1) / Unsafe.SizeOf<T>() >= _array.Length) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should allow byteOffset that points right at the end of the array. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (See discussion at #25770 (comment)) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It does allow that. We have a specific test for it: https://github.com/AtsushiKan/corefx/blob/mempool/src/System.Memory/tests/MemoryPool/MemoryPool.cs#L96 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, you are right. |
||
ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.byteOffset); | ||
|
||
GCHandle handle = GCHandle.Alloc(_array, GCHandleType.Pinned); | ||
return new MemoryHandle(this, ((byte*)handle.AddrOfPinnedObject()) + byteOffset, handle); | ||
} | ||
} | ||
|
||
public sealed override void Retain() | ||
{ | ||
if (IsDisposed) | ||
ThrowHelper.ThrowObjectDisposedException_ArrayMemoryPoolBuffer(); | ||
|
||
_refCount++; | ||
} | ||
|
||
public sealed override bool Release() | ||
{ | ||
if (IsDisposed) | ||
ThrowHelper.ThrowObjectDisposedException_ArrayMemoryPoolBuffer(); | ||
|
||
int newRefCount = --_refCount; | ||
if (newRefCount < 0) | ||
ThrowHelper.ThrowInvalidOperationException(); | ||
|
||
return newRefCount != 0; | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
// See the LICENSE file in the project root for more information. | ||
|
||
#if !netstandard | ||
using Internal.Runtime.CompilerServices; | ||
#else | ||
using System.Runtime.CompilerServices; | ||
#endif | ||
|
||
namespace System.Buffers | ||
{ | ||
internal sealed partial class ArrayMemoryPool<T> : MemoryPool<T> | ||
{ | ||
private const int s_maxBufferSize = int.MaxValue; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: s_ prefix is used for statics; consts should be PascalCased. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know - this is kinda tough case since the "best" name is already taken by the property. MaxBufferSizeConst just sounds awkward. |
||
public sealed override int MaxBufferSize => s_maxBufferSize; | ||
|
||
public sealed override OwnedMemory<T> Rent(int minimumBufferSize = -1) | ||
{ | ||
if (minimumBufferSize == -1) | ||
minimumBufferSize = 1 + (4095 / Unsafe.SizeOf<T>()); | ||
else if (((uint)minimumBufferSize) > s_maxBufferSize) | ||
ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.minimumBufferSize); | ||
|
||
return new ArrayMemoryPoolBuffer(minimumBufferSize); | ||
} | ||
|
||
protected sealed override void Dispose(bool disposing) {} // ArrayMemoryPool is a shared pool so Dispose() would be a nop even if there were native resources to dispose. | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
// See the LICENSE file in the project root for more information. | ||
|
||
namespace System.Buffers | ||
{ | ||
/// <summary> | ||
/// Represents a pool of memory blocks. | ||
/// </summary> | ||
public abstract class MemoryPool<T> : IDisposable | ||
{ | ||
private static readonly MemoryPool<T> s_shared = new ArrayMemoryPool<T>(); | ||
|
||
/// <summary> | ||
/// Returns a singleton instance of a MemoryPool based on arrays. | ||
/// </summary> | ||
public static MemoryPool<T> Shared => s_shared; | ||
|
||
/// <summary> | ||
/// Returns a memory block capable of holding at least <paramref name="minBufferSize" /> elements of T. | ||
/// </summary> | ||
/// <param name="minBufferSize">If -1 is passed, this is set to a default value for the pool.</param> | ||
public abstract OwnedMemory<T> Rent(int minBufferSize = -1); | ||
|
||
/// <summary> | ||
/// Returns the maximum buffer size supported by this pool. | ||
/// </summary> | ||
public abstract int MaxBufferSize { get; } | ||
|
||
/// <summary> | ||
/// Constructs a new instance of a memory pool. | ||
/// </summary> | ||
protected MemoryPool() {} | ||
|
||
/// <summary> | ||
/// Frees all resources used by the memory pool. | ||
/// </summary> | ||
public void Dispose() | ||
{ | ||
Dispose(true); | ||
GC.SuppressFinalize(this); | ||
} | ||
|
||
/// <summary> | ||
/// Frees all resources used by the memory pool. | ||
/// </summary> | ||
/// <param name="disposing"></param> | ||
protected abstract void Dispose(bool disposing); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We had a discussion at the API review about the default here. Did we really settle on -1? Why not have the default be 0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, ArrayPool.Shared.Rent(0) gives back an empty array. Then again, it also throws for -1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC, we have settled on -1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's on youtube, it was -1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you mean "it's on the internet"? :-)