Skip to content
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

[js/rn] Supoort New Architecture #16669

Open
wants to merge 48 commits into
base: main
Choose a base branch
from

Conversation

jhen0409
Copy link
Contributor

@jhen0409 jhen0409 commented Jul 12, 2023

Description

I’m gradually migrating my app projects to New Architecture. While the new architecture still supports legacy bridge for “module”, but usually there will be a lot of calls with onnxruntime-react-native, the new Turbo Native Module will help to improve the performance, so I make it a high priority in my work. These are what I did in this PR:

  • Support Turbo Native Module
  • For E2E project, it needs to enable the new architecture for the entire app project to test the changes
    • In React Native v0.69, it requires complex setup to enable new architecture. Starting with 0.71 it just need one flag to enable, so I upgrade the project to 0.71 version by template. (Currently Detox is supported React Native <= v0.71)
    • It’s still old-arch compatible for module, so I haven’t change the MNISTDataHandler demo module
    • The project still use old-arch by default, Detox probably not supported yet for new-arch
  • I still use RN v0.69 for react_native unit tests, it has some build issues from v0.71 on Android

Motivation and Context

Related #16031 because the interface of Turbo Native Module is based on JSI. I still keep JSIHelper because Turbo Module’s type conversation still does not seem to support buffer.

With new architecture enabled, It improve latency issue from bridge I mentioned in #16031 (comment), the latency obvious drop in my cases.

@fs-eire fs-eire requested review from YUNQIUGUO and edgchen1 July 12, 2023 06:50
@jhen0409
Copy link
Contributor Author

thanks again for the contribution.

another general question: If I understand correctly, to enable the new architecture for testing:

On Android, need to set buildconfig.IS_NEW_ARCHITECTURE_ENABLED (which is defined in build.gradle) and this option can also be passed into ./gradlew command?

For E2E project, you can change newArchEnabled=false to true.

On iOS, just need to set the ENV and uncomment the lines in Podfile?

# Use New Architecture
# ENV['RCT_NEW_ARCH_ENABLED'] = '1'

Yes.

@YUNQIUGUO
Copy link
Contributor

/azp run ONNX Runtime React Native CI Pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@fs-eire
Copy link
Contributor

fs-eire commented Sep 6, 2023

/azp run ONNX Runtime React Native CI Pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jhen0409
Copy link
Contributor Author

jhen0409 commented Sep 7, 2023

After updating RN to 0.71 in js/react_native, I forget to update the podfile. It should work now.

@fs-eire
Copy link
Contributor

fs-eire commented Sep 9, 2023

/azp run ONNX Runtime React Native CI Pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@fs-eire
Copy link
Contributor

fs-eire commented Sep 11, 2023

/azp run ONNX Runtime React Native CI Pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@fs-eire
Copy link
Contributor

fs-eire commented Sep 30, 2023

/azp run ONNX Runtime React Native CI Pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@fs-eire
Copy link
Contributor

fs-eire commented Nov 13, 2023

/azp run ONNX Runtime React Native CI Pipeline

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@fs-eire
Copy link
Contributor

fs-eire commented Nov 15, 2023

/azp run ONNX Runtime React Native CI Pipeline

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform:mobile issues related to ONNX Runtime mobile; typically submitted using template
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants