From bc122bb563d9a01c3915c5f968751b81e6304e87 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Wed, 4 Jun 2025 15:00:35 -0700 Subject: [PATCH 1/7] Optimize '[Guid]' parsing on Native AOT --- .../NativeFormatRuntimeNamedTypeInfo.cs | 22 +++++++++++++++++- .../NativeFormat/NativeFormatReaderGen.cs | 15 ++++++++++++ .../NativeFormat/NativeFormatReader.String.cs | 23 +++++++++++++++++++ 3 files changed, 59 insertions(+), 1 deletion(-) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/NativeFormat/NativeFormatRuntimeNamedTypeInfo.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/NativeFormat/NativeFormatRuntimeNamedTypeInfo.cs index 65f2c3597ecc21..d0c5598c78971f 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/NativeFormat/NativeFormatRuntimeNamedTypeInfo.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/NativeFormat/NativeFormatRuntimeNamedTypeInfo.cs @@ -79,7 +79,27 @@ public sealed override bool IsByRefLike continue; if (guidStringArgumentHandle.HandleType != HandleType.ConstantStringValue) continue; - return new Guid(guidStringArgumentHandle.ToConstantStringValueHandle(_reader).GetString(_reader)); + + static Guid ReadAndParseGuid(Handle handle, MetadataReader reader) + { + if (handle.IsNil) + { + // We don't really have a parameter, so just match the name of the 'Parse' parameter + ArgumentNullException.Throw("utf8Text"); + } + + // 68 characters is the maximum length of a guid, when formatted as 'X' + Span destination = stackalloc byte[68]; + + int charsWritten = ConstantStringValue.GetRawStringDataUtf8( + reader: reader, + handle: handle.ToConstantStringValueHandle(reader), + destinationUtf8: destination); + + return Guid.Parse(destination.Slice(0, charsWritten)); + } + + return ReadAndParseGuid(guidStringArgumentHandle, _reader); } } return null; diff --git a/src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeFormatReaderGen.cs b/src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeFormatReaderGen.cs index b2fdce3f0c0cb1..bb6bf63e51fc8a 100644 --- a/src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeFormatReaderGen.cs +++ b/src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeFormatReaderGen.cs @@ -2013,6 +2013,21 @@ internal ConstantStringValue(MetadataReader reader, ConstantStringValueHandle ha public string Value => _value; private readonly string _value; + + internal static int GetRawStringDataUtf8(MetadataReader reader, ConstantStringValueHandle handle, Span destinationUtf8) + { + if (handle.IsNil) + { + return 0; + } + + uint offset = (uint)handle.Offset; + + // We don't care about the final offset + _ = reader._streamReader.DecodeStringUtf8(offset, destinationUtf8, out int bytesWritten); + + return bytesWritten; + } } // ConstantStringValue #if SYSTEM_PRIVATE_CORELIB diff --git a/src/coreclr/tools/Common/Internal/NativeFormat/NativeFormatReader.String.cs b/src/coreclr/tools/Common/Internal/NativeFormat/NativeFormatReader.String.cs index e3b90b71132bf8..6fe0ee2d6d6c11 100644 --- a/src/coreclr/tools/Common/Internal/NativeFormat/NativeFormatReader.String.cs +++ b/src/coreclr/tools/Common/Internal/NativeFormat/NativeFormatReader.String.cs @@ -7,6 +7,7 @@ // UTF8 string reading methods // --------------------------------------------------------------------------- +using System; using System.Text; namespace Internal.NativeFormat @@ -63,6 +64,28 @@ public unsafe uint DecodeString(uint offset, out string value) return endOffset; } + public unsafe uint DecodeStringUtf8(uint offset, Span destinationUtf8, out int bytesWritten) + { + uint numBytes; + offset = DecodeUnsigned(offset, out numBytes); + + if (numBytes == 0) + { + bytesWritten = 0; + return offset; + } + + uint endOffset = offset + numBytes; + if (endOffset < numBytes || endOffset > _size) + ThrowBadImageFormatException(); + + new ReadOnlySpan(_base + offset, (int)numBytes).CopyTo(destinationUtf8); + + bytesWritten = (int)numBytes; + + return endOffset; + } + // Decode a string, but just skip it instead of returning it public uint SkipString(uint offset) { From 70d7bc21e41cd4dc948802ce684fd2e88da34324 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Wed, 4 Jun 2025 16:42:38 -0700 Subject: [PATCH 2/7] Avoid copying UTF8 buffer entirely --- .../NativeFormatRuntimeNamedTypeInfo.cs | 22 ++-------------- .../NativeFormat/NativeFormatReaderGen.cs | 15 ++++++----- .../NativeFormat/NativeFormatReader.String.cs | 25 +++++++++---------- 3 files changed, 21 insertions(+), 41 deletions(-) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/NativeFormat/NativeFormatRuntimeNamedTypeInfo.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/NativeFormat/NativeFormatRuntimeNamedTypeInfo.cs index d0c5598c78971f..0eb8cb0867ec5a 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/NativeFormat/NativeFormatRuntimeNamedTypeInfo.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/NativeFormat/NativeFormatRuntimeNamedTypeInfo.cs @@ -80,26 +80,8 @@ public sealed override bool IsByRefLike if (guidStringArgumentHandle.HandleType != HandleType.ConstantStringValue) continue; - static Guid ReadAndParseGuid(Handle handle, MetadataReader reader) - { - if (handle.IsNil) - { - // We don't really have a parameter, so just match the name of the 'Parse' parameter - ArgumentNullException.Throw("utf8Text"); - } - - // 68 characters is the maximum length of a guid, when formatted as 'X' - Span destination = stackalloc byte[68]; - - int charsWritten = ConstantStringValue.GetRawStringDataUtf8( - reader: reader, - handle: handle.ToConstantStringValueHandle(reader), - destinationUtf8: destination); - - return Guid.Parse(destination.Slice(0, charsWritten)); - } - - return ReadAndParseGuid(guidStringArgumentHandle, _reader); + // Parse a 'Guid' directly from the encoded UTF8 buffer, instead of round-tripping through a 'string' + return ConstantStringValue.ParseGuid(_reader, guidStringArgumentHandle.ToConstantStringValueHandle(_reader)); } } return null; diff --git a/src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeFormatReaderGen.cs b/src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeFormatReaderGen.cs index bb6bf63e51fc8a..4e80bea66ceded 100644 --- a/src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeFormatReaderGen.cs +++ b/src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeFormatReaderGen.cs @@ -2014,19 +2014,18 @@ internal ConstantStringValue(MetadataReader reader, ConstantStringValueHandle ha public string Value => _value; private readonly string _value; - internal static int GetRawStringDataUtf8(MetadataReader reader, ConstantStringValueHandle handle, Span destinationUtf8) + /// + /// Parses a value, matching the behavior of decoding a and parsing from that. + /// + internal static Guid ParseGuid(MetadataReader reader, ConstantStringValueHandle handle) { if (handle.IsNil) { - return 0; + // We don't really have a parameter, so just match the name of the 'Guid.ctor' parameter + ArgumentNullException.Throw("input"); } - uint offset = (uint)handle.Offset; - - // We don't care about the final offset - _ = reader._streamReader.DecodeStringUtf8(offset, destinationUtf8, out int bytesWritten); - - return bytesWritten; + return reader._streamReader.ParseGuid((uint)handle.Offset); } } // ConstantStringValue diff --git a/src/coreclr/tools/Common/Internal/NativeFormat/NativeFormatReader.String.cs b/src/coreclr/tools/Common/Internal/NativeFormat/NativeFormatReader.String.cs index 6fe0ee2d6d6c11..0ac7477c6c8d46 100644 --- a/src/coreclr/tools/Common/Internal/NativeFormat/NativeFormatReader.String.cs +++ b/src/coreclr/tools/Common/Internal/NativeFormat/NativeFormatReader.String.cs @@ -64,27 +64,26 @@ public unsafe uint DecodeString(uint offset, out string value) return endOffset; } - public unsafe uint DecodeStringUtf8(uint offset, Span destinationUtf8, out int bytesWritten) +#if !NETFX_45 + public unsafe Guid ParseGuid(uint offset) { uint numBytes; offset = DecodeUnsigned(offset, out numBytes); - if (numBytes == 0) - { - bytesWritten = 0; - return offset; - } - - uint endOffset = offset + numBytes; - if (endOffset < numBytes || endOffset > _size) - ThrowBadImageFormatException(); + ReadOnlySpan bytes = []; - new ReadOnlySpan(_base + offset, (int)numBytes).CopyTo(destinationUtf8); + if (numBytes != 0) + { + uint endOffset = offset + numBytes; + if (endOffset < numBytes || endOffset > _size) + ThrowBadImageFormatException(); - bytesWritten = (int)numBytes; + bytes = new ReadOnlySpan(_base + offset, (int)numBytes); + } - return endOffset; + return Guid.Parse(bytes); } +#endif // Decode a string, but just skip it instead of returning it public uint SkipString(uint offset) From 2dd113eff783d577e02c5e73810655deb64cc55c Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Wed, 4 Jun 2025 16:49:09 -0700 Subject: [PATCH 3/7] Fix downlevel paths --- .../Metadata/NativeFormat/NativeFormatReaderGen.cs | 4 ++++ .../Internal/NativeFormat/NativeFormatReader.String.cs | 9 +++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeFormatReaderGen.cs b/src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeFormatReaderGen.cs index 4e80bea66ceded..9baf9f0f591ee9 100644 --- a/src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeFormatReaderGen.cs +++ b/src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeFormatReaderGen.cs @@ -2022,7 +2022,11 @@ internal static Guid ParseGuid(MetadataReader reader, ConstantStringValueHandle if (handle.IsNil) { // We don't really have a parameter, so just match the name of the 'Guid.ctor' parameter +#if SYSTEM_PRIVATE_CORELIB ArgumentNullException.Throw("input"); +#else + throw new ArgumentNullException("input"); +#endif } return reader._streamReader.ParseGuid((uint)handle.Offset); diff --git a/src/coreclr/tools/Common/Internal/NativeFormat/NativeFormatReader.String.cs b/src/coreclr/tools/Common/Internal/NativeFormat/NativeFormatReader.String.cs index 0ac7477c6c8d46..eb25fd61c09756 100644 --- a/src/coreclr/tools/Common/Internal/NativeFormat/NativeFormatReader.String.cs +++ b/src/coreclr/tools/Common/Internal/NativeFormat/NativeFormatReader.String.cs @@ -64,9 +64,14 @@ public unsafe uint DecodeString(uint offset, out string value) return endOffset; } -#if !NETFX_45 public unsafe Guid ParseGuid(uint offset) { +#if NETFX_45 + _ = DecodeString(offset, out string value); + + return new(value); +#else + uint numBytes; offset = DecodeUnsigned(offset, out numBytes); @@ -82,8 +87,8 @@ public unsafe Guid ParseGuid(uint offset) } return Guid.Parse(bytes); - } #endif + } // Decode a string, but just skip it instead of returning it public uint SkipString(uint offset) From d7a5953ceac73c8ce294cd47c2ca9d8180f3afad Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 5 Jun 2025 10:26:31 -0700 Subject: [PATCH 4/7] Skip copying UTF 8 bytes --- .../NativeFormatRuntimeNamedTypeInfo.cs | 11 ++++- .../NativeFormat/NativeFormatReaderGen.cs | 18 -------- .../NativeFormat/NativeMetadataReader.cs | 5 +++ .../NativeFormat/NativeFormatReader.String.cs | 43 ++++++++----------- 4 files changed, 32 insertions(+), 45 deletions(-) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/NativeFormat/NativeFormatRuntimeNamedTypeInfo.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/NativeFormat/NativeFormatRuntimeNamedTypeInfo.cs index 0eb8cb0867ec5a..2f5edeb1f67745 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/NativeFormat/NativeFormatRuntimeNamedTypeInfo.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/NativeFormat/NativeFormatRuntimeNamedTypeInfo.cs @@ -80,8 +80,17 @@ public sealed override bool IsByRefLike if (guidStringArgumentHandle.HandleType != HandleType.ConstantStringValue) continue; + ConstantStringValueHandle constantStringValueHandle = guidStringArgumentHandle.ToConstantStringValueHandle(_reader); + + if (constantStringValueHandle.IsNil) + { + // We don't really have a parameter, so just match the name of the 'Guid.ctor' parameter. + // The reason for throwing this exception is to keep semantics with the original behavior. + ArgumentNullException.Throw("input"); + } + // Parse a 'Guid' directly from the encoded UTF8 buffer, instead of round-tripping through a 'string' - return ConstantStringValue.ParseGuid(_reader, guidStringArgumentHandle.ToConstantStringValueHandle(_reader)); + return Guid.Parse(_reader.ReadStringAsBytes(constantStringValueHandle)); } } return null; diff --git a/src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeFormatReaderGen.cs b/src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeFormatReaderGen.cs index 9baf9f0f591ee9..b2fdce3f0c0cb1 100644 --- a/src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeFormatReaderGen.cs +++ b/src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeFormatReaderGen.cs @@ -2013,24 +2013,6 @@ internal ConstantStringValue(MetadataReader reader, ConstantStringValueHandle ha public string Value => _value; private readonly string _value; - - /// - /// Parses a value, matching the behavior of decoding a and parsing from that. - /// - internal static Guid ParseGuid(MetadataReader reader, ConstantStringValueHandle handle) - { - if (handle.IsNil) - { - // We don't really have a parameter, so just match the name of the 'Guid.ctor' parameter -#if SYSTEM_PRIVATE_CORELIB - ArgumentNullException.Throw("input"); -#else - throw new ArgumentNullException("input"); -#endif - } - - return reader._streamReader.ParseGuid((uint)handle.Offset); - } } // ConstantStringValue #if SYSTEM_PRIVATE_CORELIB diff --git a/src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeMetadataReader.cs b/src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeMetadataReader.cs index 39d22fa8d2dd71..d5e6a10cbd732e 100644 --- a/src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeMetadataReader.cs +++ b/src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeMetadataReader.cs @@ -220,6 +220,11 @@ internal bool StringEquals(ConstantStringValueHandle handle, string value) { return _streamReader.StringEquals((uint)handle.Offset, value); } + + internal ReadOnlySpan ReadStringAsBytes(ConstantStringValueHandle handle) + { + return _streamReader.ReadStringAsBytes((uint)handle.Offset); + } } internal sealed partial class MetadataHeader diff --git a/src/coreclr/tools/Common/Internal/NativeFormat/NativeFormatReader.String.cs b/src/coreclr/tools/Common/Internal/NativeFormat/NativeFormatReader.String.cs index eb25fd61c09756..31127960cf0f9a 100644 --- a/src/coreclr/tools/Common/Internal/NativeFormat/NativeFormatReader.String.cs +++ b/src/coreclr/tools/Common/Internal/NativeFormat/NativeFormatReader.String.cs @@ -36,6 +36,23 @@ public string ReadString(uint offset) return value; } + public unsafe ReadOnlySpan ReadStringAsBytes(uint offset) + { + uint numBytes; + offset = DecodeUnsigned(offset, out numBytes); + + if (numBytes != 0) + { + uint endOffset = offset + numBytes; + if (endOffset < numBytes || endOffset > _size) + ThrowBadImageFormatException(); + + return new(_base + offset, (int)numBytes); + } + + return ReadOnlySpan.Empty; + } + public unsafe uint DecodeString(uint offset, out string value) { uint numBytes; @@ -64,32 +81,6 @@ public unsafe uint DecodeString(uint offset, out string value) return endOffset; } - public unsafe Guid ParseGuid(uint offset) - { -#if NETFX_45 - _ = DecodeString(offset, out string value); - - return new(value); -#else - - uint numBytes; - offset = DecodeUnsigned(offset, out numBytes); - - ReadOnlySpan bytes = []; - - if (numBytes != 0) - { - uint endOffset = offset + numBytes; - if (endOffset < numBytes || endOffset > _size) - ThrowBadImageFormatException(); - - bytes = new ReadOnlySpan(_base + offset, (int)numBytes); - } - - return Guid.Parse(bytes); -#endif - } - // Decode a string, but just skip it instead of returning it public uint SkipString(uint offset) { From 638b560ead6319f841fe8f200f9749a260343464 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 5 Jun 2025 11:53:11 -0700 Subject: [PATCH 5/7] Remove exception --- .../NativeFormat/NativeFormatRuntimeNamedTypeInfo.cs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/NativeFormat/NativeFormatRuntimeNamedTypeInfo.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/NativeFormat/NativeFormatRuntimeNamedTypeInfo.cs index 2f5edeb1f67745..2a0fc6f176cfbf 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/NativeFormat/NativeFormatRuntimeNamedTypeInfo.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/NativeFormat/NativeFormatRuntimeNamedTypeInfo.cs @@ -82,13 +82,6 @@ public sealed override bool IsByRefLike ConstantStringValueHandle constantStringValueHandle = guidStringArgumentHandle.ToConstantStringValueHandle(_reader); - if (constantStringValueHandle.IsNil) - { - // We don't really have a parameter, so just match the name of the 'Guid.ctor' parameter. - // The reason for throwing this exception is to keep semantics with the original behavior. - ArgumentNullException.Throw("input"); - } - // Parse a 'Guid' directly from the encoded UTF8 buffer, instead of round-tripping through a 'string' return Guid.Parse(_reader.ReadStringAsBytes(constantStringValueHandle)); } From 41870df2ce1ba1b2b98d8e18c0f480c81f43fa2d Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 5 Jun 2025 11:55:39 -0700 Subject: [PATCH 6/7] Add missing 'IsNil' check --- .../Internal/Metadata/NativeFormat/NativeMetadataReader.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeMetadataReader.cs b/src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeMetadataReader.cs index d5e6a10cbd732e..865e7ea69f6c4c 100644 --- a/src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeMetadataReader.cs +++ b/src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeMetadataReader.cs @@ -223,6 +223,11 @@ internal bool StringEquals(ConstantStringValueHandle handle, string value) internal ReadOnlySpan ReadStringAsBytes(ConstantStringValueHandle handle) { + if (handle.IsNil) + { + return ReadOnlySpan.Empty; + } + return _streamReader.ReadStringAsBytes((uint)handle.Offset); } } From 92c6a8a85106732570f5a252faa576f25dd3b4dc Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Sun, 22 Jun 2025 10:00:11 -0700 Subject: [PATCH 7/7] Apply suggestions from code review Co-authored-by: kasperk81 <83082615+kasperk81@users.noreply.github.com> --- .../Internal/Metadata/NativeFormat/NativeMetadataReader.cs | 2 +- .../Common/Internal/NativeFormat/NativeFormatReader.String.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeMetadataReader.cs b/src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeMetadataReader.cs index 865e7ea69f6c4c..2379832cb73c9a 100644 --- a/src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeMetadataReader.cs +++ b/src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeMetadataReader.cs @@ -225,7 +225,7 @@ internal ReadOnlySpan ReadStringAsBytes(ConstantStringValueHandle handle) { if (handle.IsNil) { - return ReadOnlySpan.Empty; + return []; } return _streamReader.ReadStringAsBytes((uint)handle.Offset); diff --git a/src/coreclr/tools/Common/Internal/NativeFormat/NativeFormatReader.String.cs b/src/coreclr/tools/Common/Internal/NativeFormat/NativeFormatReader.String.cs index 31127960cf0f9a..4e1cab18f5e917 100644 --- a/src/coreclr/tools/Common/Internal/NativeFormat/NativeFormatReader.String.cs +++ b/src/coreclr/tools/Common/Internal/NativeFormat/NativeFormatReader.String.cs @@ -50,7 +50,7 @@ public unsafe ReadOnlySpan ReadStringAsBytes(uint offset) return new(_base + offset, (int)numBytes); } - return ReadOnlySpan.Empty; + return []; } public unsafe uint DecodeString(uint offset, out string value)