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

Add attrProto.release_s interface #22977

Merged
merged 11 commits into from
Dec 13, 2024

Conversation

danyue333
Copy link
Contributor

Description

Add AttributeProto.release_s interface, which is used to obtain the string in the attribute using move semantics instead of copying it

Motivation and Context

The ep_context node stores a lot of information in attributes, which may cause the memory usage to increase. Use this interface to avoid memory waste

@danyue333 danyue333 changed the title Add release attr api Add attrProto.release_s interface Dec 2, 2024
@danyue333
Copy link
Contributor Author

Hi @jywu-msft , Can you please review this PR?

@jywu-msft
Copy link
Member

/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, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline, onnxruntime-binary-size-checks-ci-pipeline, Linux QNN CI Pipeline

@jywu-msft
Copy link
Member

/azp run Windows ARM64 QNN CI Pipeline, Windows CPU CI Pipeline, Windows GPU TensorRT CI Pipeline, Windows x64 QNN CI Pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline

Copy link

Azure Pipelines successfully started running 9 pipeline(s).

Copy link

Azure Pipelines successfully started running 4 pipeline(s).

@jywu-msft
Copy link
Member

/azp run Big Models, Linux Android Emulator QNN CI Pipeline, Windows GPU CUDA CI Pipeline, Windows GPU DML CI Pipeline, Windows GPU Doc Gen CI Pipeline

Copy link

Azure Pipelines successfully started running 5 pipeline(s).

@jywu-msft
Copy link
Member

it's failing lint checks.

Warning (CLANGFORMAT) format
See https://clang.llvm.org/docs/ClangFormat.html.
Run lintrunner -a to apply this patch.

You can run `lintrunner -a` to apply this patch.

234  234 |   ModelProto* (*model_to_proto)(Model& model);                                                                                                        // [95]
235  235 |   DllSafe<std::string> (*model_proto_serialize_as_string)(ModelProto& model_proto);                                                                   // [96]
236  236 |   void (*model_proto_delete)(ModelProto* p);                                                                                                          // [97]
236      |-  DllSafe<std::string> (*attr_proto_release_string)(AttributeProto* attr);// [98]
     237 |+  DllSafe<std::string> (*attr_proto_release_string)(AttributeProto* attr);// [98]
238  238 | };
239  239 | 
240  240 | #ifndef USE_VITISAI

[2024-12-05T17:58:49Z DEBUG lintrunner::persistent_data] Writing run info to /home/cloudtest/.local/share/lintrunner/90a885e688310be9e42a550e10042c91e89bfa0b1be8a7cf0128dbb937830178/runs/2024-12-05T17-57-19.950Z_80015b8980a08df67e03e2596acef487771aed65e28b72d8c60a6140007f8e7c

You can reproduce these results locally by using lintrunner. To set up lintrunner locally, see https://github.com/microsoft/onnxruntime/blob/main/docs/Coding_Conventions_and_Standards.md#linting .
Error: Process completed with exit code 1.

@danyue333
Copy link
Contributor Author

lintrunner -a has been executed

@jywu-msft
Copy link
Member

/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, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline, onnxruntime-binary-size-checks-ci-pipeline, Linux QNN CI Pipeline

@jywu-msft
Copy link
Member

/azp run Windows ARM64 QNN CI Pipeline, Windows CPU CI Pipeline, Windows GPU TensorRT CI Pipeline, Windows x64 QNN CI Pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline

@jywu-msft
Copy link
Member

/azp run Big Models, Linux Android Emulator QNN CI Pipeline, Windows GPU CUDA CI Pipeline, Windows GPU DML CI Pipeline, Windows GPU Doc Gen CI Pipeline

Copy link

Azure Pipelines successfully started running 4 pipeline(s).

Copy link

Azure Pipelines successfully started running 9 pipeline(s).

Copy link

Azure Pipelines successfully started running 5 pipeline(s).

@jywu-msft jywu-msft merged commit 62e7e24 into microsoft:main Dec 13, 2024
77 checks passed
@danyue333 danyue333 deleted the add_release_attr_api branch December 13, 2024 07:20
guschmue pushed a commit that referenced this pull request Dec 14, 2024
### Description
Add AttributeProto.release_s interface, which is used to obtain the
string in the attribute using move semantics instead of copying it



### Motivation and Context
The ep_context node stores a lot of information in attributes, which may
cause the memory usage to increase. Use this interface to avoid memory
waste

---------

Co-authored-by: GenMing Zhong <[email protected]>
Co-authored-by: genmingz <[email protected]>
guschmue pushed a commit that referenced this pull request Dec 16, 2024
### Description
Add AttributeProto.release_s interface, which is used to obtain the
string in the attribute using move semantics instead of copying it



### Motivation and Context
The ep_context node stores a lot of information in attributes, which may
cause the memory usage to increase. Use this interface to avoid memory
waste

---------

Co-authored-by: GenMing Zhong <[email protected]>
Co-authored-by: genmingz <[email protected]>
guschmue pushed a commit that referenced this pull request Dec 20, 2024
### Description
Add AttributeProto.release_s interface, which is used to obtain the
string in the attribute using move semantics instead of copying it



### Motivation and Context
The ep_context node stores a lot of information in attributes, which may
cause the memory usage to increase. Use this interface to avoid memory
waste

---------

Co-authored-by: GenMing Zhong <[email protected]>
Co-authored-by: genmingz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants