NativeAOT support #1998

AArnott · 2024-10-11T22:40:18Z

NativeAOT support in MessagePack would be awesome for performance and to appeal to the .NET 9 crowd marching over from use of BinaryFormatter.

AArnott · 2024-10-11T22:42:12Z

This doesn't work today. Besides having warnings that remain, the various attributes that "allow" portions of the library to include dynamic code (either Ref.Emit or closing generic types at runtime) or that require unreferenced code is viral and bubbles up the most of the library. It's unclear whether a path to success exists.

I have a meeting scheduled for next week with the .NET folks who help with such migrations and will report back on what I learn.

eerhardt · 2024-10-14T14:45:23Z

azure-pipelines/dotnet.yml

  displayName: 🧪 dotnet test
  condition: and(succeeded(), ${{ parameters.RunTests }})

+- script: dotnet publish -c Release -r ${{ parameters.osRID }}-x64 -warnaserror


You shouldn't need to pass -r (or -c for that matter, as Release is the default for publish). The RID is calculated based on the current machine.

Dropping the -r switch produces a build error:

error NETSDK1191: A runtime identifier for the property 'PublishAot' couldn't be inferred. Specify a rid explicitly.

Does that happen all the time? Or only for specific operating systems?

It happened on Windows when I tested it locally. That's as far as I got.

tests/NativeAOTCompatibility/NativeAOTCompatibility.csproj

eerhardt · 2024-10-14T14:55:51Z

src/MessagePack/Resolvers/StandardResolver.cs

+        [UnconditionalSuppressMessage("ReflectionAnalysis", "IL3050", Justification = Constants.AvoidDynamicCodeRuntimeCheck)]
        private static readonly IFormatterResolver[] Resolvers = DynamicAssembly.AvoidDynamicCode


@sbomer - Is it possible to add the FeatureGuardAttribute to PollyfillAttributes.cs and apply it to DynamicAssembly.AvoidDynamicCode? That way we wouldn't have to suppress these warnings?

I guess it may not be possible yet unless we are using a .NET 9 ILC and Roslyn Analyzers. Thoughts here?

You could polyfill and add FeatureGuard if you made it a property and inverted it (UseDynamicCode for example). That would guarantee that it gets substituted to false in an AOT'd app that targets net9.0 or above. (And would also silence analyzer warnings if MessagePack were built for net9.0).

If it only silences analyzer warnings when targeting net9.0, that won't work since I'm also targeting net8.0 and need to suppress warnings there anyway.

tests/NativeAOTCompatibility/NativeAOTCompatibility.csproj

eerhardt · 2024-10-14T14:57:49Z

src/MessagePack.Annotations/Attributes.cs

        {
            this.Key = key;
-            this.SubType = Type.GetType(subType, throwOnError: true);
+            this.SubType = Type.GetType(subType, throwOnError: true) ?? throw new Exception("SubType not found");


Since throwOnError is true, when would the new code ever get invoked?

Perhaps never. But I added it to suppress a warning. Probably because Type.GetType(string, bool) is documented as returning Type?.

eerhardt · 2024-10-14T15:03:38Z

src/MessagePack.Annotations/Attributes.cs

+#if NET8_0_OR_GREATER
+        [RequiresUnreferencedCode("The type named in 'subType' may be trimmed.")]
+#endif
        public UnionAttribute(int key, string subType)


You should be able to mark the subType with a DynamicallyAccessedMembers attribute, which will tell the trimmer that the string is a Type. And so any Type that is passed in on this string will be preserved.

I'm not sure on exactly which DynamicallyAccessedMemberTypes to use, it may work with just .None, or you may need to add PublicParameterlessConstructor. @sbomer and @agocke would know for sure. PublicParameterlessConstructor would be the safest to ensure it is kept.

Suggested change

public UnionAttribute(int key, string subType)

public UnionAttribute(int key, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] string subType)

This would allow you to remove the RequiresUnreferencedCode attribute on this constructor.

None didn't work. But PublicParameterlessConstructor worked. There's no guarantee that such a constructor is declared on this type, so I hope it works either way.

eerhardt · 2024-10-14T15:04:19Z

src/MessagePack.Annotations/Attributes.cs

    public class MessagePackFormatterAttribute : Attribute
    {
+#if NET8_0_OR_GREATER
+        [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields | DynamicallyAccessedMemberTypes.Interfaces)]


I think you also need this attribute on the Type parameters passed into the constructors.

That sounds reasonable. I wonder why the analyzer missed that.

eerhardt · 2024-10-14T15:08:27Z

