Allow ArrayPool.Shared devirtualization (redux)

[benaadams]

Follow up to #20503

[jkotas]

This should be also readonly. (The JIT does not take advantage of readonly for object references right now, but it can in future.)

[AndyAyersMS]

Note we will need jit changes (not yet PR'd, but coming very soon) to light this up.

[AndyAyersMS]

Provisional jit changes here; based on top of #20553.

[jkotas]

Does this need to be just public static ArrayPool<T> Shared => s_shared;?

I expect that Shared { get; } = will create a hidden backing field of the wrong type.

[benaadams]

Ha! Definately

[AndyAyersMS]

Yes...

[jkotas]

Thanks

[stephentoub]

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

[jaredpar]

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.

[stephentoub]

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

[jaredpar]

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

[benaadams]

Capturing link back from proposal dotnet/roslyn#30797

[AndyAyersMS]

This change, on top of #20533, is enough to enable devirtualization. The jit change I have in the works isn't needed. However it improves code size so I'll go ahead and PR that too.

PMI diffs show a modest number of hits ...

CROSSGEN

Total bytes of diff: -191 (0.00% of base)
    diff is an improvement.

Top file improvements by size (bytes):
        -191 : System.Private.CoreLib.dasm (-0.01% of base)

1 total files with size differences (1 improved, 0 regressed), 128 unchanged.

Top method improvements by size (bytes):
         -15 (-3.09% of base) : System.Private.CoreLib.dasm - RegistryKey:GetValueNames():ref:this
         -12 (-1.77% of base) : System.Private.CoreLib.dasm - BinaryWriter:Write(struct):this (3 methods)
          -9 (-2.38% of base) : System.Private.CoreLib.dasm - RegistryKey:GetSubKeyNames():ref:this
          -9 (-2.25% of base) : System.Private.CoreLib.dasm - Environment:GetEnvironmentVariableCoreHelper(ref,struct):ref
          -9 (-3.67% of base) : System.Private.CoreLib.dasm - Stream:CopyTo(ref,int):this

Top method improvements by size (percentage):
          -9 (-3.67% of base) : System.Private.CoreLib.dasm - Stream:CopyTo(ref,int):this
         -15 (-3.09% of base) : System.Private.CoreLib.dasm - RegistryKey:GetValueNames():ref:this
          -6 (-2.71% of base) : System.Private.CoreLib.dasm - ValueStringBuilder:TryCopyTo(struct,byref):bool:this
          -2 (-2.63% of base) : System.Private.CoreLib.dasm - ValueStringBuilder:Dispose():this
          -9 (-2.38% of base) : System.Private.CoreLib.dasm - RegistryKey:GetSubKeyNames():ref:this

33 total methods with size differences (33 improved, 0 regressed), 144984 unchanged.

PMI

Total bytes of diff: -1108 (0.00% of base)
    diff is an improvement.

Top file improvements by size (bytes):
        -246 : System.Security.Cryptography.Algorithms.dasm (-0.09% of base)
        -213 : System.Private.CoreLib.dasm (-0.01% of base)
        -129 : System.Security.Cryptography.Cng.dasm (-0.08% of base)
         -93 : System.Security.Cryptography.Primitives.dasm (-0.31% of base)
         -77 : System.IO.FileSystem.dasm (-0.08% of base)

19 total files with size differences (19 improved, 0 regressed), 110 unchanged.

Top method regressions by size (bytes):
           4 ( 0.90% of base) : System.Memory.dasm - ArrayMemoryPoolBuffer:Dispose():this (5 methods)
           1 ( 1.04% of base) : System.Console.dasm - ValueStringBuilder:Dispose():this
           1 ( 1.04% of base) : System.IO.FileSystem.dasm - ValueStringBuilder:Dispose():this
           1 ( 0.87% of base) : System.Net.Security.dasm - SslStreamInternal:Dispose(bool):this
           1 ( 1.19% of base) : System.Net.WebSockets.dasm - ManagedWebSocket:ReleaseSendBuffer():this

Top method improvements by size (bytes):
         -45 (-1.54% of base) : System.Security.Cryptography.Primitives.dasm - AsymmetricAlgorithm:ExportArray(struct,ref,ref):ref (5 methods)
         -18 (-1.25% of base) : System.Net.Sockets.dasm - SocketPal:Select(ref,ref,ref,int):int
         -18 (-1.06% of base) : System.Security.Cryptography.Algorithms.dasm - CngPkcs8:RewriteEncryptedPkcs8PrivateKey(ref,struct,ref):ref (2 methods)
         -18 (-1.06% of base) : System.Security.Cryptography.Cng.dasm - CngPkcs8:RewriteEncryptedPkcs8PrivateKey(ref,struct,ref):ref (2 methods)
         -15 (-2.27% of base) : Microsoft.Win32.Registry.dasm - RegistryKey:GetValueNamesCore(int):ref:this

Top method regressions by size (percentage):
           1 ( 1.19% of base) : System.Net.WebSockets.dasm - ManagedWebSocket:ReleaseSendBuffer():this
           1 ( 1.16% of base) : System.Private.CoreLib.dasm - ValueStringBuilder:Dispose():this
           1 ( 1.04% of base) : System.Console.dasm - ValueStringBuilder:Dispose():this
           1 ( 1.04% of base) : System.IO.FileSystem.dasm - ValueStringBuilder:Dispose():this
           1 ( 1.04% of base) : System.Runtime.Extensions.dasm - ValueStringBuilder:Dispose():this

Top method improvements by size (percentage):
          -6 (-4.80% of base) : System.IO.Compression.ZipFile.dasm - ZipFileUtils:EnsureCapacity(byref,int)
          -3 (-4.29% of base) : System.Net.Http.dasm - LimitArrayPoolWriteStream:Dispose(bool):this
          -6 (-4.11% of base) : System.Text.RegularExpressions.dasm - RegexWriter:Dispose():this
          -3 (-4.00% of base) : System.IO.Compression.dasm - DeflateStream:InitializeBuffer():this
          -3 (-4.00% of base) : System.Text.RegularExpressions.dasm - RegexFCD:Dispose():this

178 total methods with size differences (169 improved, 9 regressed), 192515 unchanged.

[AndyAyersMS]

Ubuntu failure is the recently-common xunit assembly load issue (#20392)

12:20:11   xUnit.net Console Runner v2.4.1-pre.build.4059 (64-bit .NET Core 4.6.27026.0)
12:20:11   System.IO.FileLoadException: Could not load file or assembly 'readytorun.DynamicMethodGCStress.XUnitWrapper, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'. An operation is not legal in the current state. (Exception from HRESULT: 0x80131509 (COR_E_INVALIDOPERATION))
12:20:11   System.IO.FileLoadException: Could not load file or assembly 'baseservices.compilerservices.XUnitWrapper, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'. An operation is not legal in the current state. (Exception from HRESULT: 0x80131509 (COR_E_INVALIDOPERATION))

Has anyone tried tracking this down?