Intrinsicify JsonReaderHelper.IndexOfOrLessThan #40877

benaadams · 2020-08-15T05:54:21Z

Update to use newer Sse2, Axv2 intrinsics; and add ARM as per the method it mimics (with changes) from SpanHelpers
Also add size optimization/simplification for wasm

x64 performance changes (up to +20% improvement)

better: 30, geomean: 1.113
worse: 1, geomean: 1.066
total diff: 31

| Slower                                                                           | diff/base | Base Median (ns) | Diff Median (ns) | Modality|
| -------------------------------------------------------------------------------- | ---------:| ----------------:| ----------------:| --------:|
| System.Text.Json.Serialization.Tests.ReadJson<SimpleStructWithProperties>.Deseri |      1.07 |           288.23 |           307.28 |         |

| Faster                                                                           | base/diff | Base Median (ns) | Diff Median (ns) | Modality|
| -------------------------------------------------------------------------------- | ---------:| ----------------:| ----------------:| --------:|
| System.Text.Json.Serialization.Tests.ReadJson<Dictionary<String, String>>.Deseri |      1.22 |         22031.18 |         18102.24 |         |
| System.Text.Json.Serialization.Tests.ReadJson<Dictionary<String, String>>.Deseri |      1.20 |         21554.46 |         17944.26 |         |
| System.Text.Json.Serialization.Tests.ReadJson<MyEventsListerViewModel>.Deseriali |      1.20 |        341085.15 |        284277.87 |         |
| System.Text.Json.Serialization.Tests.ReadJson<IndexViewModel>.DeserializeFromUtf |      1.19 |         29706.85 |         24903.98 |         |
| System.Text.Json.Serialization.Tests.ReadJson<MyEventsListerViewModel>.Deseriali |      1.19 |        330481.64 |        277947.73 |         |
| System.Text.Json.Serialization.Tests.ReadJson<MyEventsListerViewModel>.Deseriali |      1.19 |        399397.91 |        336085.63 |         |
| System.Text.Json.Serialization.Tests.ReadJson<IndexViewModel>.DeserializeFromStr |      1.18 |         31986.63 |         27064.34 |         |
| System.Text.Json.Serialization.Tests.ReadJson<Dictionary<String, String>>.Deseri |      1.18 |         23919.94 |         20268.43 |         |
| System.Text.Json.Serialization.Tests.ReadJson<IndexViewModel>.DeserializeFromStr |      1.18 |         31000.14 |         26348.41 |         |
| System.Text.Json.Serialization.Tests.ReadJson<LoginViewModel>.DeserializeFromStr |      1.15 |           522.04 |           454.92 |         |
| System.Text.Json.Serialization.Tests.ReadJson<Hashtable>.DeserializeFromUtf8Byte |      1.12 |         68993.04 |         61636.12 |         |
| System.Text.Json.Serialization.Tests.ReadJson<HashSet<String>>.DeserializeFromUt |      1.10 |         12663.49 |         11477.42 |         |
| System.Text.Json.Serialization.Tests.ReadJson<ImmutableSortedDictionary<String,  |      1.10 |         78887.97 |         71927.75 |         |
| System.Text.Json.Serialization.Tests.ReadJson<Hashtable>.DeserializeFromStream   |      1.10 |         69968.66 |         63797.29 |         |
| System.Text.Json.Serialization.Tests.ReadJson<LoginViewModel>.DeserializeFromStr |      1.09 |           763.93 |           698.34 |         |
| System.Text.Json.Serialization.Tests.ReadJson<HashSet<String>>.DeserializeFromSt |      1.09 |         12745.33 |         11734.38 |         |
| System.Text.Json.Serialization.Tests.ReadJson<ImmutableDictionary<String, String |      1.09 |         48372.57 |         44572.25 |         |
| System.Text.Json.Serialization.Tests.ReadJson<ArrayList>.DeserializeFromStream   |      1.09 |         51673.60 |         47617.28 |         |
| System.Text.Json.Serialization.Tests.ReadJson<ImmutableDictionary<String, String |      1.08 |         51226.78 |         47249.65 |         |
| System.Text.Json.Serialization.Tests.ReadJson<HashSet<String>>.DeserializeFromSt |      1.08 |         13979.59 |         12973.07 |         |
| System.Text.Json.Serialization.Tests.ReadJson<ImmutableDictionary<String, String |      1.08 |         48102.61 |         44652.30 |         |
| System.Text.Json.Serialization.Tests.ReadJson<BinaryData>.DeserializeFromStream  |      1.07 |           807.95 |           755.26 |         |
| System.Text.Json.Serialization.Tests.ReadJson<ArrayList>.DeserializeFromString   |      1.07 |         49947.46 |         46735.64 |         |
| System.Text.Json.Serialization.Tests.ReadJson<BinaryData>.DeserializeFromUtf8Byt |      1.07 |           488.59 |           458.07 |         |
| System.Text.Json.Serialization.Tests.ReadJson<LoginViewModel>.DeserializeFromUtf |      1.06 |           426.56 |           400.78 |         |
| System.Text.Json.Serialization.Tests.ReadJson<ImmutableSortedDictionary<String,  |      1.06 |         78155.94 |         73498.13 |         |
| System.Text.Json.Serialization.Tests.ReadJson<Hashtable>.DeserializeFromString   |      1.06 |         67906.38 |         63896.07 |         |
| System.Text.Json.Serialization.Tests.ReadJson<ArrayList>.DeserializeFromUtf8Byte |      1.06 |         50083.93 |         47157.52 |         |
| System.Text.Json.Serialization.Tests.ReadJson<ImmutableSortedDictionary<String,  |      1.05 |         73213.63 |         69703.93 |         |
| System.Text.Json.Serialization.Tests.ReadJson<Int32>.DeserializeFromUtf8Bytes    |      1.03 |            90.42 |            87.57 |         |

/cc @kunalspathak @jeffhandley for the intrinsics

src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.cs

gfoidl · 2020-08-15T09:12:30Z

src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.cs

For the following methods I see this pattern quite often. Maybe it's time to make these a public api?

Yeah, they're still easy to type helpers, but repeated often enough. Furthermore there could be asserts that the reads are within bounds, etc.

Most similar issue I've found is #36182

src/libraries/System.Text.Json/src/System.Text.Json.csproj

benaadams · 2020-08-17T01:50:19Z

better: 30, geomean: 1.113
worse: 1, geomean: 1.066
total diff: 31

| Slower                                                                           | diff/base | Base Median (ns) | Diff Median (ns) | Modality|
| -------------------------------------------------------------------------------- | ---------:| ----------------:| ----------------:| --------:|
| System.Text.Json.Serialization.Tests.ReadJson<SimpleStructWithProperties>.Deseri |      1.07 |           288.23 |           307.28 |         |

| Faster                                                                           | base/diff | Base Median (ns) | Diff Median (ns) | Modality|
| -------------------------------------------------------------------------------- | ---------:| ----------------:| ----------------:| --------:|
| System.Text.Json.Serialization.Tests.ReadJson<Dictionary<String, String>>.Deseri |      1.22 |         22031.18 |         18102.24 |         |
| System.Text.Json.Serialization.Tests.ReadJson<Dictionary<String, String>>.Deseri |      1.20 |         21554.46 |         17944.26 |         |
| System.Text.Json.Serialization.Tests.ReadJson<MyEventsListerViewModel>.Deseriali |      1.20 |        341085.15 |        284277.87 |         |
| System.Text.Json.Serialization.Tests.ReadJson<IndexViewModel>.DeserializeFromUtf |      1.19 |         29706.85 |         24903.98 |         |
| System.Text.Json.Serialization.Tests.ReadJson<MyEventsListerViewModel>.Deseriali |      1.19 |        330481.64 |        277947.73 |         |
| System.Text.Json.Serialization.Tests.ReadJson<MyEventsListerViewModel>.Deseriali |      1.19 |        399397.91 |        336085.63 |         |
| System.Text.Json.Serialization.Tests.ReadJson<IndexViewModel>.DeserializeFromStr |      1.18 |         31986.63 |         27064.34 |         |
| System.Text.Json.Serialization.Tests.ReadJson<Dictionary<String, String>>.Deseri |      1.18 |         23919.94 |         20268.43 |         |
| System.Text.Json.Serialization.Tests.ReadJson<IndexViewModel>.DeserializeFromStr |      1.18 |         31000.14 |         26348.41 |         |
| System.Text.Json.Serialization.Tests.ReadJson<LoginViewModel>.DeserializeFromStr |      1.15 |           522.04 |           454.92 |         |
| System.Text.Json.Serialization.Tests.ReadJson<Hashtable>.DeserializeFromUtf8Byte |      1.12 |         68993.04 |         61636.12 |         |
| System.Text.Json.Serialization.Tests.ReadJson<HashSet<String>>.DeserializeFromUt |      1.10 |         12663.49 |         11477.42 |         |
| System.Text.Json.Serialization.Tests.ReadJson<ImmutableSortedDictionary<String,  |      1.10 |         78887.97 |         71927.75 |         |
| System.Text.Json.Serialization.Tests.ReadJson<Hashtable>.DeserializeFromStream   |      1.10 |         69968.66 |         63797.29 |         |
| System.Text.Json.Serialization.Tests.ReadJson<LoginViewModel>.DeserializeFromStr |      1.09 |           763.93 |           698.34 |         |
| System.Text.Json.Serialization.Tests.ReadJson<HashSet<String>>.DeserializeFromSt |      1.09 |         12745.33 |         11734.38 |         |
| System.Text.Json.Serialization.Tests.ReadJson<ImmutableDictionary<String, String |      1.09 |         48372.57 |         44572.25 |         |
| System.Text.Json.Serialization.Tests.ReadJson<ArrayList>.DeserializeFromStream   |      1.09 |         51673.60 |         47617.28 |         |
| System.Text.Json.Serialization.Tests.ReadJson<ImmutableDictionary<String, String |      1.08 |         51226.78 |         47249.65 |         |
| System.Text.Json.Serialization.Tests.ReadJson<HashSet<String>>.DeserializeFromSt |      1.08 |         13979.59 |         12973.07 |         |
| System.Text.Json.Serialization.Tests.ReadJson<ImmutableDictionary<String, String |      1.08 |         48102.61 |         44652.30 |         |
| System.Text.Json.Serialization.Tests.ReadJson<BinaryData>.DeserializeFromStream  |      1.07 |           807.95 |           755.26 |         |
| System.Text.Json.Serialization.Tests.ReadJson<ArrayList>.DeserializeFromString   |      1.07 |         49947.46 |         46735.64 |         |
| System.Text.Json.Serialization.Tests.ReadJson<BinaryData>.DeserializeFromUtf8Byt |      1.07 |           488.59 |           458.07 |         |
| System.Text.Json.Serialization.Tests.ReadJson<LoginViewModel>.DeserializeFromUtf |      1.06 |           426.56 |           400.78 |         |
| System.Text.Json.Serialization.Tests.ReadJson<ImmutableSortedDictionary<String,  |      1.06 |         78155.94 |         73498.13 |         |
| System.Text.Json.Serialization.Tests.ReadJson<Hashtable>.DeserializeFromString   |      1.06 |         67906.38 |         63896.07 |         |
| System.Text.Json.Serialization.Tests.ReadJson<ArrayList>.DeserializeFromUtf8Byte |      1.06 |         50083.93 |         47157.52 |         |
| System.Text.Json.Serialization.Tests.ReadJson<ImmutableSortedDictionary<String,  |      1.05 |         73213.63 |         69703.93 |         |
| System.Text.Json.Serialization.Tests.ReadJson<Int32>.DeserializeFromUtf8Bytes    |      1.03 |            90.42 |            87.57 |         |

benaadams · 2020-08-17T02:44:29Z

Libraries Test Run release coreclr OSX x64 Debug failed: AcceptV4BoundToAnyV6_Success #40913

src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.SpeedOpts.cs

benaadams · 2020-08-17T03:30:51Z

src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.SpeedOpts.cs

Note: Avx2 doesn't have less than so arguments are switched for greater than

benaadams · 2020-08-17T03:33:13Z

src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.SpeedOpts.cs

Note: Also CompareGreaterThan is only sbyte (and CompareLessThan on Sse2), so subtract 0x80 before casting and doing the compare to keep the order correct

benaadams · 2020-08-17T21:52:56Z

Can't workout the linker test issues :(

dotnet build /t:PublishTrimmed /nr:false /warnaserror -p:configuration=Release

/Users/runner/work/1/s/.dotnet/sdk/5.0.100-preview.8.20362.3/NuGet.targets(128,5): 
Invalid restore input. Invalid target framework 'unsupported'. Input files:
 /Users/runner/work/1/s/src/libraries/System.Text.Json/src/System.Text.Json.csproj.
 [/Users/runner/work/1/s/artifacts/bin/trimmingTests/projects/Microsoft.Extensions.Logging.Console.TrimmingTests/
  AddConsoleFormatterTests/osx-x64/project.csproj] [/Users/runner/work/1/s/src/libraries/Microsoft.Extensions.Logging.Console/tests/TrimmingTests/
  Microsoft.Extensions.Logging.Console.TrimmingTests.proj]

layomia · 2020-08-19T18:13:55Z

Taking a look now.

benaadams · 2020-08-19T18:54:49Z

@layomia when I add the browser type $(NetCoreAppCurrent)-Browser to <TargetFrameworks> then the linker tests for TrimmingTests/Microsoft.Extensions.Logging.Console.TrimmingTests.proj which imports System.Text.Json start failing with

Errors
    NuGet.targets(128,5): Invalid restore input. Invalid target framework 'unsupported'. 
    Input files:  
        src/libraries/System.Text.Json/src/System.Text.Json.csproj.          
        trimmingTests/projects/Microsoft.Extensions.Logging.Console.TrimmingTests/AddConsoleFormatterTests/linux-x64/project.csproj               
        src/libraries/Microsoft.Extensions.Logging.Console/tests/TrimmingTests/Microsoft.Extensions.Logging.Console.TrimmingTests.proj

On Windows, Linux and OSX. However doesn't seem to provide much helpful detail in the binlog as to why its becoming "unsupported" :(

benaadams · 2020-08-19T19:03:12Z

So the unsupported bit is probably NuGet/Home#5413; which is what NuGet outputs for an unknown framework

benaadams · 2020-08-19T19:26:09Z

Current working idea looking at other build is need to override NuGet.targets with NuGet.RestoreEx.targets somehow

Overriding target "Restore" in project "/__w/1/s/.dotnet/sdk/5.0.100-preview.8.20362.3/NuGet.targets"
 with target "Restore" from project "/__w/1/s/.dotnet/sdk/5.0.100-preview.8.20362.3/NuGet.RestoreEx.targets"

layomia

I tried out some changes wrt wasm/non-wasm builds, pls see if this helps - layomia@137ac47. We don't run the linker tests specifically on wasm builds afaict so I don't think any linker test changes are needed in this PR.

layomia · 2020-08-19T17:56:33Z

src/libraries/System.Text.Json/src/System.Text.Json.csproj

  <PropertyGroup>
    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
-    <TargetFrameworks>$(NetCoreAppCurrent);netstandard2.0;netcoreapp3.0;net461</TargetFrameworks>
+    <TargetFrameworks>$(NetCoreAppCurrent)-Windows_NT;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Browser;netstandard2.0;netcoreapp3.0;net461</TargetFrameworks>


Why is this change needed?

I think it will only output an AnyCPU assembly if either the TargetFrameworks doesn't have browser or the Architectures aren't included to have wasm which will mean '$(TargetArchitecture)' == 'wasm' will never be true?

src/libraries/System.Text.Json/src/System.Text.Json.csproj

src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.SpeedOpts.cs

src/libraries/System.Text.Json/src/System.Text.Json.csproj

benaadams · 2020-08-19T22:49:43Z

We don't run the linker tests specifically on wasm builds afaict so I don't think any linker test changes are needed in this PR.

So its looks like NuGet chokes on <TargetFrameworks>net5.0;net5.0-Browser;...; so the arcade build tools normalize that before restore to <TargetFrameworks>net5.0;... so NuGet doesn't fail.

However the linker tests look to shell out to a basic build which uses the standard restore causing them to fail for all frameworks (where this changed .csproj is included)

benaadams · 2020-08-20T14:59:10Z

🥳

Going to close this PR and open a fresh one as its a bit of a tire fire

benaadams · 2020-08-20T15:49:25Z

Opened #41097

Dotnet-GitSync-Bot added the area-blazor label Aug 15, 2020

benaadams force-pushed the IndexOfOrLessThan branch from efb91b9 to 6cfe3c7 Compare August 15, 2020 05:57

benaadams changed the title ~~Intrinsicify JsonReaderHelper.IndexOfOrLessThan~~ Intrinsicify JsonReaderHelper.IndexOfOrLessThan [NoMerge] Aug 15, 2020

gfoidl reviewed Aug 15, 2020

View reviewed changes

benaadams force-pushed the IndexOfOrLessThan branch from fecb348 to 1aca492 Compare August 15, 2020 13:50

jkotas added area-System.Text.Json and removed area-blazor labels Aug 15, 2020

benaadams force-pushed the IndexOfOrLessThan branch from 1aca492 to 3a58170 Compare August 15, 2020 17:00

benaadams marked this pull request as ready for review August 15, 2020 20:30

benaadams requested review from jozkee, layomia and steveharter as code owners August 15, 2020 20:30

benaadams changed the title ~~Intrinsicify JsonReaderHelper.IndexOfOrLessThan [NoMerge]~~ Intrinsicify JsonReaderHelper.IndexOfOrLessThan Aug 15, 2020

benaadams force-pushed the IndexOfOrLessThan branch from 3a58170 to f4f85a3 Compare August 16, 2020 02:42

benaadams mentioned this pull request Aug 17, 2020

[interp] Intrinsify IndexOfOrLessThan #40705

Closed

benaadams commented Aug 17, 2020

View reviewed changes

src/libraries/System.Text.Json/src/System.Text.Json.csproj Outdated Show resolved Hide resolved

benaadams mentioned this pull request Aug 17, 2020

CI: System.ArgumentException : The supplied System.Net.Internals.SocketAddress is an invalid size for the System.Net.IPEndPoint end point. #40913

Closed

benaadams commented Aug 17, 2020

View reviewed changes

src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.SpeedOpts.cs Outdated Show resolved Hide resolved

src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.SpeedOpts.cs Outdated Show resolved Hide resolved

benaadams commented Aug 17, 2020

View reviewed changes

benaadams force-pushed the IndexOfOrLessThan branch 3 times, most recently from 7ffc080 to 3a10197 Compare August 17, 2020 16:41

benaadams added 2 commits August 17, 2020 18:54

x86, x64, ARM Intrinsicify JsonReaderHelper.IndexOfOrLessThan

6fb8886

Add optimization for wasm

19facfe

benaadams force-pushed the IndexOfOrLessThan branch from 2108029 to 19facfe Compare August 17, 2020 18:31

Add IncludePlatformAttributes

5635864

try adding framework

c2de3f7

benaadams mentioned this pull request Aug 19, 2020

OSX CI builds failing "Please install openssl before running this script" #41006

Closed

benaadams added 2 commits August 19, 2020 01:54

multi-framework

654a69e

BuildTargetFramework

53a145d

Try including Directory.Build.props

d3f1254

layomia reviewed Aug 19, 2020

View reviewed changes

Feedback

59c60ec

benaadams added 11 commits August 20, 2020 00:56

TargetFramework filter

bd953a7

Output trimming subtask binlog

f13fe7a

Add to Build.props

59dc0d5

paths

b6bcfaa

Import

bc5e9cd

Conditional

d4f1078

path

d4cdcc6

path

51acd99

path

f6b1790

path

53afcd8

Try static restore

541a856

benaadams closed this Aug 20, 2020

benaadams mentioned this pull request Aug 20, 2020

Intrinsicify (Sse, Axv2, Arm64, wasm) JsonReaderHelper.IndexOfOrLessThan #41097

Closed

runfoapp bot mentioned this pull request Sep 29, 2020

Test initial triage jaredpar/runfo#50

Closed

ghost locked as resolved and limited conversation to collaborators Dec 7, 2020

Intrinsicify JsonReaderHelper.IndexOfOrLessThan #40877

Intrinsicify JsonReaderHelper.IndexOfOrLessThan #40877

Uh oh!

Conversation

benaadams commented Aug 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gfoidl Aug 15, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

benaadams commented Aug 17, 2020

Uh oh!

benaadams commented Aug 17, 2020

Uh oh!

Uh oh!

Uh oh!

benaadams Aug 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

benaadams Aug 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

benaadams commented Aug 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

layomia commented Aug 19, 2020

Uh oh!

benaadams commented Aug 19, 2020

Uh oh!

benaadams commented Aug 19, 2020

Uh oh!

benaadams commented Aug 19, 2020

Uh oh!

layomia left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

layomia Aug 19, 2020

Choose a reason for hiding this comment

Uh oh!

benaadams Aug 19, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

benaadams commented Aug 19, 2020

Uh oh!

benaadams commented Aug 20, 2020

Uh oh!

benaadams commented Aug 20, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

benaadams commented Aug 15, 2020 •

edited

Loading

benaadams Aug 17, 2020 •

edited

Loading

benaadams Aug 17, 2020 •

edited

Loading

benaadams commented Aug 17, 2020 •

edited

Loading

layomia left a comment •

edited

Loading