src/MessagePack/Formatters/TypelessFormatter.cs

            Deserializers.TryAdd(typeof(object), _ => (object p1, ref MessagePackReader p2, MessagePackSerializerOptions p3) => new object());
        }

+#if NET8_0_OR_GREATER


(nit) Why are these are sometimes guarded with #if NET8 and sometimes not?

Oh, just because I didn't switch to polyfill style attributes until I was midway through the work, and I missed removing the #if from some places. I removed it from here.

eerhardt · 2024-10-14T15:12:03Z

src/MessagePack/Formatters/EnumAsStringFormatter`1.cs

 {
    // Note:This implementation is 'not' fastest, should more improve.
-    public sealed class EnumAsStringFormatter<T> : IMessagePackFormatter<T>
+    public sealed class EnumAsStringFormatter<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields)] T> : IMessagePackFormatter<T>


Note: in net9 we changed our trimming rules such that when T is guaranteed to be an Enum (like it is here), it no longer warns when you try to get the fields of the enum type. I don't think adding this attribute is causing you problems now, but just an FYI. See dotnet/runtime#100814 for more info.

See dotnet/runtime#88512

Co-authored-by: Eric Erhardt <[email protected]>

glennawatson · 2024-11-08T00:17:39Z

I'd imagine you might want to go the way of System Text Json and provide a source generator in the future and basically unroll the serialization loop?

Would that be something you'd be interested if I can help contribute to?

I think this PR goes a long way but for proper NativeAOT support in the future having a source generator that reads/writes direclty might be useful.

AArnott · 2024-11-08T14:37:36Z

@glennawatson Thanks for volunteering. We have a source generator. And it's very complex, and also relies on some runtime resolvers that are tainted with dynamic dependencies. I suspect we could eventually achieve NativeAOT safety (certified by satisfied analyzers) but it will be so much work that I started fresh at https://github.com/aarnott/nerdbank.messagepack. It is very nearly feature complete and is NativeAOT ready from the ground up. Much simpler code and much simpler to use, and still very fast. I'd love to hear what you think.

AArnott · 2024-12-03T22:32:54Z

Aborting this effort. Explanation here: #1997 (comment)

AArnott added 6 commits October 11, 2024 12:46

Assert that library is AOT friendly

ab22ce8

Work toward SkipClrVisibilityChecks being annotated

48ab1e8

WIP

3ef4f2a

WIP

44c1805

WIP

eede261

WIP: as far as we get?

ad53a55

AArnott added this to the v3.0 milestone Oct 11, 2024

AArnott linked an issue Oct 11, 2024 that may be closed by this pull request

NativeAOT assertion for v3 #1997

Closed

eerhardt reviewed Oct 14, 2024

View reviewed changes

AArnott and others added 6 commits October 14, 2024 09:54

Workaround lack of AllFields and AllProperties options

0098658

See dotnet/runtime#88512

Update tests/NativeAOTCompatibility/NativeAOTCompatibility.csproj

f4df21c

Co-authored-by: Eric Erhardt <[email protected]>

Update tests/NativeAOTCompatibility/NativeAOTCompatibility.csproj

802309e

Co-authored-by: Eric Erhardt <[email protected]>

Annotation touch-ups

2c13324

Merge remote-tracking branch 'origin/develop' into 1997-nativeaot

ad08f00

Spray a bunch of "Star" Requires attributes around

2094aa5

FireController1847 mentioned this pull request Nov 12, 2024

Question about custom converters and arrays AArnott/Nerdbank.MessagePack#71

Closed

AArnott closed this Dec 3, 2024

mayuki mentioned this pull request Jan 23, 2025

Protects the generated resolver type metadata from trimmer #2134

Merged

		[UnconditionalSuppressMessage("ReflectionAnalysis", "IL3050", Justification = Constants.AvoidDynamicCodeRuntimeCheck)]
		private static readonly IFormatterResolver[] Resolvers = DynamicAssembly.AvoidDynamicCode

	public UnionAttribute(int key, string subType)
	public UnionAttribute(int key, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] string subType)

Uh oh!

NativeAOT support #1998

NativeAOT support #1998

Uh oh!

Conversation

AArnott commented Oct 11, 2024

Uh oh!

AArnott commented Oct 11, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AArnott Oct 15, 2024 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

glennawatson commented Nov 8, 2024

Uh oh!

AArnott commented Nov 8, 2024

Uh oh!

AArnott commented Dec 3, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

AArnott Oct 15, 2024 •

edited

Loading