Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make MonoGame trimming/AOT compatible as-is #8104

Open
mrhelmut opened this issue Dec 14, 2023 · 13 comments
Open

Make MonoGame trimming/AOT compatible as-is #8104

mrhelmut opened this issue Dec 14, 2023 · 13 comments

Comments

@mrhelmut
Copy link
Contributor

mrhelmut commented Dec 14, 2023

Game developers are likely to be willing to produce apps using either the PublishTrimmed or PublishAot build options (both being forced when building for gaming consoles). Right now, this requires projects using MonoGame to root the MonoGame assembly so that commonly dynamically loaded types (e.g. typically anything loaded with the Content Pipeline) are properly linked by adding this to their .csproj definitions:

  <ItemGroup>
    <TrimmerRootAssembly Include="MonoGame.Framework" />
  </ItemGroup>

There's a couple of problems with this approach:

  • Even though this can be partly fixed by adding this to project templates, it is not the intended .NET experience and is error-prone if anyone is creating a project from scratch or editing their .csproj
  • This doesn't make MonoGame trimmable, it forces the IL compiler to not trim MonoGame at all, which is a bad fix in regard to what users might expect from trimming

We should try building MonoGame with <EnableTrimAnalyzer>true</EnableTrimAnalyzer>, verify if any IL warnings related to trimming / AOT scenarios show up, and fix them.

Once those warnings are fixed, we will be able to mark the MonoGame library as <IsAotCompatible>true</IsAotCompatible> (which also sets <IsTrimmable>true</IsTrimmable>). This way users will know that MonoGame has been tailored to support trimming / AOT scenarios and won't have to add anything to their .csproj.

As a side note, users willing to have their game running on consoles should also aim at fixing any warnings generated by enabling <EnableTrimAnalyzer>true</EnableTrimAnalyzer> because console games are built using AOT compilation (we should add this to the documentation to prepare people for building for consoles).

@mrhelmut
Copy link
Contributor Author

mrhelmut commented Dec 15, 2023

One of the warnings we should fix (this one is from DesktopGL builds):

ILC : warning IL3000: MonoGame.Framework.Utilities.FuncLoader.LoadLibraryExt(String): 'System.Reflection.Assembly.Location.get' always returns an empty string for assemblies embedded in a single-file app. If the path to the app directory is needed, consider calling 'System.AppContext.BaseDirectory'.

Pushed a fix for this one #8105. It was making macOS builds to crash with AOT compilation.

@Mindfulplays
Copy link
Contributor

(e.g. typically anything loaded with the Content Pipeline) are properly linked by adding this to their .csproj definitions:

I was looking at this, in particular something a la this . That approach may not be scalable and may be error prone (better approach is to add the correct attribute at the class-level).

Is this along the lines of what you are thinking?

@mrhelmut
Copy link
Contributor Author

That approach used to work with old .NET <= 4.5, but modern .NET will see the trick and trim this out from the assembly. So we need to come up with something else.

Related proposal #8014. Though the intended solution as per the latest .NET release is to mark types with [DynamicallyAccessedMembers] (see this documentation).

@mrhelmut
Copy link
Contributor Author

There is another issue related to NativeAOT vs BRUTE that we need to fix beside trimming compatibility: #7872
Any thought on this one @harry-cpp ? We should come up with a solution that works no matter the runtime and no matter how XNBs were built.

@harry-cpp
Copy link
Member

Oh, it seems I forgot to merge the fix: #7895

@Mindfulplays
Copy link
Contributor

I wonder if a long term solution is to use a source generator (simple one, not using Roslyn) that uses reflection only at compile time collecting data to be fed into the runtime code. Similar to 'protobuf-net' etc.

This requires adding a MG attribute that can both help capture reflection data and help mark those classes/generics and mark them as dynamic to prevent the compiler from optimizing away these out for the linker.

@harry-cpp
Copy link
Member

The issue is that base stuff C# types are written differently depending on if you use newer or older mgcb, because its found in different base namespaces between dotnet, .NET, .NET Framework and mono. This is written inside the content files themselves, so either a System.Private.CoreLib or mscorelib will be the base namespace. This means during content loading we need to just replace the base namespaces with whatever our current runtime that we are using is. I don't think your suggestion helps in any way with this.

@Mindfulplays
Copy link
Contributor

There are three separate problems here: one is a stable name for the classes referred to in the mgcb; another is ensuring all classes potentially used by mgcb are not optimized away during compilation; and importantly no use of reflection for NativeAot.

To solve all these problems, one approach is to limit the use of reflection during compilation, use that reflection to collect stable base class names (which means respecting the current content type reader name as expected by the code linked above), stick it in a static read only list that is autogenerated and used by the runtime.

E.g.

static readonly Dictionary<string, CreatorFn> _mapTypenames

The string->CreatorFn will allow constructing the type for the provided stable class name used by the mgcb with no reflection nor Activator.CreateInstance use and be safe for AOT.

@mrhelmut
Copy link
Contributor Author

MonoGame has a very stable code in regard to what gets dynamically loaded from its own types or not (and otherwise has already been avoiding reflection in all the other parts of the code). These types haven't changed in years so I believe that we should be safe by just adding [DynamicallyAccessedMembers] to the types listed here.

Other than that, we should be safe now, we already fixed the few outstanding compatibility issues. For good measure, we should enable <EnableTrimAnalyzer>true</EnableTrimAnalyzer> on all target projects to check if there are any AOT warning remaining but as far as I have checked, we're good now.

The only problem remaining is marking those types with the attribute to be able to add <IsAotCompatible>true</IsAotCompatible>.

If anything fails beyond that, it is likely due to user code, and not to MonoGame itself. In that regard, users will see in their build log that the trimmer raised warnings. All we can do is adding a documentation about trimming pointing to the official .NET documentation so that users know how to fix those warnings in their own code.

@harry-cpp
Copy link
Member

one is a stable name for the classes referred to in the mgcb

It's not about what mgcb writes, the latest mgcb writes the correct types for the latest .NET, aka. System.Private.CoreLib, it's about what game itself needs as I said above.

If your game is compiled against mono or .NET Framework, mscorelib is the base type it expects.

If your game is compiled against .NET Core or .NET, System.Private.CoreLib is the base type it expects.

These are some core .NET types we are talking about here, like String. We wanna be able to load both so we are compatible with XNA content as well. These aren't gonna randomly change unless there is a brand new C# runtime.

@Mindfulplays
Copy link
Contributor

Thanks for the discussion, sounds good!

@recore67
Copy link

recore67 commented Jan 2, 2024

any possible solution for the current public release of monogame?

Edit:
this works

<ItemGroup>
	<TrimmerRootAssembly Include="MonoGame.Framework" />
	<TrimmerRootAssembly Include="mscorlib" />
</ItemGroup>

@srodrigo
Copy link

srodrigo commented Feb 6, 2024

As a side note, users willing to have their game running on consoles should also aim at fixing any warnings generated by enabling <EnableTrimAnalyzer>true</EnableTrimAnalyzer> because console games are built using AOT compilation (we should add this to the documentation to prepare people for building for consoles).

Please! This would be very useful. Also, it'd be great to have a section to explain preventive good practices for people willing to port to consoles down the road but not having development kits yet. Something along the lines of "avoid doing this and that so porting to consoles is easier", similar to what FNA does on their wiki.

@mrhelmut mrhelmut added this to the 3.8.2 milestone Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants