Use Roslyn support for RuntimeHelpers.CreateSpan #70179

stephentoub · 2022-06-02T22:07:50Z

As of dotnet/roslyn#61414, the optimization that previously extended only to byte/sbyte/bool will now also support char/ushort/short/uint/int/ulong/long/float/double. This PR uses it throughout dotnet/runtime. However, because of endianness issues, this requires the RuntimeHelpers.CreateSpan method that only exists as of .NET 7, so trying to use this optimization in builds targeting < .NET 7 will be a significant deoptimization. Until we have dedicated syntax (ala dotnet/csharplang#5295) and/or an analyzer (#69577) to avoid such issues, we need to be very careful where this applied.

I'm putting this up as a draft now for feedback, but we must not merge this until we've picked up a Roslyn compiler that has the aforementioned improvement.

Closes #60948
cc: @VSadov, @jcouv, @davidwrighton, @GrabYourPitchforks

dotnet-issue-labeler · 2022-06-02T22:07:58Z

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

src/libraries/Common/src/System/Drawing/KnownColorTable.cs

src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLGuid.cs

huoyaoyuan · 2022-06-03T07:42:10Z

Current implementation throws hardly for big endian. It still allows the option to copy the field and reverse the endianness in memory, right? Just make sure this feature is not impossible or extremely inefficient to implement for big endian.

stephentoub · 2022-06-03T11:00:00Z

Current implementation throws hardly for big endian. It still allows the option to copy the field and reverse the endianness in memory, right? Just make sure this feature is not impossible or extremely inefficient to implement for big endian.

The purpose of the CreateSpan API is to address endianness. coreclr doesn't support big endian at all today (well beyond this particular method), but mono does, including with this method.

runtime/src/mono/mono/metadata/icall.c

Line 960 in 5adac4a

#if G_BYTE_ORDER != G_LITTLE_ENDIAN

ghost · 2022-06-03T11:31:42Z

Tagging subscribers to this area: @dotnet/area-meta
See info in area-owners.md if you want to be subscribed.

Issue Details

As of dotnet/roslyn#61414, the optimization that previously extended only to byte/sbyte/bool will now also support char/ushort/short/uint/int/ulong/long/float/double. This PR uses it throughout dotnet/runtime. However, because of endianness issues, this requires the RuntimeHelpers.CreateSpan method that only exists as of .NET 7, so trying to use this optimization in builds targeting < .NET 7 will be a significant deoptimization. Until we have dedicated syntax (ala dotnet/csharplang#5295) and/or an analyzer (#69577) to avoid such issues, we need to be very careful where this applied.

I'm putting this up as a draft now for feedback, but we must not merge this until we've picked up a Roslyn compiler that has the aforementioned improvement.

Closes #60948
cc: @VSadov, @jcouv, @davidwrighton, @GrabYourPitchforks

Author:	stephentoub
Assignees:	stephentoub
Labels:	`NO-MERGE`, `area-Meta`, `tenet-performance`
Milestone:	7.0.0

src/libraries/System.Private.Xml/src/System/Xml/Xsl/XPath/XPathParser.cs

src/libraries/System.Private.CoreLib/src/System/Globalization/EastAsianLunisolarCalendar.cs

src/libraries/System.Private.CoreLib/src/System/Reflection/AssemblyNameParser.cs

src/libraries/System.Text.RegularExpressions/tools/DataTable.cs

src/libraries/System.Net.HttpListener/src/System/Net/HttpListenerResponse.cs

marek-safar · 2022-06-17T11:38:07Z

src/libraries/System.Private.CoreLib/src/System/Globalization/DateTimeParse.cs

@@ -5455,12 +5453,14 @@ internal bool MatchSpecifiedWords(string target, bool checkWordBoundary, ref int
                // Check word by word
                int targetPosition = 0;                 // Where we are in the target string
                int thisPosition = Index;         // Where we are in this string
-                int wsIndex = target.IndexOfAny(WhiteSpaceChecks, targetPosition);


Wondering if there is analyzer for such patterns

The optimization that previously extended only to byte/sbyte/bool now also supports char/ushort/short/uint/int/ulong/long/float/double.

stephentoub · 2022-07-27T11:40:12Z

It looks like the compiler dependency won't land in time for the compiler change to be merged. Closing for .NET 7.

stephentoub added NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) tenet-performance Performance related issue labels Jun 2, 2022

stephentoub added this to the 7.0.0 milestone Jun 2, 2022

ghost assigned stephentoub Jun 2, 2022

stephentoub force-pushed the usecreatespan branch 3 times, most recently from dee4484 to 196281d Compare June 3, 2022 01:04