Combine two table lookups into one while formatting Guid on Arm64 by SwapnilGaikwad · Pull Request #87126 · dotnet/runtime

SwapnilGaikwad · 2023-06-05T12:50:22Z

Contributes towards the issue #84328

SwapnilGaikwad · 2023-06-05T12:52:08Z

Micro-benchmarking on an N1 system shows the following improvements.

|                     Method |        Job |                                                                                             Toolchain |                        value |       Mean |     Error |    StdDev |     Median |        Min |        Max | Ratio | MannWhitney(2%) | Allocated | Alloc Ratio |
|--------------------------- |----------- |------------------------------------------------------------------------------------------------------ |----------------------------- |-----------:|----------:|----------:|-----------:|-----------:|-----------:|------:|---------------- |----------:|------------:|
|            FormatterDouble | Job-KYVZJY |    /main/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |     -1.7976931348623157E+308 | 719.049 ns | 1.7385 ns | 1.6262 ns | 719.644 ns | 714.862 ns | 720.304 ns |  1.00 |            Base |         - |          NA |
|            FormatterDouble | Job-RTINGX | /runtime/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |     -1.7976931348623157E+308 | 653.947 ns | 3.7146 ns | 3.4746 ns | 655.705 ns | 644.141 ns | 656.125 ns |  0.91 |          Faster |         - |          NA |
|                            |            |                                                                                                       |                              |            |           |           |            |            |            |       |                 |           |             |
|             FormatterInt32 | Job-KYVZJY |    /main/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |                  -2147483648 |  27.632 ns | 0.0369 ns | 0.0327 ns |  27.623 ns |  27.583 ns |  27.693 ns |  1.00 |            Base |         - |          NA |
|             FormatterInt32 | Job-RTINGX | /runtime/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |                  -2147483648 |  18.398 ns | 0.0239 ns | 0.0212 ns |  18.388 ns |  18.378 ns |  18.439 ns |  0.67 |          Faster |         - |          NA |
|                            |            |                                                                                                       |                              |            |           |           |            |            |            |       |                 |           |             |
|             FormatterInt64 | Job-KYVZJY |    /main/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |         -9223372036854775808 |  43.181 ns | 0.0768 ns | 0.0719 ns |  43.186 ns |  42.986 ns |  43.264 ns |  1.00 |            Base |         - |          NA |
|             FormatterInt64 | Job-RTINGX | /runtime/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |         -9223372036854775808 |  35.830 ns | 0.0053 ns | 0.0047 ns |  35.829 ns |  35.824 ns |  35.841 ns |  0.83 |          Faster |         - |          NA |
|                            |            |                                                                                                       |                              |            |           |           |            |            |            |       |                 |           |             |
|            FormatterUInt64 | Job-KYVZJY |    /main/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |                            0 |  12.813 ns | 0.0066 ns | 0.0061 ns |  12.812 ns |  12.806 ns |  12.825 ns |  1.00 |            Base |         - |          NA |
|            FormatterUInt64 | Job-RTINGX | /runtime/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |                            0 |   6.148 ns | 0.0020 ns | 0.0018 ns |   6.147 ns |   6.145 ns |   6.151 ns |  0.48 |          Faster |         - |          NA |
|                            |            |                                                                                                       |                              |            |           |           |            |            |            |       |                 |           |             |
|            FormatterUInt32 | Job-KYVZJY |    /main/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |                            0 |   4.992 ns | 0.0092 ns | 0.0072 ns |   4.989 ns |   4.988 ns |   5.014 ns |  1.00 |            Base |         - |          NA |
|            FormatterUInt32 | Job-RTINGX | /runtime/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |                            0 |   4.477 ns | 0.0014 ns | 0.0013 ns |   4.477 ns |   4.474 ns |   4.478 ns |  0.90 |          Faster |         - |          NA |
|                            |            |                                                                                                       |                              |            |           |           |            |            |            |       |                 |           |             |
|            FormatterDouble | Job-KYVZJY |    /main/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |      1.7976931348623157E+308 | 717.626 ns | 2.3723 ns | 2.2190 ns | 718.797 ns | 714.878 ns | 720.597 ns |  1.00 |            Base |         - |          NA |
|            FormatterDouble | Job-RTINGX | /runtime/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |      1.7976931348623157E+308 | 656.692 ns | 3.8938 ns | 3.6422 ns | 658.688 ns | 646.814 ns | 659.652 ns |  0.92 |          Faster |         - |          NA |
|                            |            |                                                                                                       |                              |            |           |           |            |            |            |       |                 |           |             |
| FormatterDateTimeOffsetNow | Job-KYVZJY |    /main/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun | 12/30/2017 3:45:22 AM -08:00 |  78.494 ns | 0.2743 ns | 0.2566 ns |  78.541 ns |  78.050 ns |  78.838 ns |  1.00 |            Base |         - |          NA |
| FormatterDateTimeOffsetNow | Job-RTINGX | /runtime/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun | 12/30/2017 3:45:22 AM -08:00 |  43.786 ns | 0.0035 ns | 0.0031 ns |  43.786 ns |  43.780 ns |  43.791 ns |  0.56 |          Faster |         - |          NA |
|                            |            |                                                                                                       |                              |            |           |           |            |            |            |       |                 |           |             |
|             FormatterInt64 | Job-KYVZJY |    /main/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |                        12345 |   9.508 ns | 0.0012 ns | 0.0012 ns |   9.508 ns |   9.507 ns |   9.510 ns |  1.00 |            Base |         - |          NA |
|             FormatterInt64 | Job-RTINGX | /runtime/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |                        12345 |   9.837 ns | 0.0018 ns | 0.0017 ns |   9.837 ns |   9.835 ns |   9.840 ns |  1.03 |          Slower |         - |          NA |
|                            |            |                                                                                                       |                              |            |           |           |            |            |            |       |                 |           |             |
|            FormatterUInt64 | Job-KYVZJY |    /main/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |                        12345 |  17.086 ns | 0.0298 ns | 0.0279 ns |  17.100 ns |  17.033 ns |  17.116 ns |  1.00 |            Base |         - |          NA |
|            FormatterUInt64 | Job-RTINGX | /runtime/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |                        12345 |   9.658 ns | 0.0018 ns | 0.0016 ns |   9.658 ns |   9.657 ns |   9.661 ns |  0.57 |          Faster |         - |          NA |
|                            |            |                                                                                                       |                              |            |           |           |            |            |            |       |                 |           |             |
|             FormatterInt32 | Job-KYVZJY |    /main/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |                            4 |  13.620 ns | 0.1192 ns | 0.1115 ns |  13.627 ns |  13.457 ns |  13.817 ns |  1.00 |            Base |         - |          NA |
|             FormatterInt32 | Job-RTINGX | /runtime/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |                            4 |   6.292 ns | 0.0005 ns | 0.0004 ns |   6.292 ns |   6.291 ns |   6.293 ns |  0.46 |          Faster |         - |          NA |
|                            |            |                                                                                                       |                              |            |           |           |            |            |            |       |                 |           |             |
|             FormatterInt32 | Job-KYVZJY |    /main/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |                        12345 |  12.839 ns | 0.0083 ns | 0.0077 ns |  12.837 ns |  12.825 ns |  12.852 ns |  1.00 |            Base |         - |          NA |
|             FormatterInt32 | Job-RTINGX | /runtime/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |                        12345 |   8.271 ns | 0.0013 ns | 0.0011 ns |   8.271 ns |   8.269 ns |   8.273 ns |  0.64 |          Faster |         - |          NA |
|                            |            |                                                                                                       |                              |            |           |           |            |            |            |       |                 |           |             |
|            FormatterUInt32 | Job-KYVZJY |    /main/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |                        12345 |   6.370 ns | 0.0021 ns | 0.0019 ns |   6.370 ns |   6.368 ns |   6.374 ns |  1.00 |            Base |         - |          NA |
|            FormatterUInt32 | Job-RTINGX | /runtime/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |                        12345 |   5.455 ns | 0.0013 ns | 0.0012 ns |   5.455 ns |   5.452 ns |   5.456 ns |  0.86 |          Faster |         - |          NA |
|                            |            |                                                                                                       |                              |            |           |           |            |            |            |       |                 |           |             |
|            FormatterDouble | Job-KYVZJY |    /main/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |                        12345 | 308.785 ns | 0.4602 ns | 0.4304 ns | 308.800 ns | 308.037 ns | 309.567 ns |  1.00 |            Base |         - |          NA |
|            FormatterDouble | Job-RTINGX | /runtime/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |                        12345 | 289.306 ns | 0.2112 ns | 0.1976 ns | 289.341 ns | 288.920 ns | 289.701 ns |  0.94 |          Faster |         - |          NA |
|                            |            |                                                                                                       |                              |            |           |           |            |            |            |       |                 |           |             |
|           FormatterDecimal | Job-KYVZJY |    /main/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |                   123456.789 | 180.820 ns | 0.3125 ns | 0.2923 ns | 180.794 ns | 180.095 ns | 181.339 ns |  1.00 |            Base |         - |          NA |
|           FormatterDecimal | Job-RTINGX | /runtime/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |                   123456.789 | 170.822 ns | 1.6344 ns | 1.5288 ns | 170.302 ns | 168.708 ns | 173.102 ns |  0.94 |          Faster |         - |          NA |
|                            |            |                                                                                                       |                              |            |           |           |            |            |            |       |                 |           |             |
|            FormatterUInt64 | Job-KYVZJY |    /main/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |         18446744073709551615 |  38.537 ns | 0.0164 ns | 0.0146 ns |  38.537 ns |  38.509 ns |  38.561 ns |  1.00 |            Base |         - |          NA |
|            FormatterUInt64 | Job-RTINGX | /runtime/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |         18446744073709551615 |  30.125 ns | 0.5294 ns | 0.4952 ns |  29.936 ns |  29.927 ns |  31.795 ns |  0.78 |          Faster |         - |          NA |
|                            |            |                                                                                                       |                              |            |           |           |            |            |            |       |                 |           |             |
|             FormatterInt32 | Job-KYVZJY |    /main/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |                   2147483647 |  17.946 ns | 0.0300 ns | 0.0266 ns |  17.941 ns |  17.907 ns |  17.994 ns |  1.00 |            Base |         - |          NA |
|             FormatterInt32 | Job-RTINGX | /runtime/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |                   2147483647 |  13.035 ns | 0.0020 ns | 0.0017 ns |  13.034 ns |  13.033 ns |  13.038 ns |  0.73 |          Faster |         - |          NA |
|                            |            |                                                                                                       |                              |            |           |           |            |            |            |       |                 |           |             |
|            FormatterUInt32 | Job-KYVZJY |    /main/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |                   4294967295 |  11.676 ns | 0.0020 ns | 0.0019 ns |  11.676 ns |  11.673 ns |  11.679 ns |  1.00 |            Base |         - |          NA |
|            FormatterUInt32 | Job-RTINGX | /runtime/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |                   4294967295 |  11.613 ns | 0.0094 ns | 0.0083 ns |  11.610 ns |  11.605 ns |  11.626 ns |  0.99 |            Same |         - |          NA |
|                            |            |                                                                                                       |                              |            |           |           |            |            |            |       |                 |           |             |
|             FormatterInt64 | Job-KYVZJY |    /main/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |          9223372036854775807 |  30.190 ns | 0.0034 ns | 0.0031 ns |  30.190 ns |  30.187 ns |  30.197 ns |  1.00 |            Base |         - |          NA |
|             FormatterInt64 | Job-RTINGX | /runtime/artifacts/bin/testhost/net8.0-linux-Release-arm64/shared/Microsoft.NETCore.App/8.0.0/corerun |          9223372036854775807 |  29.670 ns | 0.0136 ns | 0.0114 ns |  29.670 ns |  29.640 ns |  29.687 ns |  0.98 |            Same |         - |          NA |

EgorBo

LGTM, although, I'd prefer us doing this in JIT (to merge two subsequent table lookups)

src/libraries/System.Private.CoreLib/src/System/Guid.cs

kunalspathak · 2023-06-05T14:27:25Z

/azp run runtime-coreclr libraries-jitstressregs, runtime-coreclr libraries-jitstress-random, runtime-coreclr libraries-jitstress, runtime-coreclr libraries-jitstress2-jitstressregs

azure-pipelines · 2023-06-05T14:27:49Z

Azure Pipelines successfully started running 4 pipeline(s).

kunalspathak · 2023-06-05T17:16:00Z

/azp run runtime-coreclr libraries-jitstressregs, runtime-coreclr libraries-jitstress-random, runtime-coreclr libraries-jitstress, runtime-coreclr libraries-jitstress2-jitstressregs

azure-pipelines · 2023-06-05T17:17:28Z

Azure Pipelines successfully started running 4 pipeline(s).

ghost · 2023-06-05T17:45:31Z

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

Contributes towards the issue #84328

Author:	SwapnilGaikwad
Assignees:	-
Labels:	`area-CodeGen-coreclr`, `community-contribution`
Milestone:	-

SwapnilGaikwad · 2023-06-06T09:40:25Z

Seems like the test failures are because of the cancellations. Not sure why the tests got cancelled.

