Skip to content

HighPerformance package improvements #3351

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
29 commits merged into from
Sep 24, 2020
Merged

HighPerformance package improvements #3351

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
b464ce2
Removed unnecessary attributes, improved codegen
Sergio0694 Jun 16, 2020
301b5fa
Added custom ArrayPool<T> field to Memoryowner<T>
Sergio0694 Jun 16, 2020
6bd669f
Added custom pool to ArrayPoolBufferWriter<T>
Sergio0694 Jun 16, 2020
d9b07d7
Minor codegen improvement to Box<T>
Sergio0694 Jun 16, 2020
a2217fa
Added custom pool to SpanwOwner<T>
Sergio0694 Jun 16, 2020
f4da592
Minor tweaks to ArrayPoolBufferWriter<T>
Sergio0694 Jun 16, 2020
1f6aed0
Added tests for invalid allocation sizes
Sergio0694 Jun 16, 2020
ae589c7
Added unit tests for custom array pool overloads
Sergio0694 Jun 16, 2020
a925554
Merge branch 'master' into improvement/high-performance-tweaks
Sergio0694 Jun 22, 2020
7e0d20a
Fixed bugs in two unit tests
Sergio0694 Jun 23, 2020
26b0a6d
Merge branch 'master' into improvement/high-performance-tweaks
Sergio0694 Jun 24, 2020
034a643
Merge branch 'master' into improvement/high-performance-tweaks
Sergio0694 Jun 26, 2020
5356623
Merge branch 'master' into improvement/high-performance-tweaks
Sergio0694 Jun 30, 2020
234dae7
Merge branch 'master' into improvement/high-performance-tweaks
Sergio0694 Jul 10, 2020
b22494c
Merge branch 'master' into improvement/high-performance-tweaks
Sergio0694 Jul 13, 2020
f31c2c7
Minor code style tweaks
Sergio0694 Jul 21, 2020
ae961dd
Minor codegen improvements
Sergio0694 Jul 21, 2020
54acdb3
Merge branch 'master' into improvement/high-performance-tweaks
Sergio0694 Jul 21, 2020
1438e39
Added more readonly modifiers to enumerators
Sergio0694 Jul 21, 2020
fbc3a4a
Merge branch 'master' into improvement/high-performance-tweaks
Sergio0694 Jul 31, 2020
8baecc2
Merge branch 'master' into improvement/high-performance-tweaks
Sergio0694 Aug 7, 2020
580eda5
Improved a comment, fixed a typo
Sergio0694 Aug 7, 2020
cf0f8aa
Merge branch 'master' into improvement/high-performance-tweaks
Sergio0694 Aug 8, 2020
84918ef
Merge branch 'master' into improvement/high-performance-tweaks
Sergio0694 Aug 12, 2020
ca46e0c
Added #ifdef block for unneeded using directive
Sergio0694 Sep 17, 2020
0c79764
Fixed incorrect XML docs
Sergio0694 Sep 17, 2020
49b1eba
Switched a Span<T>.CopyTo to Array.Copy
Sergio0694 Sep 18, 2020
7455284
Merge branch 'master' into improvement/high-performance-tweaks
Rosuavio Sep 23, 2020
036dc78
Merge branch 'master' into improvement/high-performance-tweaks
Sergio0694 Sep 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions Microsoft.Toolkit.HighPerformance/Box{T}.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ public static bool TryGetFrom(object obj, [NotNullWhen(true)] out Box<T>? box)
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static implicit operator T(Box<T> box)
{
return Unsafe.Unbox<T>(box);
return (T)(object)box;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am curious what the difference is here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory the codegen should be identical, but this version has a better IL encoding which should help with scenarios such as R2R when using crossgen (which has some inlining limitations with method calls), as well as cases where the indirect load might cause stack spills or just not be optimized best by the JIT (especially on older runtimes).

The difference is that the previous version basically invoked that generic Unsafe.Unbox<T> and then issued a ldobj !!T instruction to dereference that T& value on the stack, whereas the second is simply resolved into a single unbox.any !!T opcode, which is the same being used by normal unbox operations. This change is mostly just a precaution to help crossgen and the JIT to always produce the most optimal codegen possible, it has no actual functional changes.

If you're interested, I made a small repro you can check out (here) - you can see that the asm x64 codegen is exactly the same on .NET Core 3.1, but if you switch to the IL tab you'll be able to see that different encoding I mentioned 😊

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, that's pretty cool (the change and sharplab.io). Thanks @Sergio0694 😃

}

/// <summary>
Expand Down Expand Up @@ -180,7 +180,6 @@ public override int GetHashCode()
/// <summary>
/// Throws an <see cref="InvalidCastException"/> when a cast from an invalid <see cref="object"/> is attempted.
/// </summary>
[MethodImpl(MethodImplOptions.NoInlining)]
private static void ThrowInvalidCastExceptionForGetFrom()
{
throw new InvalidCastException($"Can't cast the input object to the type Box<{typeof(T)}>");
Expand Down
67 changes: 38 additions & 29 deletions Microsoft.Toolkit.HighPerformance/Buffers/ArrayPoolBufferWriter{T}.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ public sealed class ArrayPoolBufferWriter<T> : IBuffer<T>, IMemoryOwner<T>
/// </summary>
private const int DefaultInitialBufferSize = 256;

/// <summary>
/// The <see cref="ArrayPool{T}"/> instance used to rent <see cref="array"/>.
/// </summary>
private readonly ArrayPool<T> pool;

/// <summary>
/// The underlying <typeparamref name="T"/> array.
/// </summary>
Expand All @@ -49,36 +54,52 @@ public sealed class ArrayPoolBufferWriter<T> : IBuffer<T>, IMemoryOwner<T>
/// Initializes a new instance of the <see cref="ArrayPoolBufferWriter{T}"/> class.
/// </summary>
public ArrayPoolBufferWriter()
: this(ArrayPool<T>.Shared, DefaultInitialBufferSize)
{
}

/// <summary>
/// Initializes a new instance of the <see cref="ArrayPoolBufferWriter{T}"/> class.
/// </summary>
/// <param name="pool">The <see cref="ArrayPool{T}"/> instance to use.</param>
public ArrayPoolBufferWriter(ArrayPool<T> pool)
: this(pool, DefaultInitialBufferSize)
{
// Since we're using pooled arrays, we can rent the buffer with the
// default size immediately, we don't need to use lazy initialization
// to save unnecessary memory allocations in this case.
this.array = ArrayPool<T>.Shared.Rent(DefaultInitialBufferSize);
this.index = 0;
}

/// <summary>
/// Initializes a new instance of the <see cref="ArrayPoolBufferWriter{T}"/> class.
/// </summary>
/// <param name="initialCapacity">The minimum capacity with which to initialize the underlying buffer.</param>
/// <exception cref="ArgumentException">
/// Thrown when <paramref name="initialCapacity"/> is not positive (i.e. less than or equal to 0).
/// </exception>
/// <exception cref="ArgumentOutOfRangeException">Thrown when <paramref name="initialCapacity"/> is not valid.</exception>
public ArrayPoolBufferWriter(int initialCapacity)
: this(ArrayPool<T>.Shared, initialCapacity)
{
if (initialCapacity <= 0)
{
ThrowArgumentOutOfRangeExceptionForInitialCapacity();
}
}

this.array = ArrayPool<T>.Shared.Rent(initialCapacity);
/// <summary>
/// Initializes a new instance of the <see cref="ArrayPoolBufferWriter{T}"/> class.
/// </summary>
/// <param name="pool">The <see cref="ArrayPool{T}"/> instance to use.</param>
/// <param name="initialCapacity">The minimum capacity with which to initialize the underlying buffer.</param>
/// <exception cref="ArgumentOutOfRangeException">Thrown when <paramref name="initialCapacity"/> is not valid.</exception>
public ArrayPoolBufferWriter(ArrayPool<T> pool, int initialCapacity)
{
// Since we're using pooled arrays, we can rent the buffer with the
// default size immediately, we don't need to use lazy initialization
// to save unnecessary memory allocations in this case.
// Additionally, we don't need to manually throw the exception if
// the requested size is not valid, as that'll be thrown automatically
// by the array pool in use when we try to rent an array with that size.
this.pool = pool;
this.array = pool.Rent(initialCapacity);
this.index = 0;
}

/// <summary>
/// Finalizes an instance of the <see cref="ArrayPoolBufferWriter{T}"/> class.
/// </summary>
~ArrayPoolBufferWriter() => this.Dispose();
~ArrayPoolBufferWriter() => Dispose();

/// <inheritdoc/>
Memory<T> IMemoryOwner<T>.Memory
Expand Down Expand Up @@ -182,6 +203,7 @@ public void Clear()
}

array.AsSpan(0, this.index).Clear();

this.index = 0;
}

Expand Down Expand Up @@ -250,7 +272,7 @@ private void CheckAndResizeBuffer(int sizeHint)
{
int minimumSize = this.index + sizeHint;

ArrayPool<T>.Shared.Resize(ref this.array, minimumSize);
this.pool.Resize(ref this.array, minimumSize);
}
}

Expand All @@ -268,7 +290,7 @@ public void Dispose()

this.array = null;

ArrayPool<T>.Shared.Return(array);
this.pool.Return(array);
}

/// <inheritdoc/>
Expand All @@ -286,19 +308,9 @@ public override string ToString()
return $"Microsoft.Toolkit.HighPerformance.Buffers.ArrayPoolBufferWriter<{typeof(T)}>[{this.index}]";
}

/// <summary>
/// Throws an <see cref="ArgumentOutOfRangeException"/> when the initial capacity is invalid.
/// </summary>
[MethodImpl(MethodImplOptions.NoInlining)]
private static void ThrowArgumentOutOfRangeExceptionForInitialCapacity()
{
throw new ArgumentOutOfRangeException("initialCapacity", "The initial capacity must be a positive value");
}

/// <summary>
/// Throws an <see cref="ArgumentOutOfRangeException"/> when the requested count is negative.
/// </summary>
[MethodImpl(MethodImplOptions.NoInlining)]
private static void ThrowArgumentOutOfRangeExceptionForNegativeCount()
{
throw new ArgumentOutOfRangeException("count", "The count can't be a negative value");
Expand All @@ -307,7 +319,6 @@ private static void ThrowArgumentOutOfRangeExceptionForNegativeCount()
/// <summary>
/// Throws an <see cref="ArgumentOutOfRangeException"/> when the size hint is negative.
/// </summary>
[MethodImpl(MethodImplOptions.NoInlining)]
private static void ThrowArgumentOutOfRangeExceptionForNegativeSizeHint()
{
throw new ArgumentOutOfRangeException("sizeHint", "The size hint can't be a negative value");
Expand All @@ -316,7 +327,6 @@ private static void ThrowArgumentOutOfRangeExceptionForNegativeSizeHint()
/// <summary>
/// Throws an <see cref="ArgumentOutOfRangeException"/> when the requested count is negative.
/// </summary>
[MethodImpl(MethodImplOptions.NoInlining)]
private static void ThrowArgumentExceptionForAdvancedTooFar()
{
throw new ArgumentException("The buffer writer has advanced too far");
Expand All @@ -325,7 +335,6 @@ private static void ThrowArgumentExceptionForAdvancedTooFar()
/// <summary>
/// Throws an <see cref="ObjectDisposedException"/> when <see cref="array"/> is <see langword="null"/>.
/// </summary>
[MethodImpl(MethodImplOptions.NoInlining)]
private static void ThrowObjectDisposedException()
{
throw new ObjectDisposedException("The current buffer has already been disposed");
Expand Down
8 changes: 2 additions & 6 deletions Microsoft.Toolkit.HighPerformance/Buffers/MemoryBufferWriter{T}.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,15 +106,15 @@ public void Advance(int count)
/// <inheritdoc/>
public Memory<T> GetMemory(int sizeHint = 0)
{
this.ValidateSizeHint(sizeHint);
ValidateSizeHint(sizeHint);

return this.memory.Slice(this.index);
}

/// <inheritdoc/>
public Span<T> GetSpan(int sizeHint = 0)
{
this.ValidateSizeHint(sizeHint);
ValidateSizeHint(sizeHint);

return this.memory.Slice(this.index).Span;
}
Expand Down Expand Up @@ -159,7 +159,6 @@ public override string ToString()
/// <summary>
/// Throws an <see cref="ArgumentOutOfRangeException"/> when the requested count is negative.
/// </summary>
[MethodImpl(MethodImplOptions.NoInlining)]
private static void ThrowArgumentOutOfRangeExceptionForNegativeCount()
{
throw new ArgumentOutOfRangeException("count", "The count can't be a negative value");
Expand All @@ -168,7 +167,6 @@ private static void ThrowArgumentOutOfRangeExceptionForNegativeCount()
/// <summary>
/// Throws an <see cref="ArgumentOutOfRangeException"/> when the size hint is negative.
/// </summary>
[MethodImpl(MethodImplOptions.NoInlining)]
private static void ThrowArgumentOutOfRangeExceptionForNegativeSizeHint()
{
throw new ArgumentOutOfRangeException("sizeHint", "The size hint can't be a negative value");
Expand All @@ -177,7 +175,6 @@ private static void ThrowArgumentOutOfRangeExceptionForNegativeSizeHint()
/// <summary>
/// Throws an <see cref="ArgumentOutOfRangeException"/> when the requested count is negative.
/// </summary>
[MethodImpl(MethodImplOptions.NoInlining)]
private static void ThrowArgumentExceptionForAdvancedTooFar()
{
throw new ArgumentException("The buffer writer has advanced too far");
Expand All @@ -186,7 +183,6 @@ private static void ThrowArgumentExceptionForAdvancedTooFar()
/// <summary>
/// Throws an <see cref="ArgumentException"/> when the requested size exceeds the capacity.
/// </summary>
[MethodImpl(MethodImplOptions.NoInlining)]
private static void ThrowArgumentExceptionForCapacityExceeded()
{
throw new ArgumentException("The buffer writer doesn't have enough capacity left");
Expand Down
57 changes: 44 additions & 13 deletions Microsoft.Toolkit.HighPerformance/Buffers/MemoryOwner{T}.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ public sealed class MemoryOwner<T> : IMemoryOwner<T>
private readonly int length;
#pragma warning restore IDE0032

/// <summary>
/// The <see cref="ArrayPool{T}"/> instance used to rent <see cref="array"/>.
/// </summary>
private readonly ArrayPool<T> pool;

/// <summary>
/// The underlying <typeparamref name="T"/> array.
/// </summary>
Expand All @@ -41,12 +46,14 @@ public sealed class MemoryOwner<T> : IMemoryOwner<T>
/// Initializes a new instance of the <see cref="MemoryOwner{T}"/> class with the specified parameters.
/// </summary>
/// <param name="length">The length of the new memory buffer to use.</param>
/// <param name="pool">The <see cref="ArrayPool{T}"/> instance to use.</param>
/// <param name="mode">Indicates the allocation mode to use for the new buffer to rent.</param>
private MemoryOwner(int length, AllocationMode mode)
private MemoryOwner(int length, ArrayPool<T> pool, AllocationMode mode)
{
this.start = 0;
this.length = length;
this.array = ArrayPool<T>.Shared.Rent(length);
this.pool = pool;
this.array = pool.Rent(length);

if (mode == AllocationMode.Clear)
{
Expand All @@ -57,20 +64,22 @@ private MemoryOwner(int length, AllocationMode mode)
/// <summary>
/// Initializes a new instance of the <see cref="MemoryOwner{T}"/> class with the specified parameters.
/// </summary>
/// <param name="array">The input <typeparamref name="T"/> array to use.</param>
/// <param name="start">The starting offset within <paramref name="array"/>.</param>
/// <param name="length">The length of the array to use.</param>
private MemoryOwner(T[] array, int start, int length)
/// <param name="pool">The <see cref="ArrayPool{T}"/> instance currently in use.</param>
/// <param name="array">The input <typeparamref name="T"/> array to use.</param>
private MemoryOwner(int start, int length, ArrayPool<T> pool, T[] array)
{
this.start = start;
this.length = length;
this.pool = pool;
this.array = array;
}

/// <summary>
/// Finalizes an instance of the <see cref="MemoryOwner{T}"/> class.
/// </summary>
~MemoryOwner() => this.Dispose();
~MemoryOwner() => Dispose();

/// <summary>
/// Gets an empty <see cref="MemoryOwner{T}"/> instance.
Expand All @@ -79,7 +88,7 @@ private MemoryOwner(T[] array, int start, int length)
public static MemoryOwner<T> Empty
{
[MethodImpl(MethodImplOptions.AggressiveInlining)]
get => new MemoryOwner<T>(0, AllocationMode.Default);
get => new MemoryOwner<T>(0, ArrayPool<T>.Shared, AllocationMode.Default);
}

/// <summary>
Expand All @@ -91,19 +100,44 @@ public static MemoryOwner<T> Empty
/// <remarks>This method is just a proxy for the <see langword="private"/> constructor, for clarity.</remarks>
[Pure]
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static MemoryOwner<T> Allocate(int size) => new MemoryOwner<T>(size, AllocationMode.Default);
public static MemoryOwner<T> Allocate(int size) => new MemoryOwner<T>(size, ArrayPool<T>.Shared, AllocationMode.Default);

/// <summary>
/// Creates a new <see cref="MemoryOwner{T}"/> instance with the specified parameters.
/// </summary>
/// <param name="size">The length of the new memory buffer to use.</param>
/// <param name="pool">The <see cref="ArrayPool{T}"/> instance currently in use.</param>
/// <returns>A <see cref="MemoryOwner{T}"/> instance of the requested length.</returns>
/// <exception cref="ArgumentOutOfRangeException">Thrown when <paramref name="size"/> is not valid.</exception>
/// <remarks>This method is just a proxy for the <see langword="private"/> constructor, for clarity.</remarks>
[Pure]
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static MemoryOwner<T> Allocate(int size, ArrayPool<T> pool) => new MemoryOwner<T>(size, pool, AllocationMode.Default);

/// <summary>
/// Creates a new <see cref="MemoryOwner{T}"/> instance with the specified parameters.
/// </summary>
/// <param name="size">The length of the new memory buffer to use.</param>
/// <param name="mode">Indicates the allocation mode to use for the new buffer to rent.</param>
/// <returns>A <see cref="MemoryOwner{T}"/> instance of the requested length.</returns>
/// <exception cref="ArgumentOutOfRangeException">Thrown when <paramref name="size"/> is not valid.</exception>
/// <remarks>This method is just a proxy for the <see langword="private"/> constructor, for clarity.</remarks>
[Pure]
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static MemoryOwner<T> Allocate(int size, AllocationMode mode) => new MemoryOwner<T>(size, ArrayPool<T>.Shared, mode);

/// <summary>
/// Creates a new <see cref="MemoryOwner{T}"/> instance with the specified parameters.
/// </summary>
/// <param name="size">The length of the new memory buffer to use.</param>
/// <param name="pool">The <see cref="ArrayPool{T}"/> instance currently in use.</param>
/// <param name="mode">Indicates the allocation mode to use for the new buffer to rent.</param>
/// <returns>A <see cref="MemoryOwner{T}"/> instance of the requested length.</returns>
/// <exception cref="ArgumentOutOfRangeException">Thrown when <paramref name="size"/> is not valid.</exception>
/// <remarks>This method is just a proxy for the <see langword="private"/> constructor, for clarity.</remarks>
[Pure]
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static MemoryOwner<T> Allocate(int size, AllocationMode mode) => new MemoryOwner<T>(size, mode);
public static MemoryOwner<T> Allocate(int size, ArrayPool<T> pool, AllocationMode mode) => new MemoryOwner<T>(size, pool, mode);

/// <summary>
/// Gets the number of items in the current instance
Expand Down Expand Up @@ -210,7 +244,7 @@ public MemoryOwner<T> Slice(int start, int length)
ThrowInvalidLengthException();
}

return new MemoryOwner<T>(array!, start, length);
return new MemoryOwner<T>(start, length, this.pool, array!);
}

/// <inheritdoc/>
Expand All @@ -227,7 +261,7 @@ public void Dispose()

this.array = null;

ArrayPool<T>.Shared.Return(array);
this.pool.Return(array);
}

/// <inheritdoc/>
Expand All @@ -251,7 +285,6 @@ public override string ToString()
/// <summary>
/// Throws an <see cref="ObjectDisposedException"/> when <see cref="array"/> is <see langword="null"/>.
/// </summary>
[MethodImpl(MethodImplOptions.NoInlining)]
private static void ThrowObjectDisposedException()
{
throw new ObjectDisposedException(nameof(MemoryOwner<T>), "The current buffer has already been disposed");
Expand All @@ -260,7 +293,6 @@ private static void ThrowObjectDisposedException()
/// <summary>
/// Throws an <see cref="ArgumentOutOfRangeException"/> when the <see cref="start"/> is invalid.
/// </summary>
[MethodImpl(MethodImplOptions.NoInlining)]
private static void ThrowInvalidOffsetException()
{
throw new ArgumentOutOfRangeException(nameof(start), "The input start parameter was not valid");
Expand All @@ -269,7 +301,6 @@ private static void ThrowInvalidOffsetException()
/// <summary>
/// Throws an <see cref="ArgumentOutOfRangeException"/> when the <see cref="length"/> is invalid.
/// </summary>
[MethodImpl(MethodImplOptions.NoInlining)]
private static void ThrowInvalidLengthException()
{
throw new ArgumentOutOfRangeException(nameof(length), "The input length parameter was not valid");
Expand Down
Loading