Optimize '[Guid]' parsing on Native AOT

[Sergio0694]

We're going to need to use [Guid] in some scenarios in CsWinRT 3.0 (see #114179), and while looking at the Type.GUID logic in Native AOT I saw some potential small perf improvements we could do. Figured I'd give it a shot and see if it's well received 😄

[Copilot]

Pull Request Overview

This PR aims to optimize the parsing of [Guid] in Native AOT by improving the underlying UTF8 string decoding and Guid parsing logic. Key changes include:

• Adding a new DecodeStringUtf8 method in NativeFormatReader.String.cs to efficiently decode UTF8 strings.
• Introducing a GetRawStringDataUtf8 helper in ConstantStringValue to leverage the new decoding method.
• Refactoring Guid parsing in NativeFormatRuntimeNamedTypeInfo.cs to use a static local method that processes UTF8 data for performance gains.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
src/coreclr/tools/Common/Internal/NativeFormat/NativeFormatReader.String.cs	Added method for decoding UTF8 strings with boundary checks and copying to a destination span
src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeFormatReaderGen.cs	Added a helper that uses the new decode method to extract UTF8 encoded string data
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/NativeFormat/NativeFormatRuntimeNamedTypeInfo.cs	Replaced direct Guid creation with a static local method that decodes and parses UTF8 data for Guid construction

Comments suppressed due to low confidence (1)

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/NativeFormat/NativeFormatRuntimeNamedTypeInfo.cs:88

• [nitpick] Consider using a parameter name in the exception call that more clearly reflects the null context of the handle value rather than 'utf8Text', which might be misleading.

ArgumentNullException.Throw("utf8Text");

[dotnet-policy-service]

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

[jkotas]

Any perf numbers?

[jkotas]

This is dead ifdef. You can delete it

[Sergio0694]

There's another one in the method above, should I remove that one too?

[Sergio0694]

Mmh if I remove it, building fails with this at the Guid.Parse call:

D:\git\runtime\src\coreclr\tools\Common\Internal\NativeFormat\NativeFormatReader.String.cs(75,31): error CS1503: Argume
nt 1: cannot convert from 'System.ReadOnlySpan<byte>' to 'System.ReadOnlySpan<char>' [D:\git\runtime\src\coreclr\tools\
aot\ILCompiler.MetadataTransform\ILCompiler.MetadataTransform.csproj]

[jkotas]

Hmm, that's because we have not picked up #105654 in the LKG SDK yet.

It should be fixed by https://github.com/dotnet/runtime/pull/116328/files#diff-8df3cd354bc584349d04ad5675b33c042d8b99b741b8b95af394c55e0f5001bfR3

[Sergio0694]

It seems like it's fine now after doing the changes Michal suggested. I removed that ifdef 👍

[jkotas]

Could you please delete the ifdef (and the define in .csproj) while you are on in, even though it is not strictly required for the rest?

[Sergio0694]

Yup, of course, but just to clarify, I need to wait for that PR you linked to be merged first, and then to update this branch, right?

[jkotas]

I do not think you need to wait for that PR. The earlier build break was caused by the new code that you have introduced. The existing code should compile just fine with the ifdef deleted.

[jkotas]

Where was this exception thrown before?

[Sergio0694]

By the Guid constructor I think. The code was getting a ConstantStringValue value, which immediatley returns in the constructor if the input handle is null. That leaves the string field set to default, and that's then directly passed to the Guid constructor. I added this check here to try to match the semantics in case of failures as close as possible.

[MichalStrehovsky]

/azp run runtime-nativeaot-outerloop

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jkotas]

Could you please measure the perf improvement for Type.GUID?

[Sergio0694]

Yup yup, will set some time aside to whip together a small benchmark. Was chatting with @EgorBo who said he might look into adding NAOT support to the bot, but I might just do a simple thing locally for now to unblock this sooner 🙂

[Sergio0694]

I'm struggling to figure out how to run BenchmarkDotNet with two local Native AOT builds, are there any docs? 😅
The docs here seem outdated (they mention .NET 7) and point to an output path that doesn't exist.

What I did is:

• build.cmd clr.aot+libs -rc Release both on main and this PR

And then I have:

using System.Runtime.InteropServices;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;

BenchmarkRunner.Run<Benchmark>();

[MemoryDiagnoser]
public class Benchmark
{
    private readonly Type _type = typeof(IFoo);

    [Benchmark]
    public Guid GetGUID() => _type.GUID;
}

[Guid("1DB7A2FC-9CB3-4A95-BEED-758C77F7715B")]
public interface IFoo;

I would imagine I'd need a way to setup two jobs, each with IlcSdkPath set for \artifacts\bin\coreclr\windows.x64.Release\aotsdk\ in the build for either main or the PR branch, but it's not clear to me how to do this. Are there docs or workflow instructions for this specific scenario anywhere, by any chance? Thank you!

[jkotas]

point to an output path that doesn't exist.

artifacts\packages\Release\Shipping gets built only in subsets that build packages. The easiest way to build it is by running build -c release.

It is the same as what's documented in
https://github.com/dotnet/runtime/blob/main/docs/workflow/building/coreclr/nativeaot.md#building-packages .

[Sergio0694]

Thank you! Made some progress but things are still not working. I have this code:

using System.Runtime.InteropServices;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Configs;
using BenchmarkDotNet.Jobs;
using BenchmarkDotNet.Running;
using BenchmarkDotNet.Toolchains.NativeAot;

var config = DefaultConfig.Instance
    .AddJob(Job.ShortRun
        .WithToolchain(NativeAotToolchain.CreateBuilder()
            .UseLocalBuild(new DirectoryInfo(@"D:\diffs\nativeaot\guid_optimize\main\Shipping\"))
            .DisplayName("NativeAOT main")
            .TargetFrameworkMoniker("net10.0")
            .ToToolchain()))
    .AddJob(Job.ShortRun
        .WithToolchain(NativeAotToolchain.CreateBuilder()
            .UseLocalBuild(new DirectoryInfo(@"D:\diffs\nativeaot\guid_optimize\pr\Shipping\"))
            .DisplayName("NativeAOT PR")
            .TargetFrameworkMoniker("net10.0")
            .ToToolchain()));

BenchmarkRunner.Run<Benchmark>(config);

[MemoryDiagnoser]
public class Benchmark
{
    private readonly Type _type = typeof(IFoo);

    [Benchmark]
    public Guid GetGUID() => _type.GUID;
}

[Guid("1DB7A2FC-9CB3-4A95-BEED-758C77F7715B")]
public interface IFoo;

I built the two packages in those two directories.
Then I setup a .csproj with .NET 10, and I'm doing dotnet run -c Release using the stable public .NET 10 Preview 4.

When BDN tries building the two benchmarks, it fails with this error:

EXEC : error : Unrecognized instruction set sse [C:\Users\sergiopedri\AppData\Local\Temp\GuidBenchmark-ShortRun-1\BenchmarkDotNet.Autogenerated.csproj]
  System.CommandLine.CommandLineException: Unrecognized instruction set sse
     at System.CommandLine.Helpers.ConfigureInstructionSetSupport(String, Int32, Boolean, TargetArchitecture, TargetOS, String, String, Logger, Boolean) + 0xced
     at ILCompiler.Program.Run() + 0x777
     at ILCompiler.ILCompilerRootCommand.<>c__DisplayClass255_0.<.ctor>b__0(ParseResult) + 0x314

@EgorBo mentioned it's likely because SSE was removed, but BDN tries to use it here.

Any possible workaround I could use? 😅

[jkotas]

mentioned it's likely because SSE was removed, but BDN tries to use it here.

Why is it not taking the native instruction set path a few lines above?

[EgorBo]

When I was trying to follow the steps, I ended up with this error:

error LNK2001: unresolved external symbol CompressionNative_InflateReset2_

🙁

[jkotas]

I am fine if you just build a trivial manual test as a console app and run it before/after. Personally, I tend to do that for quick measurements - it is faster, guaranteed to work, and you can tell exactly what it is measuring.

using System.Diagnostics;
using System.Runtime.InteropServices;

Stopwatch sw = new();
for (;;)
{
    sw.Restart();
    for (int i = 0; i < 10000000; i++)
    {
        Guid g = typeof(MyType).GUID;
    }
    Console.WriteLine(sw.ElapsedMilliseconds);
}

[Guid("690FCAC9-4CEC-406A-88DE-DA86B7914EA7")]
class MyType
{
}

[Sergio0694]

Results aren't making any sense to me given the changes:

main	PR
4840	5226
4865	5159
4843	5164
4831	5169
4833	5163

I'll re-test. I don't see how this could possibly be the case.

[Sergio0694]

Double checked after rebasing. Same exact results. I did not accidentally swap them, the numbers are correct.

main	PR
4860	5230
4894	5331
4905	5168
4886	5174
4893	5195

I genuinely don't understand how the changes here could possibly regress perf.
Could anyone help double check? Or, does anyone have any ideas? Could this be something else..? 😅

[jkotas]

I see a very small (~1.5%) improvement (win-x64, default PublishAot, AMD EPYC 7763 - standard MS DevBox). It is not unexpected since this is not improving where the time is actually spent in Type.GUID.

[jkotas]

Or, does anyone have any ideas?

What are the machine specs that you run this on? Just want to double check - do you run this on release builds of the runtime (build -c release)?

[Sergio0694]

"I see a very small (~1.5%) improvement"

That's in line with what I was expecting too: small perf improvement, and less memory use (which we can't see here though).

"What are the machine specs that you run this on?"

Local desktop, i9-12900K.

"do you run this on release builds of the runtime"

Yeah I just did build.cmd -c release on both commits, and then published with the latest nightly + IlcAotSdkPath.

[jkotas]

and less memory use (which we can't see here though).

Sounds good, making the property zero allocation (when called repeatedly) is a reasonable justification.