-
Notifications
You must be signed in to change notification settings - Fork 443
fix: incorrect loading of native library with multiple backends #594
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
Conversation
cc @vvdb-architecture @adammikulis @xuzeyu91 Would you like to help to test on this PR? Here's some steps for reference:
|
If you're unable to delete those two folders my best guess would be that we're explicitly loading one library (e.g. CUDA) but then it's also implicitly loading the AVX library when one of the native methods is called. Maybe we should experiment with using |
Yes, there must be two libraries loaded at the same time. I've tried |
What I meant about using |
I agree that we need the same way to load both libraries, llama.dll and llava_share.dll. My approach was to use the same directory layout, so I would suggest keeping this path and adapt the directory structure to the one that works better to the project. The only thing that we didn´t discuss yet was if llava libraries will be in the same nuget packages (that's my preference, it will be the easiest way to the user). |
@AsakusaRinne did you still want me to try this change out or should I wait while you all explore SetDllImportResolver as the solution? |
It's okay to try it later, no hurry please |
I've started experimenting with a Edit: Here it is: #603 |
After some test, I believe #603 is a better approach to resolve this problem and it'll be completed in another PR. Close this now, thank you all! |
What this PR does
It fixes the incorrect loading of native library when having multiple backends installed. This issue has been mentioned in #456 and #589.
Description about the issue
The behavior in master branch now is quite wierd.
Let's consider the case2. I had many tries and it's concluded as below.
WithCuda
orWithLibrary
to load the cuda version library, the log shown it had selected that file to load but actually no gpu is used. Besides I could confirm it's not because of the model loading problem.llama.dll
,avx
,avx2
,avx512
,cuda11
. When I was trying to load the cuda library, I could delete neitheravx
orcuda11
folder. That means, these two files were both occupied by a certain process, which is obviously the one I started.avx
folder, the case became thatavx2
andcuda11
were both occupied. Still, cuda library could not be used.After deleting all the folders except
cuda11
, I found it works. The only thing I could come up with is thatavx
,avx2
, etc. hit the keywords of dotnet runtime and it does not obey the rules we write in our code.Finally I changed all the paths and insert one more level directory
llama-sharp
into the paths of native libraries. All things worked as expected again on my PC.To be honest I don't think my solution is good enough because I still don't know the reason behind it. Besides, through this debug I found some problems in current native loading code, so I'd like to take this PR as a discussion before releasing next version with LLaVA.
@martindevans @SignalRT Hope that this mention doesn't bother you. Your participation will help a lot. :)
Discussion
NativeLibraryConfig with LLaVA
I think we just need to use the same way with llama.dll to deal with LLaVA here. If I'm not mistaking it, there was already a discussion about whether to put llava libraries in each backend packages. I'm not sure if it still needs discussion.
OpenCL support
It seems that OpenCL related logic has not been added to
NativeLibraryConfig
. I'll add it later if it's not ignored intentionally.More flexible loading strategy
I think maybe we should make a better abstraction and allow user to override some APIs to add more flexible control of library loading. Currently, as far as I know, users who want to publish an app with LLamaSharp often maintain the location of native libraries themselves, while their user could have various environments. Finally it always fallbacks to using
WithLibrary
, limiting the usage of our control strategy.However the time cost of it seems to be a problem. I think it should at least be no hurry.