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

New Version is not aot compatible and can't use static library #14

Closed
AhmedZero opened this issue Sep 12, 2024 · 12 comments
Closed

New Version is not aot compatible and can't use static library #14

AhmedZero opened this issue Sep 12, 2024 · 12 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@AhmedZero
Copy link

In version 1.1.0, I can use the static library and NativeAOT. However, in version 2.0.0 and later, it uses reflections, making it incompatible with AOT, and I can’t depend on the static library too.

@JunaMeinhold
Copy link
Member

JunaMeinhold commented Sep 12, 2024

Do you get an exception or a trim warning? Because the static analyzers for aot don't report anything. And i think i tested it with my UI Framework a while ago

@AhmedZero
Copy link
Author

AhmedZero commented Sep 12, 2024

@JunaMeinhold
Copy link
Member

image
Okay will be fixed soon...

@JunaMeinhold JunaMeinhold self-assigned this Sep 12, 2024
@JunaMeinhold JunaMeinhold added the bug Something isn't working label Sep 12, 2024
@JunaMeinhold
Copy link
Member

Closing this issue, tested it before uploading, if any other issue regarding aot persists just reopen the issue.

@AhmedZero
Copy link
Author

AhmedZero commented Sep 13, 2024

The first issue is fixed, but I still can’t depend on the static library.

		<DirectPInvoke Include="cimgui" />
		<NativeLibrary Include="cimgui.lib" />

Reference:
https://learn.microsoft.com/en-us/dotnet/core/deploying/native-aot/interop
because you use VTable and Loadlibrary which it's already inside exe. make Global Property depends on VTable or LibraryImport

@JunaMeinhold
Copy link
Member

JunaMeinhold commented Sep 13, 2024

Thanks for the information. However, it seems like there might be a misunderstanding. I don’t provide the static libraries for cimgui.

For now, I will only use the shared libraries. If the static library dependency is a real concern for you, feel free to make a fork of the project and submit a pull request with the necessary changes. I'm open to reviewing it if that approach fits your needs. (Please do not add or modify any binaries including shared and static libs, you can modify the github actions files for that. And don't use LibraryImport, it's too slow)

@AhmedZero
Copy link
Author

AhmedZero commented Sep 13, 2024

I add static library manually in my project and it automatically depend on it.
in NativeAOT with static library, it converts DLLImport to DirectPInvoke at build time.

@AhmedZero
Copy link
Author

or create two seperated package the first, VTable with Dynamic Library and the second DLLImport with Static Library.

@JunaMeinhold
Copy link
Member

I currently don't have the motivation for doing it on my own, as i said if you want you can do a fork and make a pull request, the generator supports different modes for generating bindings (see https://github.com/HexaEngine/HexaGen/blob/7a1a3c664b55c7e98b2000c9f7df6caf4e9ecd4f/HexaGen/CsCodeGeneratorConfig.cs#L24) you just have to change that in the generator,json files located here https://github.com/HexaEngine/Hexa.NET.ImGui/blob/master/Generator/cimgui/generator.json (note each lib has it's own config file 4 in total).

DllImport and LibraryImport (which just generates a DllImport) is slow compared to the VTable method (I probably should use a different name, a api function table would be more accurate) when using normal C#.

@JunaMeinhold JunaMeinhold reopened this Sep 13, 2024
@JunaMeinhold JunaMeinhold added the enhancement New feature or request label Sep 13, 2024
@amerkoleci
Copy link

amerkoleci commented Sep 21, 2024

Hi,
Based on what concept LibraryImport and DllImport are slow? Did you benchmarked that?

In all mine bindings I've used LibraryImport and I'm not sure what slowness your meaning?

@JunaMeinhold
Copy link
Member

JunaMeinhold commented Sep 21, 2024

@amerkoleci
Hi,
DllImport (LibraryImport is just DllImport) has the problem that it's lazy loaded means you always have checks on if it's loaded or not and loading it does again cause the whole system to stutter if you access one function the first time. Additionally there is memory fragmentation and CPU cache efficiency which is not guaranteed with DllImport since the function pointers could be stored somewhere but don't ask me where. I didn't do explicit benchmarks but i tested it on my engine's editor and the startup timing was about 1s less and CPU usage from the editor components where less than with DllImport, like from 0,44ms to 0,22ms. And DllImport does some marshalling if not explicitly disabled via the attribute, but that means DllImport does some internal magic which again adds overhead (even checking if it's disabled means you still have one if). And memory usage is more predictable with the length of the table times sizeof(nint). DllImport also does some safety checks, which the direct calls bypass.

@JunaMeinhold
Copy link
Member

Additionally DllImport has a limitation in netstandard and can't be used for multiple platforms. Since on linux shared libs usually start with libXXX.so and on windows we have XXX.dll which often creates name differences and this becomes a problem when we try to load system libs, and setting NativeLibrary.SetDllImportResolver only works on .NET Core 3.0 or greater excluding netstandard versions and this would break .NET Framework compatibility.

For this reason i close this issue as not planned.

@JunaMeinhold JunaMeinhold closed this as not planned Won't fix, can't repro, duplicate, stale Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants