Terminal.Gui
Terminal.Gui copied to clipboard
AOT support with .Net 8?
Required reading:
- https://sentry.engineering/blog/should-you-could-you-aot
ToDos:
- [ ] Annotate anywhere reflection is used.
- [ ] Figure out how to refactor
ConfigurationManagergiven it is heavy on reflection AND serialization
I've been thinking about this, as well.
There's a fair bit of reflection that would make it difficult to do, as-is, without, at minimum, use of attributes to exempt some code from trimming.
@tig @BDisp This, I think, is a pretty valuable goal to strive for in V2, or a later release, considering where and how the library is generally going to be used.
The biggest blocker for full AOT compatibility, right now, is reflection.
Both for AOT and just in general, I'd urge everyone to try to avoid reflection in the library itself if at all possible, even though sometimes it may result in an easier implementation.
At minimum, when it does get used, or when GetType or similar methods are called on a type not known at compile-time, I would urge everyone to at least decorate the containing method, property, or class with [RequiresUnreferencedCode], so that the compiler knows a given unit is incompatible with trimming.
That's a blunt instrument and doesn't fix the problem, but at least lets consumers know they cant use that code if they intend to AOT, because the compiler understands that attribute and will throw appropriate warnings/errors when used in AoT situations. And it also makes it easy to track down and address such code at a future time.
But there are also other attributes that can be used in reflection scenarios that allow more nuance in the compiler behavior, and which can actually allow the associated code to be trim-compatible, such as DynamicallyAccessedMembers.
Both for AOT and just in general, I'd urge everyone to try to avoid reflection in the library itself if at all possible, even though sometimes it may result in an easier implementation.
I haz sad. Reflection is FUN. ;-)
But you're right, and I'll get over it.
We should add to the first post of this issue a list of known places we use reflection as Todo's.
Came across this today, which puts a lot of the relevant concepts in a real-world context.
https://sentry.engineering/blog/should-you-could-you-aot
Came across this today, which puts a lot of the relevant concepts in a real-world context.
https://sentry.engineering/blog/should-you-could-you-aot
I learned a ton. Thanks.
Updated first post in this issue....
So...
I brought this up in another issue, but AoT makes it even more relevant, now:
ConfigurationManager may not be prudent to spend significant effort on, for this, because the dotnet Configuration stuff does it all and is trim-friendly, since it uses code generation.
I know a lot of work went into that, and I certainly empathize there, but I think it is very worth considering either adapting it to use the built-in stuff or replacing it with the built-in stuff. Even just from the point of view of maintainability, not having to deal with an in-house configuration infrastructure would be a load off of the project and future development.
Another biggie that isn't optional at all:
P/Invoke
All DllImport attributes and the methods they adorn need to be migrated to the new LibraryImport form, which uses source generation and is AoT compatible.
DllImport uses runtime code generation and will always be trimmed out as a result, making it completely broken.
It's a pretty simple change though.
Here are two examples:
public static partial class ConsoleKeyMapping {
#if !WT_ISSUE_8871_FIXED // https://github.com/microsoft/terminal/issues/8871
/// <summary>
/// Translates (maps) a virtual-key code into a scan code or character value, or translates a scan code into a virtual-key code.
/// </summary>
/// <param name="vk"></param>
/// <param name="uMapType">
/// If MAPVK_VK_TO_CHAR (2) - The uCode parameter is a virtual-key code and is translated into an un-shifted
/// character value in the low order word of the return value.
/// </param>
/// <param name="dwhkl"></param>
/// <returns>An un-shifted character value in the low order word of the return value. Dead keys (diacritics)
/// are indicated by setting the top bit of the return value. If there is no translation,
/// the function returns 0. See Remarks.</returns>
[LibraryImport("user32", EntryPoint = "MapVirtualKeyExW", StringMarshalling = StringMarshalling.Utf16)]
internal static partial uint MapVirtualKeyEx (VK vk, uint uMapType, nint dwhkl);
/// <summary>
/// Retrieves the active input locale identifier (formerly called the keyboard layout).
/// </summary>
/// <param name="idThread">0 for current thread</param>
/// <returns>The return value is the input locale identifier for the thread.
/// The low word contains a Language Identifier for the input language
/// and the high word contains a device handle to the physical layout of the keyboard.
/// </returns>
[LibraryImport("user32", EntryPoint = "GetKeyboardLayout", StringMarshalling = StringMarshalling.Utf16)]
internal static partial nint GetKeyboardLayout (nint idThread);
...
}
Key differences:
- The containing class has to be partial (it's source generation after all).
- The method itself is required to have an access modifier (source generation)
- The method must be marked
static partial(again, source generation)externgoes away completely
- It is not necessary or recommended to specify the file extension for the required library. The resolver will automatically attempt to use platform-specific rules.
- It is also not necessary to include the 'lib' prefix or suffix for libraries on linux, as that is a permutation the runtime will try, if the no-prefix version doesn't exist. Just use the naked library name without lib prefix or suffix, and without extension.
- I also opportunistically changed the
IntPtrs tonint, which is the dotnet 7 and up way of doing it.
There's also a free performance benefit to doing it, even for non-AoT situations, as the old method is never eligible for most optimizations, whereas this method is, at compile-time, which is awesome. At minimum, it'll usually inline the calls, avoiding a stack push and potential context switch.
Related to #3212, ReSharper is also capable of having custom patterns defined which are then treated as inspections.
This would allow, for example, defining a pattern that matches "DllImport" and flags it as an error, so that future uses are also prevented.
Yep. I already started migrating to LibraryImport as part of
- #2610
I made some really interesting discoveries about DllImport, LibraryImport, and what all that stuff is REALLY doing....
I'll be posting a discussion for it. I'll link it here when it's posted...
Short version is DllImport may not be as unacceptable as the docs make it seem...
Here's the discussion post: https://github.com/gui-cs/Terminal.Gui/discussions/3238
I've posted the initial written code and the first level of analysis, with two more to come.
I think you'll find it pretty interesting....