[js/rn] Implement blob exchange by JSI instead of use base64 #16094

jhen0409 · 2023-05-25T06:41:02Z

Description

Create OnnxruntimeJSIHelper native module to provide two JSI functions
- jsiOnnxruntimeStoreArrayBuffer: Store buffer in Blob Manager & return blob object (iOS: RCTBlobManager, Android: BlobModule)
- jsiOnnxruntimeResolveArrayBuffer: Use blob object to get buffer
- The part of implementation is reference to react-native-blob-jsi-helper
Replace base64 encode/decode
- loadModelFromBlob: Rename from loadModelFromBase64EncodedBuffer
- run: Use blob object to replace input.data & results[].data

For this context, it saved a lot of time and avoid JS thread blocking in decode return type, it is 3700ms -> 5~20ms for the case. (resolve function only takes 0.x ms)

Motivation and Context

It’s related to #16031, but not a full implementation for migrate to JSI.

It just uses JSI through BlobManager to replace the slow part (base64 encode / decode).

Rewriting it entirely in JSI could be complicated, like type convertion and threading. This PR might be considered a minor change.

/cc @skottmckay

jhen0409 · 2023-05-25T06:52:52Z

@microsoft-github-policy-service agree

fs-eire · 2023-05-27T01:12:50Z

The typescript part looks good to me.

I don't know much details about JSI. I have 2 questions regarding the implementation:

is there a way to export jsiOnnxruntimeStoreArrayBuffer and jsiOnnxruntimeResolveArrayBuffer to the module object instead of global? In general exporting to global is not preferred.
is the lifecycle of the allocated ArrayBuffers correctly managed? I want to make sure that buffers are not released too early (otherwise a UB in memory may happen) or too late/never (a memory leak)

jhen0409 · 2023-05-27T03:06:36Z

is there a way to export jsiOnnxruntimeStoreArrayBuffer and jsiOnnxruntimeResolveArrayBuffer to the module object instead of global? In general exporting to global is not preferred.

It probably can't, because the install function is use RCT_EXPORT_BLOCKING_SYNCHRONOUS_METHOD not a JSI function, so we can't use JSI object as argument or return type.

The official way is use Turbo Native Module, but it required new architecture enabled and currently not stable.

We can just delete it in global after installation, but not perfect.

is the lifecycle of the allocated ArrayBuffers correctly managed? I want to make sure that buffers are not released too early (otherwise a UB in memory may happen) or too late/never (a memory leak)

Currently the implementation is take the ArrayBuffer constructor from JS, so it should be managed by the JS runtime. Also it remove blob in blobManager after it becomes ArrayBuffer.

For iOS, the blob (NSData*) should dealloc by ARC. For Android, the bytes reference (jbyte*) should managed by Java.

UPDATE: I found some memory leak issue on load model with my examples, not sure if it is related to this PR (it seems fine by patch this to v1.14), will investigate this.

jhen0409 · 2023-05-27T04:21:01Z

UPDATE: I found some memory leak issue on load model with my examples, not sure if it is related to this PR (it seems fine by patch this to v1.14), will investigate this.

Confirmed it is not related to this PR, looks like some load model behaviors are changed from v1.14, and it's bug I think. I'll open another issue or PR. (#16131)

fs-eire

I think it is good for the typescript part and my concerns are resolved. Please wait for @skottmckay review the android/ios part.

js/react_native/android/src/main/cpp/cpp-adapter.cpp

YUNQIUGUO

thanks for the contribution!

js/react_native/ios/TensorHelper.mm

js/react_native/android/CMakeLists.txt

js/react_native/android/src/main/cpp/cpp-adapter.cpp

js/react_native/android/src/main/java/ai/onnxruntime/reactnative/OnnxruntimeJSIHelper.java

js/react_native/android/src/main/cpp/cpp-adapter.cpp

js/react_native/android/src/main/java/ai/onnxruntime/reactnative/OnnxruntimeJSIHelper.java

js/react_native/ios/OnnxruntimeModuleTest/TensorHelperTest.mm

skottmckay · 2023-06-12T08:11:43Z

/azp run Windows ARM64 QNN CI Pipeline,Windows CPU CI Pipeline,Windows GPU CI Pipeline,Windows GPU TensorRT CI Pipeline,ONNX Runtime Web CI Pipeline,Linux CPU CI Pipeline,Linux CPU Minimal Build E2E CI Pipeline,Linux GPU CI Pipeline,Linux GPU TensorRT CI Pipeline,Linux OpenVINO CI Pipeline

skottmckay · 2023-06-12T08:11:45Z

/azp run Linux QNN CI Pipeline,MacOS CI Pipeline,orttraining-amd-gpu-ci-pipeline,orttraining-linux-ci-pipeline,orttraining-linux-gpu-ci-pipeline,orttraining-ortmodule-distributed,onnxruntime-python-checks-ci-pipeline,onnxruntime-binary-size-checks-ci-pipeline

azure-pipelines · 2023-06-12T08:12:17Z

Azure Pipelines successfully started running 7 pipeline(s).

azure-pipelines · 2023-06-12T08:12:23Z

Azure Pipelines successfully started running 10 pipeline(s).

jhen0409 · 2023-06-12T11:07:48Z

C:\a\_work\1\s\js\react_native\lib\backend.ts
  126:18  error  This assertion is unnecessary since it does not change the type of the expression  @typescript-eslint/no-unnecessary-type-assertion

✖ 1 problem (1 error, 0 warnings)
  1 error and 0 warnings potentially fixable with the `--fix` option.

Sorry about that, it doesn't happened in my environment, but it's the obvious part that can be corrected so I'll fix it.

skottmckay · 2023-06-12T21:42:57Z

/azp run Windows ARM64 QNN CI Pipeline,Windows CPU CI Pipeline,Windows GPU CI Pipeline,Windows GPU TensorRT CI Pipeline,ONNX Runtime Web CI Pipeline,Linux CPU CI Pipeline,Linux CPU Minimal Build E2E CI Pipeline,Linux GPU CI Pipeline,Linux GPU TensorRT CI Pipeline,Linux OpenVINO CI Pipeline

skottmckay · 2023-06-12T21:42:58Z

/azp run Linux QNN CI Pipeline,MacOS CI Pipeline,orttraining-amd-gpu-ci-pipeline,orttraining-linux-ci-pipeline,orttraining-linux-gpu-ci-pipeline,orttraining-ortmodule-distributed,onnxruntime-python-checks-ci-pipeline,onnxruntime-binary-size-checks-ci-pipeline

azure-pipelines · 2023-06-12T21:43:28Z

Azure Pipelines successfully started running 7 pipeline(s).

azure-pipelines · 2023-06-12T21:43:35Z

Azure Pipelines successfully started running 10 pipeline(s).

skottmckay · 2023-06-14T02:00:40Z

/azp run Windows ARM64 QNN CI Pipeline,Windows CPU CI Pipeline,Windows GPU CI Pipeline,Windows GPU TensorRT CI Pipeline,ONNX Runtime Web CI Pipeline,Linux CPU CI Pipeline,Linux CPU Minimal Build E2E CI Pipeline,Linux GPU CI Pipeline,Linux GPU TensorRT CI Pipeline,Linux OpenVINO CI Pipeline

skottmckay · 2023-06-14T02:00:42Z

/azp run Linux QNN CI Pipeline,MacOS CI Pipeline,orttraining-amd-gpu-ci-pipeline,orttraining-linux-ci-pipeline,orttraining-linux-gpu-ci-pipeline,orttraining-ortmodule-distributed,onnxruntime-python-checks-ci-pipeline,onnxruntime-binary-size-checks-ci-pipeline

azure-pipelines · 2023-06-14T02:01:41Z

Azure Pipelines successfully started running 7 pipeline(s).

azure-pipelines · 2023-06-14T02:01:45Z

Azure Pipelines successfully started running 10 pipeline(s).

skottmckay · 2023-06-15T22:28:46Z

Sorry - the PR to fix the broken CI pipeline needed a second fix. That is now checked in and the pipeline has been passing for the last day. Could you please update to the latest main one more time? Apologies for the inconvenience.

jhen0409 · 2023-06-15T23:20:56Z

Sorry - the PR to fix the broken CI pipeline needed a second fix. That is now checked in and the pipeline has been passing for the last day. Could you please update to the latest main one more time? Apologies for the inconvenience.

OK, updated.

skottmckay · 2023-06-16T00:21:28Z

/azp run Windows ARM64 QNN CI Pipeline,Windows CPU CI Pipeline,Windows GPU CI Pipeline,Windows GPU TensorRT CI Pipeline,ONNX Runtime Web CI Pipeline,Linux CPU CI Pipeline,Linux CPU Minimal Build E2E CI Pipeline,Linux GPU CI Pipeline,Linux GPU TensorRT CI Pipeline,Linux OpenVINO CI Pipeline

skottmckay · 2023-06-16T00:21:30Z

/azp run Linux QNN CI Pipeline,MacOS CI Pipeline,orttraining-amd-gpu-ci-pipeline,orttraining-linux-ci-pipeline,orttraining-linux-gpu-ci-pipeline,orttraining-ortmodule-distributed,onnxruntime-python-checks-ci-pipeline,onnxruntime-binary-size-checks-ci-pipeline

azure-pipelines · 2023-06-16T00:22:04Z

Azure Pipelines successfully started running 7 pipeline(s).

azure-pipelines · 2023-06-16T00:22:08Z

Azure Pipelines successfully started running 10 pipeline(s).

YUNQIUGUO · 2023-06-19T06:34:42Z

Did we manually run the ONNX Runtime React Native CI pipeline on this pr before? I think this change breaks the react native E2E test again...

@skottmckay

…ft#16094) ### Description  - Create `OnnxruntimeJSIHelper` native module to provide two JSI functions - `jsiOnnxruntimeStoreArrayBuffer`: Store buffer in Blob Manager & return blob object (iOS: RCTBlobManager, Android: BlobModule) - `jsiOnnxruntimeResolveArrayBuffer`: Use blob object to get buffer - The part of implementation is reference to [react-native-blob-jsi-helper](https://github.com/mrousavy/react-native-blob-jsi-helper) - Replace base64 encode/decode - `loadModelFromBlob`: Rename from `loadModelFromBase64EncodedBuffer` - `run`: Use blob object to replace input.data & results[].data For [this context](microsoft#16031 (comment)), it saved a lot of time and avoid JS thread blocking in decode return type, it is 3700ms -> 5~20ms for the case. (resolve function only takes 0.x ms) ### Motivation and Context  It’s related to microsoft#16031, but not a full implementation for migrate to JSI. It just uses JSI through BlobManager to replace the slow part (base64 encode / decode). Rewriting it entirely in JSI could be complicated, like type convertion and threading. This PR might be considered a minor change. /cc @skottmckay

jhen0409 added 7 commits May 25, 2023 06:17

iOS: Implement blob exchange by JSI instead of base64

d6035ad

[js/rn] Android: Implement blob exchange by JSI instead of base64

6043c7f

iOS: Remove unnecessary import

d070d33

Android: Fix cmake build on RN v0.69

1f442b2

Android: Update java tests

c21dc3c

iOS: Update objective-c tests

585d4dc

JS: Fix resolved buffer type

62893f3

snnn assigned skottmckay May 25, 2023

snnn requested review from fs-eire and skottmckay May 25, 2023 06:55

jhen0409 added 4 commits May 25, 2023 15:13

Android: Use getReactApplicationContext() for get blob manager

b5af470

Android: Revert unnecessary changes

1d336fa

iOS: Correct copyright for new test files

3c05e1d

Android: Fix build for React Native v0.71

7897572

jhen0409 force-pushed the jhen-rn-use-blob branch from e037a60 to 7897572 Compare May 26, 2023 05:31

TS: Remove global functions after installation

0728034

fs-eire previously approved these changes May 27, 2023

View reviewed changes

iOS: Delete blobManager ref on dealloc

d66f095

jhen0409 dismissed fs-eire’s stale review via d66f095 May 27, 2023 05:53

jhen0409 mentioned this pull request May 31, 2023

Use onnxruntime packages from fork for test purpose hans00/react-native-transformers-example#3

Draft

1 task

YUNQIUGUO reviewed Jun 1, 2023

View reviewed changes

js/react_native/android/src/main/cpp/cpp-adapter.cpp Outdated Show resolved Hide resolved

jhen0409 added 2 commits June 2, 2023 06:40

Android: Correct JSI function names

034d201

iOS: Correct JSI function names & add args count check

e5e5e6b

YUNQIUGUO reviewed Jun 2, 2023

View reviewed changes

jhen0409 added 2 commits June 2, 2023 09:57

Cleanup unnecessary includes & imports & logs

c6309db

Android: Refactor getBytesFromBlob / createBlob cpp functions

d4660bb

Merge branch 'main' into jhen-rn-use-blob

d212271

Fix lint error

f071c74

Merge remote-tracking branch 'origin' into jhen-rn-use-blob

f08ab3a

Merge branch 'main' into jhen-rn-use-blob

7f9c916

skottmckay approved these changes Jun 16, 2023

View reviewed changes

skottmckay merged commit ea1a5cf into microsoft:main Jun 16, 2023

jhen0409 deleted the jhen-rn-use-blob branch June 16, 2023 10:12

jhen0409 mentioned this pull request Jun 19, 2023

[js/rn] Fix CI pipeline by bump AGP version #16411

Closed

YUNQIUGUO mentioned this pull request Sep 28, 2023

[Mobile] Creating InferenceSession from Uint8Array gives an assertion error on the native side #17732

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[js/rn] Implement blob exchange by JSI instead of use base64 #16094

[js/rn] Implement blob exchange by JSI instead of use base64 #16094

jhen0409 commented May 25, 2023 •

edited

Loading

jhen0409 commented May 25, 2023

fs-eire commented May 27, 2023

jhen0409 commented May 27, 2023 •

edited

Loading

jhen0409 commented May 27, 2023 •

edited

Loading

fs-eire left a comment

YUNQIUGUO left a comment

skottmckay commented Jun 12, 2023

skottmckay commented Jun 12, 2023

azure-pipelines bot commented Jun 12, 2023

azure-pipelines bot commented Jun 12, 2023

jhen0409 commented Jun 12, 2023

skottmckay commented Jun 12, 2023

skottmckay commented Jun 12, 2023

azure-pipelines bot commented Jun 12, 2023

azure-pipelines bot commented Jun 12, 2023

skottmckay commented Jun 14, 2023

skottmckay commented Jun 14, 2023

azure-pipelines bot commented Jun 14, 2023

azure-pipelines bot commented Jun 14, 2023

skottmckay commented Jun 15, 2023

jhen0409 commented Jun 15, 2023

skottmckay commented Jun 16, 2023

skottmckay commented Jun 16, 2023

azure-pipelines bot commented Jun 16, 2023

azure-pipelines bot commented Jun 16, 2023

YUNQIUGUO commented Jun 19, 2023

[js/rn] Implement blob exchange by JSI instead of use base64 #16094

[js/rn] Implement blob exchange by JSI instead of use base64 #16094

Conversation

jhen0409 commented May 25, 2023 • edited Loading

Description

Motivation and Context

jhen0409 commented May 25, 2023

fs-eire commented May 27, 2023

jhen0409 commented May 27, 2023 • edited Loading

jhen0409 commented May 27, 2023 • edited Loading

fs-eire left a comment

Choose a reason for hiding this comment

YUNQIUGUO left a comment

Choose a reason for hiding this comment

skottmckay commented Jun 12, 2023

skottmckay commented Jun 12, 2023

azure-pipelines bot commented Jun 12, 2023

azure-pipelines bot commented Jun 12, 2023

jhen0409 commented Jun 12, 2023

skottmckay commented Jun 12, 2023

skottmckay commented Jun 12, 2023

azure-pipelines bot commented Jun 12, 2023

azure-pipelines bot commented Jun 12, 2023

skottmckay commented Jun 14, 2023

skottmckay commented Jun 14, 2023

azure-pipelines bot commented Jun 14, 2023

azure-pipelines bot commented Jun 14, 2023

skottmckay commented Jun 15, 2023

jhen0409 commented Jun 15, 2023

skottmckay commented Jun 16, 2023

skottmckay commented Jun 16, 2023

azure-pipelines bot commented Jun 16, 2023

azure-pipelines bot commented Jun 16, 2023

YUNQIUGUO commented Jun 19, 2023

jhen0409 commented May 25, 2023 •

edited

Loading

jhen0409 commented May 27, 2023 •

edited

Loading

jhen0409 commented May 27, 2023 •

edited

Loading