-
Notifications
You must be signed in to change notification settings - Fork 913
Support React Native #118
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
base: main
Are you sure you want to change the base?
Support React Native #118
Conversation
Woah nice! There are a lot of people who are interested in doing stuff like this! Looking forward to reviewing this when you're ready! cc @pcuenca |
I'm investigating a performance issue with Whisper tiny.en, and it looks like the performance is not as expected in the example. I quickly add log for # Encode result
LOG ONNX session run finished 767 ms
# Decode result
LOG ONNX session run finished 4518 ms
# ... 4 runs ...
LOG ONNX session run finished 4856 ms
LOG Time: 40016
LOG Result: {"text": " (buzzing)"} Click to expandconst t0 = performance.now()
// this.#inferenceSession === NativeModules.Onnxruntime
const results: Binding.ReturnType = await this.#inferenceSession.run(this.#key, input, outputNames, options);
const output = this.decodeReturnType(results);
console.log('ONNX session run finished', performance.now() - t0) Also see logs of native part (added log to OnnxruntimeModule.java), it is significantly less than the time taken by JS part: # Encode result
09:47:59.203 ONNXRUNTIME_RN run() is finished, time: 273 ms
# Decode result
09:48:00.280 ONNXRUNTIME_RN run() is finished, time: 339 ms
# ... 4 runs ...
09:48:23.807 ONNXRUNTIME_RN run() is finished, time: 541 ms
EDIT: It seems the logger have some bugs leads me to think that the problem is from the native bridge. I add timeout await to the decodeReturnType call and found the issue is from this function. |
@xenova here is ready for review |
@hans00 , I'v seen a lot of commits from you on this lately. Thanks for that! Were you able to get this working? Do you know if it will be merged into the main project soon? |
@xenova ping |
Hi @hans00! 👋 Thanks so much for your work here - it will be very impactful. I'm finally able to commit more time to this, as I now have the ability to do mobile development in my environment. My main concern with the PR in its current state is the additional dependencies that it introduces. Ideally, the only new dependency that would be introduced is What do you think? |
Ok, I had removed JS codecs for RN. For FS access, |
Make it support React Native.
And I made a example for it.
https://github.com/hans00/react-native-transformers-example
On Android need some library patch.TODO:
Check models are works fineResearch more efficiently image processing