Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Allow ArrayPool.Shared devirtualization (redux) #20637

Merged
merged 3 commits into from
Oct 26, 2018
Merged

Allow ArrayPool.Shared devirtualization (redux) #20637

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fcfe2a4
Allow ArrayPool.Shared devirtualization (redux)
benaadams Oct 26, 2018
551c318
readonly static
benaadams Oct 26, 2018
05d28dd
Fix property
benaadams Oct 26, 2018
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
6 changes: 5 additions & 1 deletion src/System.Private.CoreLib/shared/System/Buffers/ArrayPool.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ namespace System.Buffers
/// </remarks>
public abstract class ArrayPool<T>
{
// Store the shared ArrayPool in a field of its derived sealed type so the Jit can "see" the exact type
// when the Shared property is inlined which will allow it to devirtualize calls made on it.
private readonly static TlsOverPerCoreLockedStacksArrayPool<T> s_shared = new TlsOverPerCoreLockedStacksArrayPool<T>();
Copy link
Member

Choose a reason for hiding this comment

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

In the case of a reference type (or more generally where no boxing or conversion is needed), it seems like the C# compiler could be improved here slightly to work better with devirtualization to make the automatic backing field be of the type of the expression on the RHS rather than the type of the property, e.g.

public class C
{
    public static A Shared { get; } = new B();
}
public abstract class A { }
public sealed class B : A { }

produces

    private static readonly A <Shared>k__BackingField = new B();
    public static A Shared => <Shared>k__BackingField;

but it could instead do:

    private static readonly B <Shared>k__BackingField = new B();
    public static A Shared => <Shared>k__BackingField;

cc: @jaredpar

Copy link
Member

Choose a reason for hiding this comment

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

I think that could only be done when there was an identity preserving conversion. This couldn't be done for instance when the conversion from B to A was boxing, user defined, etc ... Otherwise that change would be quite observable.

Copy link
Member

Choose a reason for hiding this comment

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

Yes. (I tried to note that in my description but maybe I used the wrong words :))

Copy link
Member

Choose a reason for hiding this comment

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

It's Friday, my brain is mush. I didn't read your description carefully enough 😦

Copy link
Member Author

@benaadams benaadams Oct 27, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Capturing link back from proposal dotnet/roslyn#30797


/// <summary>
/// Retrieves a shared <see cref="ArrayPool{T}"/> instance.
/// </summary>
Expand All @@ -33,7 +37,7 @@ public abstract class ArrayPool<T>
/// optimized for very fast access speeds, at the expense of more memory consumption.
/// The shared pool instance is created lazily on first access.
/// </remarks>
public static ArrayPool<T> Shared { get; } = new TlsOverPerCoreLockedStacksArrayPool<T>();
public static ArrayPool<T> Shared => s_shared;

/// <summary>
/// Creates a new <see cref="ArrayPool{T}"/> instance using default configuration options.
Expand Down