-
Notifications
You must be signed in to change notification settings - Fork 103
[Proof Of Concept] Aot support #73
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
base: master
Are you sure you want to change the base?
Conversation
Hi! Thank you for your interest in contributing to In a few days I am traveling to Japan for one month, so I don't have the time to check the code changes. I know very little about AOT, just that Microsoft is trying hard to make it possible, so your suggestions sounds reasonable. A few remarks:
It looks like Is this one task or two? :)
There is no error handing with Could you explain about Please know that I wrote all of this very fast, so if something doesn't make sense, just point it out :) |
Hi @Jinjinov, thank you for your fast response and no pressure. This is just a proof of concept and not something that is ready to be merged. Your remarks sound reasonable, and I am willing to work on aot support for Regarding My suggestion is that you do your travel first and then take a look at the code - when you have time. |
Ok, I agree. I made a new git branch Here are a few suggestions:
This way, all public interfaces and classes stay the same, so there are no breaking changes. And there is no need:
I still have not decided if internal should become public (always risky, because any change to classes that were internal could become a breaking change because users of the NuGet can now access and use everything) or if internal should stay internal and the new projects should use
I would wait with I know that this is a lot of work, but as you said, there is no rush. I can help when I come back. I also do this in my free time after work. Out of all my C# projects, I spend the most time working on https://github.com/Jinjinov/OpenHabitTracker - but I will take some time for this too. Feel free to comment or point out anything that doesn't make sense :) |
@Jinjinov Is this the package structure that you have in mind? It might be possible to include |
Oh, you added Yes, this is the package structure I had in mind. Only 2 lines are missing: Only |
Yes, if this is possible, this would be perfect :) |
Hi,
I wanted to share my proof of concept for aot support.
Under Windows,
Hardware.Info
uses WMI to gather hardware details. COM is used to interface with WMI. The "old" COM intertop in .net doesn't support aot compilation. Luckily com wrappers have been introduced which "provide an optimized and AOT-friendly COM interop solution".This proof of concept is not ready to be merged as-is but shows that it's possible to to have both - WMI and aot.
Changes
interface IHardwareInfoRetrieval
andclass HardwareInfoBase
are now publicclass HardwareInfo
has a new constructor which accepts an instance ofIHardwareInfoRetrieval
Hardware.Info.Windows.Aot
which implementsclass HardwareInfoRetrievalWindowsAot
Hardware.Info.TestAot
which enables aot publication and usesHardware.Info.Windows.Aot
My goal has been to compare both implementations (old COM interop and new COM wrappers). Aot support should be a drop-in solution and not require any other changes. Therefore I made
interface IHardwareInfoRetrieval
andclass HardwareInfoBase
are public.Next Steps
If aot support is a viable path forward, I suggest that
Hardware.Info
still references non-aot compatible packages. I think that's fine as long as the code paths aren't used - they might actually be trimmed out when trimming is enabled. But this should be investigated to prevent issues for users. Moving the windows implementation out into a separate library would avoid all these issues but that's a breaking change.System.Management
initializes and un-initializes com (see issues section).Console.WriteLine
Issues
System.Numerics.BigInteger
might be a better alternative - but that's a breaking change.