-
-
Notifications
You must be signed in to change notification settings - Fork 41
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: config plugin for android gpu libraries #109
base: main
Are you sure you want to change the base?
Conversation
this is huge!! thanks so much - lmk when it's ready :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as per ESLint rule eqeqeq:
It is considered good practice to use the type-safe equality operators === and !== instead of their regular counterparts == and !=.
Yes, I am not 100% sure either as noted in my comment #84 . How did you test this, on your android devices - bare or expo? Did you see any performance difference? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for starting this...I have tried my best to test this inside Expo:
- I copied your changes to my local
node_modules
files - updated app.config to reference plugin
- ran
patch-package
to bring in the changes, and ran app vianpx expo run:android --device
My model will not load at all if I include any delegate
options to loadTensorflowModel()
I have also tried different options passed into the app.config#plugin for this lib.
Hey, thanks for trying it. I guess if just copy the change to I personally test it with:
Not sure, if that helps. I think I might have time later today and have a better look |
I am unable to find your branch... @TkTioNG did you make a commit from a fork? |
Yes, it is from a fork, I tested it with expo app and it works. |
ok, yes! I copied the repo and added your changes, then Now that it's linking, I have tried to load the model with GPUDelegate
however, the model never loads if I pass an I am on "react-native-fast-tflite": "~1.3.0" - anything newer fails to build due to #91 and #99 I am using a real 💩 Android Blu G53 device so it struggles with anything, even browsing web Notes related to app.config settings:
I am surprised that performance wasn't improved with the default How do I know if I need GPU enabled settings? I don't have time to test out a preview build if performance is improved outside of development environment...I also don't think I can build with a |
@lucksp I believe your case is more of a GPU vendor support. I just did a check, that BLU G53 is using PowerVR GPU, and the vendor OpenCL support is So may be you can try with When trying, you can try to look at the android log, it at least can tell you something. And regarding the nnapi do not has performance improvement, it might be most operations is falling back to use CPU because the nnapi module not supporting it, it just a guess, but you can also check it out in the android log. There's a way to check for the vendor provided lib name too, by using device explorer in android studio. But there's not standard directory it store, it could be inside |
Thanks for the info. Since there’s so many different options of Android devices, is there a way to know how to best use the config setting with our negatively impacting some devices by accident? any thoughts on the why |
Well, I can't answer you why the I was unaware of including other GPU library, may affect the entire loading though, this needs more checking, may be because when you include I think that better case will be include a working library directly inside the package, and load it, or may be scan the device vendor support to select the proper lib. But it is much more complicated |
Thank you. All makes sense. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does as intended. Device support varies which is separate issue.
Looks good to me @mrousavy
jsut to update, that this PR + #112 is working well when I set my I am going to Thank you @TkTioNG |
Allow setting needed gpu library for expo/expo-managed app through expo config plugins.
NOTE
uses-native-library
even works on expo-managed app, i think it don't? Unless it works, else it is a WIP.fix: #84 (comment)