Skip to content

Incrementally free initializers while saving to OrtValue instances - #12485

Merged
Scott McKay (skottmckay) merged 9 commits into
masterfrom
skottmckay/FreeInitializersWhileSavingToOrtValue
Aug 9, 2022
Merged

Incrementally free initializers while saving to OrtValue instances#12485
Scott McKay (skottmckay) merged 9 commits into
masterfrom
skottmckay/FreeInitializersWhileSavingToOrtValue

Conversation

@skottmckay

Copy link
Copy Markdown
Contributor

Description:
After copying each TensorProto to an OrtValue free the TensorProto. This helps reduce peak memory usage during model initialization vs. the current approach of copying all the initializers before freeing them.

Also fix an unnecessary conversion of the TensorProto name from std::string to const char*. The usage of that value required std::string so converting to const char* incurred an unnecessary std::string construction in every usage of the value.

Motivation and Context
Reduce peak memory usage to enable 1P production scenario.

For a ~13.5MB model the peak memory usage was reduced by ~10MB.

@hariharans29

Hariharan Seshadri (hariharans29) commented Aug 5, 2022

Copy link
Copy Markdown
Member

Just curious if it is possible/safe to just clear the under-lying ModelProto held the Model instance after its job is done in the Graph instance ?

Comment thread onnxruntime/core/framework/session_state_utils.cc Outdated
Co-authored-by: Pranav Sharma <prs@microsoft.com>
virtual common::Status GetPreallocatedBuffer(int ort_value_index, const char* name,
* @return
*/
virtual common::Status GetPreallocatedBuffer(int ort_value_index, const std::string& name,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

std::string&

Consider std::string_view to cover all string types?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The current usage only involves 'name' coming from a TensorProto so is always a std::string.

@skottmckay
Scott McKay (skottmckay) merged commit 56bd96a into master Aug 9, 2022
@skottmckay
Scott McKay (skottmckay) deleted the skottmckay/FreeInitializersWhileSavingToOrtValue branch August 9, 2022 00:59
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.

4 participants