-
Notifications
You must be signed in to change notification settings - Fork 357
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
feat: support dynamic native library loading in .NET standard 2.0. #738
base: master
Are you sure you want to change the base?
feat: support dynamic native library loading in .NET standard 2.0. #738
Conversation
Co-authored-by: Martin Evans <martindevans@gmail.com>
Co-authored-by: Martin Evans <martindevans@gmail.com>
Using this PR, I was able to switch between AVX2 and CUDA in an Unity game. 👍 |
@m0nsky Thank you for your feedback. In which system did you test it? I've only tested this feature on Windows11 and Ubuntu22.04 yet. I'll appreciate it if you could help to test on other platforms. :) |
My tests were done on Windows 10. Unfortunately I don't have any other platforms to test on. |
Thank you. That's enough. At least now I know it works properly on windows 10. :) |
My main concern with this PR is the massive amount of extra complexity involved in importing functions. If every single function definition goes from: static extern LLamaContextParams llama_context_default_params(); To something like: #if NETSTANDARD
[DllImport(NativeApi.libraryName, CallingConvention = CallingConvention.Cdecl)]
static extern LLamaContextParams llama_context_default_params_r();
[UnmanagedFunctionPointer(CallingConvention.Cdecl)]
delegate LLamaContextParams llama_context_default_params_t();
static LLamaContextParams llama_context_default_params() => NativeLibraryConfig.DynamicLoadingDisabled ?
llama_context_default_params_r() : NativeApi.GetLLamaExport<llama_context_default_params_t>("llama_context_default_params")();
#else
[DllImport(NativeApi.libraryName, CallingConvention = CallingConvention.Cdecl)]
static extern LLamaContextParams llama_context_default_params();
#endif That's going to make upgrading the binaries a huge amount of work (more than I want to do every month, personally). I think if we want to go this path we need to find a way to reduce the boilerplate involved, or to autogenerate it all from the C header files (or some other config file). |
Code generation is a good way to handle with it but the source generator is not supported in .NET standard2.0. If we want to do so, maybe we need to write a script to manually run it every time we're going to update the binaries. In fact most of the code transforms in this PR are completed by copilot. :) |
We could use t4 templates, they're supported in every version of dotnet (since they're a stage that runs before the compiler, not as part of it). I've used them extensively in another project of mine, here's an example of one alongside the code it generates.
We could write one t4 file which contains all of the generation code, and then import that into another which just acts as a "config file" - i.e. that's what gets hand edited every time a binary update is done, specifying the function signatures that need to be generated. |
After some investigation I found I was wrong. The source generator supports .NET standard2.0. t4 template is also a good option. I'll try to use them to generate the code. Besides, I will put the code of NativeLibrary.NetStandard inside our repo instead in this PR. The code of that repo is of small size and not likely to increase in the future. It will reduce the dependency number of LLamaSharp. What's more, though it might be out-of-scope, I wonder if we should remove the dependency for |
@AsakusaRinne I think there is an even better use case for this now, since Vulkan support was introduced to LLamaSharp in 0.14 (July 16). Is there any chance you could update this PR to be compatible with master? This will allow for deploying Unity games on both AMD & NVIDIA GPUs until a better solution is ready. |
This PR should only be merged after #688.
It supports dynamic native library loading but needs more test.
Checks
TODO (not necessary in this PR)
I'll appreciate it if you'd like to help to test it!
The way to test it:
LLamaSharp.csproj
, replacing the target frameworks with<TargetFramework>netstandard2.0</TargetFramework>
.NetAppTest
project.If you want to test on other runtimes such as .NET framework and Mono, please replace the target framework yourself.
P.S. The
NetAppTest
project will be removed before it get merged.