kunalspathak · 2023-06-06T17:10:18Z

/azp run runtime-coreclr libraries-jitstressregs, runtime-coreclr libraries-jitstress-random, runtime-coreclr libraries-jitstress, runtime-coreclr libraries-jitstress2-jitstressregs

azure-pipelines · 2023-06-06T17:10:45Z

Azure Pipelines successfully started running 4 pipeline(s).

kunalspathak · 2023-06-07T14:15:52Z

/azp run runtime-coreclr libraries-jitstressregs, runtime-coreclr libraries-jitstress-random, runtime-coreclr libraries-jitstress, runtime-coreclr libraries-jitstress2-jitstressregs

azure-pipelines · 2023-06-07T14:16:21Z

Azure Pipelines successfully started running 4 pipeline(s).

kunalspathak · 2023-06-07T21:36:29Z

One of the failing issue is #83316, but there are some arm64 failures in https://dev.azure.com/dnceng-public/public/_build/results?buildId=298474&view=ms.vss-test-web.build-test-results-tab&runId=6039742&resultId=199446&paneView=debug that you might want to check.

SwapnilGaikwad · 2023-06-08T15:38:14Z

One of the failing issue is #83316, but there are some arm64 failures in https://dev.azure.com/dnceng-public/public/_build/results?buildId=298474&view=ms.vss-test-web.build-test-results-tab&runId=6039742&resultId=199446&paneView=debug that you might want to check.

Hi @kunalspathak, I couldn't run the windows on Arm64 failure as I don't have access to such machine. Instead, I tried to reproduce the failure on ubuntu 18.04 as it looked like the similar System.NullReferenceException issue.
I could download the artifacts from the azure pipeline and run them to produce a failure. However, I found the same error when replace the System.Private.CoreLib.dll from the main branch. I'm not sure whether the issue is related to the patch. Do you have any suggestions to proceed on this?

While running the failures from the pipeline, we get a script to reproduce failure that basically runs the following command after setting the required environment variables.

/home/user/dotnet/main/artifacts/bin/testhost/net8.0-linux-Release-arm64/dotnet exec --runtimeconfig \
System.Private.Xml.Tests.runtimeconfig.json --depsfile System.Private.Xml.Tests.deps.json xunit.console.dll \
System.Private.Xml.Tests.dll -xml testResults.xml -nologo -nocolor -notrait category=IgnoreForCI \
-notrait category=OuterLoop -notrait category=failing

This runs all the System.Private.Xml.Tests tests. Do you know if we can force it to run a single test? I tried the flags such as -p:XunitMethodName="System.Private.Xml.Tests.XmlSerializerTests" but they don't seem to scope it down.

kunalspathak · 2023-06-09T02:57:34Z

I just looked around and noticed that similar tests failed few weeks back and there is #86563 for that. So I think we should be good here. Since they repro with random seeds, the failures is not consistent week over week.

kunalspathak

LGTM. Thanks!

SwapnilGaikwad · 2023-06-13T07:57:33Z

Hi @kunalspathak @EgorBo, is there anything that I can do to push this patch forward? 🙂

kunalspathak · 2023-06-13T15:28:46Z

Hi @kunalspathak @EgorBo, is there anything that I can do to push this patch forward? 🙂

Not really. I was waiting for CI to complete. Failures are #87477, #86563, #83316, #70307

src/libraries/System.Private.CoreLib/src/System/Guid.cs

Combine two table lookups into one while formatting Guid on Arm64

49e6bbf

ghost added needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners community-contribution Indicates that the PR has been added by a community member labels Jun 5, 2023

EgorBo approved these changes Jun 5, 2023

View reviewed changes

EgorBo reviewed Jun 5, 2023

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/Guid.cs Show resolved Hide resolved

Add a comment to explain the combined table look-up and hex indices

9d7af3a

EgorBo added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jun 5, 2023

build-analysis bot mentioned this pull request Jun 5, 2023

Tracking issue for CI build timeouts #76454

Closed

build-analysis bot mentioned this pull request Jun 8, 2023

[mono] Tests failing with Failed to find compatible device: arm64-v8a #87267

Closed

kunalspathak approved these changes Jun 9, 2023

View reviewed changes

kunalspathak closed this Jun 9, 2023

kunalspathak reopened this Jun 9, 2023

This was referenced Jun 9, 2023

Roslyn source generator crash on mono/linux/arm64 #81123

Closed

Official runtime builds for .NET 7 and 8 frequently failing during artifact upload dotnet/arcade#12936

Closed

CI System.Text.RegularExpressions.Tests failure #84323

Closed

This was referenced Jun 9, 2023

MsQuicPlatformDetectionTests failures in CI #87275

Closed

Work item tests crash on OSX x64 Debug #87287

Closed

NAOT dwarfdump has more warnings than expected #87321

Closed

Unable to clean up running docker containers #87323

Closed

kunalspathak closed this Jun 9, 2023

kunalspathak reopened this Jun 9, 2023

kunalspathak merged commit bec3b04 into dotnet:main Jun 13, 2023

danmoseley reviewed Jun 29, 2023

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/Guid.cs Show resolved Hide resolved

SwapnilGaikwad deleted the github-merge-shuffle-guid branch June 29, 2023 12:39

kunalspathak mentioned this pull request Jun 29, 2023

Arm64: Use VectorTableLookup/VectorTableLookupExtension APIs in libraries #84328

Closed

SwapnilGaikwad mentioned this pull request Jul 3, 2023

Remove TODO from a comment in System.Guid.cs #88337

Merged

JulieLeeMSFT mentioned this pull request Jul 5, 2023

What's new in .NET 8 Preview 6 [WIP] dotnet/core#8437

Closed

3 tasks

ghost locked as resolved and limited conversation to collaborators Aug 2, 2023

Conversation

SwapnilGaikwad commented Jun 5, 2023

Uh oh!

SwapnilGaikwad commented Jun 5, 2023

Uh oh!

EgorBo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kunalspathak commented Jun 5, 2023

Uh oh!

azure-pipelines bot commented Jun 5, 2023

Uh oh!

kunalspathak commented Jun 5, 2023

Uh oh!

azure-pipelines bot commented Jun 5, 2023

Uh oh!

ghost commented Jun 5, 2023

Uh oh!

SwapnilGaikwad commented Jun 6, 2023

Uh oh!

kunalspathak commented Jun 6, 2023

Uh oh!

azure-pipelines bot commented Jun 6, 2023

Uh oh!

kunalspathak commented Jun 7, 2023

Uh oh!

azure-pipelines bot commented Jun 7, 2023

Uh oh!

kunalspathak commented Jun 7, 2023

Uh oh!

SwapnilGaikwad commented Jun 8, 2023

Uh oh!

kunalspathak commented Jun 9, 2023

Uh oh!

kunalspathak left a comment

Choose a reason for hiding this comment

Uh oh!

SwapnilGaikwad commented Jun 13, 2023

Uh oh!

kunalspathak commented Jun 13, 2023

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants