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

Aot support 2.0 #75

Draft
wants to merge 57 commits into
base: aot
Choose a base branch
from
Draft

Aot support 2.0 #75

wants to merge 57 commits into from

Conversation

Gitii
Copy link

@Gitii Gitii commented Sep 29, 2024

Hi @Jinjinov,

I have created this draft PR because I would like to get some early feedback.

What has been done so far:

  • Refactored code base as requested
  • Added new workflow that builds and tests both nuget packages on each supported platform

I use the new workflow to verify that the nuget packages can be used on each support platform. I encountered some issues with target framework resolution, so this turned out to be a good approach.
I haven't created a Hardware.Info.Aot.Test but instead made the existing Hardware.Info.Test some sort of test runner that verifies expectations (architecture and jit/aot compiler) and then executes all hardware info queries as before.
See this workflow run to see it in action: https://github.com/Gitii/Hardware.Info.Aot.Poc/actions/runs/11094018584/job/30821001822

I wanted to avoid code duplication and the same code should be executed in all six targets (win, linux, macos x jit, aot x x64).

Right now, there are some platform-dependant issues (see linked run) but I would like to check with you first, if the current approach is fine for you. Otherwise, I will refactor again and then iron out these "smaller" issues later.

@Gitii
Copy link
Author

Gitii commented Oct 4, 2024

I have fixed the known bugs, including:

  • proper error handling (throw error when query setup fails)
  • proper cleanup handling (always dispose query & query items correctly)
  • fix for weird property type issues (jit vs aot)

Limitations / Things that need to be disclaimed to end users:

  • The code is not thread safe. There are no safe guards.
  • Not tested on arm64 (GH action runners that support arm64 are coming out end of 2024)
  • Initializing and Un-initializing of COM still looks sketchy, but it works. I have added a comment about com initialization. The gist is that you can initialize and un-initialize multiple times but always 1:1. The only gotcha is that the same parameter need to be used.
  • The code needs to run in a "trusted" process (that's also true for the legacy COM interop)

Suggestions:
Right now, implementations using COM wrappers and legacy COM Interop are separate but share a lot of code and have 99% the same logic. In theory, it should be possible to reuse the main logic (query strings, field mapping) in both implementations. Either by OOP abstraction or including the same file in both projects and preprocessor statements (hacky, but works without yet another library).

In the future, the existing github workflow should be used to verify that jit and aot code paths still work on all supported platforms. The attached artifacts can be uploaded to nuget registry, too. It think that the workflow should be run for PRs and for new commits in main.

The readme needs to be updated for aot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant