From f87610a42be8d6d95bc49b846d98c627e5ce9f36 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Fri, 7 Apr 2023 15:02:26 +0200 Subject: [PATCH 1/3] Switch source generator cache to ArrayPool --- ...ityToolkit.Mvvm.SourceGenerators.projitems | 1 - .../Helpers/ImmutableArrayBuilder{T}.cs | 61 +++---- .../Helpers/ObjectPool{T}.cs | 163 ------------------ 3 files changed, 23 insertions(+), 202 deletions(-) delete mode 100644 src/CommunityToolkit.Mvvm.SourceGenerators/Helpers/ObjectPool{T}.cs diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/CommunityToolkit.Mvvm.SourceGenerators.projitems b/src/CommunityToolkit.Mvvm.SourceGenerators/CommunityToolkit.Mvvm.SourceGenerators.projitems index 87939916..401916e3 100644 --- a/src/CommunityToolkit.Mvvm.SourceGenerators/CommunityToolkit.Mvvm.SourceGenerators.projitems +++ b/src/CommunityToolkit.Mvvm.SourceGenerators/CommunityToolkit.Mvvm.SourceGenerators.projitems @@ -67,7 +67,6 @@ - diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/Helpers/ImmutableArrayBuilder{T}.cs b/src/CommunityToolkit.Mvvm.SourceGenerators/Helpers/ImmutableArrayBuilder{T}.cs index b7982d70..91e2f9bd 100644 --- a/src/CommunityToolkit.Mvvm.SourceGenerators/Helpers/ImmutableArrayBuilder{T}.cs +++ b/src/CommunityToolkit.Mvvm.SourceGenerators/Helpers/ImmutableArrayBuilder{T}.cs @@ -6,6 +6,7 @@ // more info in ThirdPartyNotices.txt in the root of the project. using System; +using System.Buffers; using System.Collections.Immutable; using System.Runtime.CompilerServices; @@ -17,11 +18,6 @@ namespace CommunityToolkit.Mvvm.SourceGenerators.Helpers; /// The type of items to create sequences for. internal struct ImmutableArrayBuilder : IDisposable { - /// - /// The shared instance to share objects. - /// - private static readonly ObjectPool SharedObjectPool = new(static () => new Writer()); - /// /// The rented instance to use. /// @@ -33,7 +29,7 @@ internal struct ImmutableArrayBuilder : IDisposable /// A instance to write data to. public static ImmutableArrayBuilder Rent() { - return new(SharedObjectPool.Allocate()); + return new(new Writer()); } /// @@ -103,23 +99,18 @@ public void Dispose() this.writer = null; - if (writer is not null) - { - writer.Clear(); - - SharedObjectPool.Free(writer); - } + writer?.Dispose(); } /// /// A class handling the actual buffer writing. /// - private sealed class Writer + private sealed class Writer : IDisposable { /// /// The underlying array. /// - private T[] array; + private T?[]? array; /// /// The starting offset within . @@ -131,15 +122,7 @@ private sealed class Writer /// public Writer() { - if (typeof(T) == typeof(char)) - { - this.array = new T[1024]; - } - else - { - this.array = new T[8]; - } - + this.array = ArrayPool.Shared.Rent(typeof(T) == typeof(char) ? 1024 : 8); this.index = 0; } @@ -154,7 +137,7 @@ public int Count public ReadOnlySpan WrittenSpan { [MethodImpl(MethodImplOptions.AggressiveInlining)] - get => new(this.array, 0, this.index); + get => new(this.array!, 0, this.index); } /// @@ -162,7 +145,7 @@ public void Add(T value) { EnsureCapacity(1); - this.array[this.index++] = value; + this.array![this.index++] = value; } /// @@ -170,22 +153,22 @@ public void AddRange(ReadOnlySpan items) { EnsureCapacity(items.Length); - items.CopyTo(this.array.AsSpan(this.index)); + items.CopyTo(this.array.AsSpan(this.index)!); this.index += items.Length; } - /// - /// Clears the items in the current writer. - /// - public void Clear() + /// + public void Dispose() { - if (typeof(T) != typeof(char)) + T?[]? array = this.array; + + this.array = null; + + if (array is not null) { - this.array.AsSpan(0, this.index).Clear(); + ArrayPool.Shared.Return(array, clearArray: typeof(T) != typeof(char)); } - - this.index = 0; } /// @@ -195,7 +178,7 @@ public void Clear() [MethodImpl(MethodImplOptions.AggressiveInlining)] private void EnsureCapacity(int requestedSize) { - if (requestedSize > this.array.Length - this.index) + if (requestedSize > this.array!.Length - this.index) { ResizeBuffer(requestedSize); } @@ -209,13 +192,15 @@ private void EnsureCapacity(int requestedSize) private void ResizeBuffer(int sizeHint) { int minimumSize = this.index + sizeHint; - int requestedSize = Math.Max(this.array.Length * 2, minimumSize); - T[] newArray = new T[requestedSize]; + T?[] oldArray = this.array!; + T?[] newArray = ArrayPool.Shared.Rent(minimumSize); - Array.Copy(this.array, newArray, this.index); + Array.Copy(oldArray, newArray, this.index); this.array = newArray; + + ArrayPool.Shared.Return(oldArray, clearArray: typeof(T) != typeof(char)); } } } diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/Helpers/ObjectPool{T}.cs b/src/CommunityToolkit.Mvvm.SourceGenerators/Helpers/ObjectPool{T}.cs deleted file mode 100644 index 73e26d42..00000000 --- a/src/CommunityToolkit.Mvvm.SourceGenerators/Helpers/ObjectPool{T}.cs +++ /dev/null @@ -1,163 +0,0 @@ -// 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. - -// Ported from Roslyn, see: https://github.com/dotnet/roslyn/blob/main/src/Dependencies/PooledObjects/ObjectPool%601.cs. - -using System; -using System.Runtime.CompilerServices; -using System.Threading; - -namespace CommunityToolkit.Mvvm.SourceGenerators.Helpers; - -/// -/// -/// Generic implementation of object pooling pattern with predefined pool size limit. The main purpose -/// is that limited number of frequently used objects can be kept in the pool for further recycling. -/// -/// -/// Notes: -/// -/// -/// It is not the goal to keep all returned objects. Pool is not meant for storage. If there -/// is no space in the pool, extra returned objects will be dropped. -/// -/// -/// It is implied that if object was obtained from a pool, the caller will return it back in -/// a relatively short time. Keeping checked out objects for long durations is ok, but -/// reduces usefulness of pooling. Just new up your own. -/// -/// -/// -/// -/// Not returning objects to the pool in not detrimental to the pool's work, but is a bad practice. -/// Rationale: if there is no intent for reusing the object, do not use pool - just use "new". -/// -/// -/// The type of objects to pool. -internal sealed class ObjectPool - where T : class -{ - /// - /// The factory is stored for the lifetime of the pool. We will call this only when pool needs to - /// expand. compared to "new T()", Func gives more flexibility to implementers and faster than "new T()". - /// - private readonly Func factory; - - /// - /// The array of cached items. - /// - private readonly Element[] items; - - /// - /// Storage for the pool objects. The first item is stored in a dedicated field - /// because we expect to be able to satisfy most requests from it. - /// - private T? firstItem; - - /// - /// Creates a new instance with the specified parameters. - /// - /// The input factory to produce items. - public ObjectPool(Func factory) - : this(factory, Environment.ProcessorCount * 2) - { - } - - /// - /// Creates a new instance with the specified parameters. - /// - /// The input factory to produce items. - /// The pool size to use. - public ObjectPool(Func factory, int size) - { - this.factory = factory; - this.items = new Element[size - 1]; - } - - /// - /// Produces a instance. - /// - /// The returned item to use. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public T Allocate() - { - T? item = this.firstItem; - - if (item is null || item != Interlocked.CompareExchange(ref this.firstItem, null, item)) - { - item = AllocateSlow(); - } - - return item; - } - - /// - /// Returns a given instance to the pool. - /// - /// The instance to return. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public void Free(T obj) - { - if (this.firstItem is null) - { - this.firstItem = obj; - } - else - { - FreeSlow(obj); - } - } - - /// - /// Allocates a new item. - /// - /// The returned item to use. - [MethodImpl(MethodImplOptions.NoInlining)] - private T AllocateSlow() - { - foreach (ref Element element in this.items.AsSpan()) - { - T? instance = element.Value; - - if (instance is not null) - { - if (instance == Interlocked.CompareExchange(ref element.Value, null, instance)) - { - return instance; - } - } - } - - return this.factory(); - } - - /// - /// Frees a given item. - /// - /// The item to return to the pool. - [MethodImpl(MethodImplOptions.NoInlining)] - private void FreeSlow(T obj) - { - foreach (ref Element element in this.items.AsSpan()) - { - if (element.Value is null) - { - element.Value = obj; - - break; - } - } - } - - /// - /// A container for a produced item (using a wrapper to avoid covariance checks). - /// - private struct Element - { - /// - /// The value held at the current element. - /// - internal T? Value; - } -} \ No newline at end of file From f5922687cc79111246e486d7576c96417e5b5a94 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Tue, 16 May 2023 14:14:02 +0200 Subject: [PATCH 2/3] Add ref scope annotations to ImmutableArrayBuilder --- .../Helpers/ImmutableArrayBuilder{T}.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/Helpers/ImmutableArrayBuilder{T}.cs b/src/CommunityToolkit.Mvvm.SourceGenerators/Helpers/ImmutableArrayBuilder{T}.cs index 91e2f9bd..10503a1e 100644 --- a/src/CommunityToolkit.Mvvm.SourceGenerators/Helpers/ImmutableArrayBuilder{T}.cs +++ b/src/CommunityToolkit.Mvvm.SourceGenerators/Helpers/ImmutableArrayBuilder{T}.cs @@ -8,6 +8,7 @@ using System; using System.Buffers; using System.Collections.Immutable; +using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; namespace CommunityToolkit.Mvvm.SourceGenerators.Helpers; @@ -16,7 +17,7 @@ namespace CommunityToolkit.Mvvm.SourceGenerators.Helpers; /// A helper type to build sequences of values with pooled buffers. /// /// The type of items to create sequences for. -internal struct ImmutableArrayBuilder : IDisposable +internal ref struct ImmutableArrayBuilder { /// /// The rented instance to use. @@ -51,6 +52,7 @@ public int Count /// /// Gets the data written to the underlying buffer so far, as a . /// + [UnscopedRef] public readonly ReadOnlySpan WrittenSpan { [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -67,7 +69,7 @@ public readonly void Add(T item) /// Adds the specified items to the end of the array. /// /// The items to add at the end of the array. - public readonly void AddRange(ReadOnlySpan items) + public readonly void AddRange(scoped ReadOnlySpan items) { this.writer!.AddRange(items); } @@ -92,7 +94,7 @@ public override readonly string ToString() return this.writer!.WrittenSpan.ToString(); } - /// + /// public void Dispose() { Writer? writer = this.writer; From d2f77a949729291abc266be6944668cd5edbedd7 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Tue, 16 May 2023 14:20:33 +0200 Subject: [PATCH 3/3] Add missing readonly modifier --- .../Helpers/ImmutableArrayBuilder{T}.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/Helpers/ImmutableArrayBuilder{T}.cs b/src/CommunityToolkit.Mvvm.SourceGenerators/Helpers/ImmutableArrayBuilder{T}.cs index 10503a1e..2cb48495 100644 --- a/src/CommunityToolkit.Mvvm.SourceGenerators/Helpers/ImmutableArrayBuilder{T}.cs +++ b/src/CommunityToolkit.Mvvm.SourceGenerators/Helpers/ImmutableArrayBuilder{T}.cs @@ -43,7 +43,7 @@ private ImmutableArrayBuilder(Writer writer) } /// - public int Count + public readonly int Count { [MethodImpl(MethodImplOptions.AggressiveInlining)] get => this.writer!.Count;