-
Notifications
You must be signed in to change notification settings - Fork 51
feat: add C++ JSI interface and Executorch dependency #346
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
Port native implementation of style transfer to C++ #227. This is a part of a larger effort to merge native code from Kotlin and Objective C to a single implementation in C++. Old style transfer modules have been replaced by new, stateful implementation. For this purpose non-static versions of useModule and BaseModule have been introduced. Old implementations of style transfer have been removed. Added methods for handling fetching data via https that are passed on as function objects to C++. - Ported image processing capabilities equivalent to the ObjC/Kotlin implementations. - `ModelHostObject` serves as an automatic interface between model implementations and JSI. This is done by template metaprogramming; defining methods in `JsiConversions.h` for types used by the model is necessary for it to work. - A factory method for loading a style transfer model is registered in `RnExecutorchInstaller`. - `common/rnexecutorch/jsi` originates from react-native-audio-api - `base64.*` is used for converting from base64 due to https://renenyffenegger.ch/notes/development/Base64/Encoding-and-decoding-base-64-with-cpp/ - `ada` is a header-only library for parsing urls due to https://github.com/ada-url/ada - headers for Executorch 0.6 have been updated - OpenCV 4.11.0 dependency is introduced for Android/C++. For iOS the version is bumped via Cocoapods to 4.11.0. - C10 headers are the dependency of OpenCV. - Exceptions in native C++ do not result in promise getting rejected when forwarding yet. - Garbage collection can malfunction for host objects. C++ host objects need to notify JS runtime about their size via external memory pressure for the garbage collector to know to free them. This is not yet done.
## Description Fix exception handling in C++ native code so that it results in JS promise getting rejected with an appropriate message. There are two fixes here: 1. Runtime type information in the current version of React Native is disabled because one of its dependencies, which results in C++ not being able to recognize that `std::runtime_error` is inheriting from `std::exception` so to extract the exception message we need to match the type exactly. A fix has been introduced in RN repo, but not yet present in any release: facebook/react-native@3132cc8. 2. When rejecting a promise the old code was accessing JS runtime in an unsafe way, this is fixed now. ### Type of change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] Documentation update (improves or adds clarity to existing documentation) ### Tested on - [x] iOS - [x] Android
## Description In the C++ code we need some interface between raw JSI and our logic using JS promises. The current implementation uses too many nested lambdas, as well requires `std::function` bindings to each lambda. ### Type of change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] Documentation update (improves or adds clarity to existing documentation) ### Tested on - [x] iOS - [x] Android ### Related issues #255
## Description Add generic loading of host functions in C++. ### Type of change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] Documentation update (improves or adds clarity to existing documentation) ### Tested on - [x] iOS - [x] Android
## Description Make the model host function installation generic, so that you can easily introduce additional methods that resolve with a promise. ### Tested on - [x] iOS - [x] Android ### Related issues #255 ### Checklist - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have updated the documentation accordingly - [x] My changes generate no new warnings
Port image segmentation native code to C++. - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] Documentation update (improves or adds clarity to existing documentation) - [x] iOS - [x] Android - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have updated the documentation accordingly - [x] My changes generate no new warnings --------- Co-authored-by: Mateusz Sluszniak <[email protected]>
What exactly files should be reviewed in this PR? |
|
This is a cherry-pick of the main C++ PR which had the logic reviewed. The change compared to that PR is the adaptation to the monorepo structure, so the main review points should be the gradle/podspec setup. |
## Description Remove a dangling reference when handling exceptions in forward method in native C++. ### Type of change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] Documentation update (improves or adds clarity to existing documentation) ### Tested on - [x] iOS - [x] Android ### Checklist - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have updated the documentation accordingly - [x] My changes generate no new warnings
chmjkb
left a comment
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.
Checked out all of the examples and it looks good to me, thanks!
## Description Add C++ JSI interface and Executorch dependency for Android and iOS. For Android: - Add CMake integration for building C++ code. - Add JSI installer that is called from Kotlin via JNI. - Add Executorch C++ dependency by a dynamic library extracted from the .aar provided by an Executorch script. For iOS: - Add JSI installer in Objective C++. - Modify the podspec to statically link Executorch. The binaries are extracted from the xcframeworks provided by an Executorch script. Port style transfer and image segmentation to C++ for iOS/Android. ### Type of change - [ ] Bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] Documentation update (improves or adds clarity to existing documentation) ### Tested on - [x] iOS - [x] Android ### Checklist - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have updated the documentation accordingly - [ ] My changes generate no new warnings --------- Co-authored-by: Mateusz Sluszniak <[email protected]>
Description
Add C++ JSI interface and Executorch dependency for Android and iOS.
For Android:
For iOS:
Port style transfer and image segmentation to C++ for iOS/Android.
Type of change
Tested on
Checklist