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] Implement dispose native method #16131

Merged
merged 4 commits into from
Jun 8, 2023

Conversation

jhen0409
Copy link
Contributor

Description

Implement dispose react native method.

Motivation and Context

Currently we are not able to release the memory used by model in JS runtime if we don't want to use it anymore, we can do that only by reload app on debug or restart app on release.

@snnn snnn requested review from YUNQIUGUO, skottmckay and fs-eire May 30, 2023 04:28
@snnn
Copy link
Member

snnn commented May 30, 2023

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

@snnn
Copy link
Member

snnn commented May 30, 2023

/azp run Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, onnxruntime-binary-size-checks-ci-pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed

@azure-pipelines
Copy link

Azure Pipelines successfully started running 9 pipeline(s).

@azure-pipelines
Copy link

Azure Pipelines successfully started running 7 pipeline(s).

@YUNQIUGUO
Copy link
Contributor

/azp run ONNX Runtime React Native CI Pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@snnn snnn added the platform:mobile issues related to ONNX Runtime mobile; typically submitted using template label May 30, 2023
Copy link
Contributor

@YUNQIUGUO YUNQIUGUO left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the contribution. the dispose native methods for both android/ios look good to me.

I started a ORT React Native CI pipeline run to see if unit tests all pass.

may need to wait for other reviewers.

Copy link
Contributor

@fs-eire fs-eire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JS part looks good to me and I added another PR to add an JSAPI to allow session disposal.

fs-eire added a commit that referenced this pull request May 31, 2023
### Description
This change adds a new instance function (method) to type
`InferenceSession` to allow users to manually release an inference
session instance.

#16131 depends on this change to work correctly.
@skottmckay skottmckay merged commit ac8444f into microsoft:main Jun 8, 2023
@jhen0409 jhen0409 deleted the jhen-rn-impl-dispose branch June 9, 2023 00:45
siweic0 pushed a commit to siweic0/onnxruntime-web that referenced this pull request May 9, 2024
### Description
This change adds a new instance function (method) to type
`InferenceSession` to allow users to manually release an inference
session instance.

microsoft#16131 depends on this change to work correctly.
siweic0 pushed a commit to siweic0/onnxruntime-web that referenced this pull request May 9, 2024
### Description
<!-- Describe your changes. -->

Implement `dispose` react native method.

### Motivation and Context
<!-- - Why is this change required? What problem does it solve?
- If it fixes an open issue, please link to the issue here. -->

Currently we are not able to release the memory used by model in JS
runtime if we don't want to use it anymore, we can do that only by
reload app on debug or restart app on release.
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.

5 participants