Properly remove in-memory references by yuslepukhin · Pull Request #25652 · microsoft/onnxruntime

yuslepukhin · 2025-08-05T01:36:33Z

Description

Make Node::ToProto() const
call Graph::ToGraphProto() const so it does process all of the
subgraphs recursively and removes all the in-memory references.

Motivation and Context

Node::ToProto() const does not respect constness of subgraph attributes
and calls non-const version of Graph::ToGraphProto() which does not process
subgraph initializers and does not remove in-memory references.

call Graph::ToGraphProto() const so it does process all of the subgraphs recursively and removes all the in-memory references.

Copilot

Pull Request Overview

This PR fixes a constness violation in Node::ToProto() that was preventing proper removal of in-memory references from subgraphs. The changes ensure that the const version of Graph::ToGraphProto() is called, which properly processes subgraph initializers and removes in-memory references recursively.

Modifies Node::ToProto() to force calling the const version of Graph::ToGraphProto()
Refactors ProcessSubgraphsInMemoryData() to accept a main_graph parameter for better comparison logic
Updates documentation to clarify deprecated usage and parameter meanings

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
onnxruntime/core/graph/graph.cc	Implements constness fix in Node::ToProto() and refactors ProcessSubgraphsInMemoryData() parameter handling
include/onnxruntime/core/graph/graph.h	Updates method signature and adds deprecation documentation

onnxruntime/core/graph/graph.cc

include/onnxruntime/core/graph/graph.h

onnxruntime/core/graph/graph.cc

skottmckay

gedoensmax · 2025-08-06T14:31:36Z

Ok thank you, that does the trick for NV TRT EP.

Remove from Resolve().

onnxruntime/core/session/inference_session.cc

gaugarg-nv · 2025-08-07T06:59:32Z

@skottmckay Can you please tag this PR for cherry-picking into the 1.23.0 release? This is a blocker to run Phi models with TRT-RTX EP.

onnxruntime/core/graph/graph.cc

Make Node::ToProto() const call Graph::ToGraphProto() const so it does process all of the subgraphs recursively and removes all the in-memory references.  Node::ToProto() const does not respect constness of subgraph attributes and calls non-const version of Graph::ToGraphProto() which does not process subgraph initializers and does not remove in-memory references.

…5, 25652 (#25701) ### Description Cherry-pick the following PRs into the `rel-1.23.0` branch: - #25391 - #25611 - #25656 - #25346 - #25374 - #25664 - #25675 - #25652 ### Motivation and Context  --------- Co-authored-by: Yulong Wang <7679871+fs-eire@users.noreply.github.com> Co-authored-by: Ishwar Raut <iraut@nvidia.com> Co-authored-by: Maximilian Müller <44298237+gedoensmax@users.noreply.github.com> Co-authored-by: Gaurav Garg <gaugarg@nvidia.com> Co-authored-by: Scott McKay <skottmckay@gmail.com> Co-authored-by: Chi Lo <54722500+chilo-ms@users.noreply.github.com> Co-authored-by: Abhishek Jindal <abjindal@microsoft.com> Co-authored-by: Dmitri Smirnov <yuslepukhin@users.noreply.github.com>

### Description  Make Node::ToProto() const call Graph::ToGraphProto() const so it does process all of the subgraphs recursively and removes all the in-memory references. ### Motivation and Context  Node::ToProto() const does not respect constness of subgraph attributes and calls non-const version of Graph::ToGraphProto() which does not process subgraph initializers and does not remove in-memory references.

This reverts commit 3ca49d8. It also makes adjustments for the current source code state.

…lues early (#26345) ### Description Converts weights early and revert "Properly remove in-memory references (#25652)" This reverts commit 3ca49d8 and makes appropriate adjustments for the current state of the code. This PR is made possible and on the heels of: #26263 #25833. Previous history: #23979 #25320 #25626 #25652 The first change (#26263) allows us to convert initializers to OrtValues early and save lots of memory at model loading time. Specifically, for Phi-4-mini-instruct-INT4 model before and after looks like this: **Before** <img width="1204" height="124" alt="Before change DEBUG 2025-10-16 144819" src="https://github.com/user-attachments/assets/674ff75b-057f-498a-a906-0140d59d46e6" /> **After** <img width="997" height="114" alt="After change DEBUG 2025-10-16 144819" src="https://github.com/user-attachments/assets/df1783af-7f50-4cd2-b3ad-6868f23be53f" /> The two peaks represent memory usage at optimization time (8.1Gb before) and after weights memory mapping (6.5Gb) After this change corresponding numbers look 3.5Gb and 4.7Gb respectively. Most of the savings during optimization phase come from `ConstantFolding` where we are able to reuse the resulting OrtValues directly for the new initializers. This PR concludes a series of PRs converting initializers to OrtValues. Memory consumption before the conversion began was 9.3Gb and 6.7Gb respectively. We are saving almost 6Gb during optimization and 2Gb for the steady state. <img width="1175" height="139" alt="image" src="https://github.com/user-attachments/assets/80e7d228-8a8e-4316-8e04-b02c2be30f04" /> The model also loads about 12 seconds faster. Example of ConstantFolding being one of the top contributors where we duplicate memory for higher peak before Resolve takes care of no longer used initializers. <img width="1100" height="558" alt="Sanpshot 3 Peak on ConstantFolding Transpose Optimizer" src="https://github.com/user-attachments/assets/95545abd-3f99-46d9-862e-bbf27cbb5b40" /> <img width="1060" height="600" alt="Snapshot 4 Peak AddInitializer from ConstantFolding" src="https://github.com/user-attachments/assets/dd457ec6-23ee-4efd-8c60-625d5faad61e" /> <img width="325" height="160" alt="image" src="https://github.com/user-attachments/assets/37c1194d-f683-49a7-afb1-073dfbb9bbfc" /> ### Motivation and Context  Reduce memory usage.

Make Node::ToProto() const

858b960

call Graph::ToGraphProto() const so it does process all of the subgraphs recursively and removes all the in-memory references.

yuslepukhin requested review from adrianlizarraga, Copilot and skottmckay August 5, 2025 01:36

Copilot AI reviewed Aug 5, 2025

View reviewed changes

onnxruntime/core/graph/graph.cc Show resolved Hide resolved

onnxruntime/core/graph/graph.cc Outdated Show resolved Hide resolved

skottmckay reviewed Aug 5, 2025

View reviewed changes

include/onnxruntime/core/graph/graph.h Outdated Show resolved Hide resolved

onnxruntime/core/graph/graph.cc Outdated Show resolved Hide resolved

onnxruntime/core/graph/graph.cc Outdated Show resolved Hide resolved

onnxruntime/core/graph/graph.cc Outdated Show resolved Hide resolved

adrianlizarraga mentioned this pull request Aug 5, 2025

Bugfix vitisai ep model clone with 23979 25320 #25654

Closed

HectorSVC mentioned this pull request Aug 5, 2025

[VitisAI] bugfix model_clone optimization #25629

Merged

Address review comments, refactor recursion

78b3c6d

yuslepukhin mentioned this pull request Aug 5, 2025

OpenVINO EP fails to run models with in-memory external data #25304

Closed

yuslepukhin marked this pull request as ready for review August 5, 2025 22:58

skottmckay previously approved these changes Aug 6, 2025

View reviewed changes

skottmckay mentioned this pull request Aug 6, 2025

Draft: [NV TRT RTX EP] Fix onnx checker for constants in subgraph #25579

Closed

2 tasks

yuslepukhin added 2 commits August 6, 2025 14:10

Make optimizers add initializers inline instead externally

b9f4824

Convert initializers to OrtValues once just before paritioning.

2d1c46f

Remove from Resolve().

yuslepukhin dismissed skottmckay’s stale review via 2d1c46f August 6, 2025 23:19

Remove stray test

bb2ef3f

skottmckay reviewed Aug 6, 2025

View reviewed changes

onnxruntime/core/session/inference_session.cc Show resolved Hide resolved

yuslepukhin added 3 commits August 6, 2025 16:50

Add comment for the conversion call

e4a16f5

Merge branch 'main' into yuslepukhin/node_to_proto_subgraph_fix

e7f277c

Fix failing test, add more comments

5174725

skottmckay previously approved these changes Aug 7, 2025

View reviewed changes

skottmckay added the release:1.23.0 label Aug 7, 2025

Preserve initializer order in ToGraphProto()

314121b

yuslepukhin dismissed skottmckay’s stale review via 314121b August 7, 2025 18:06

Preserve the order in sync required case

0c2d91c

adrianlizarraga approved these changes Aug 7, 2025

View reviewed changes

skottmckay reviewed Aug 7, 2025

View reviewed changes

onnxruntime/core/graph/graph.cc Show resolved Hide resolved

yuslepukhin merged commit 3ca49d8 into main Aug 7, 2025
96 of 98 checks passed

yuslepukhin deleted the yuslepukhin/node_to_proto_subgraph_fix branch August 7, 2025 22:32

adrianlizarraga mentioned this pull request Aug 7, 2025

[1.23.0] Cherry-pick PR #25652 (properly remove in-memory references) #25691

Closed

adrianlizarraga mentioned this pull request Aug 8, 2025

1.23.0 cherry-pick prs 25391, 25611, 25656, 25346, 25374, 25664, 25675, 25652 #25701

Merged

jywu-msft removed the release:1.23.0 label Aug 28, 2025

yuslepukhin added a commit that referenced this pull request Oct 16, 2025

Revert "Properly remove in-memory references (#25652)"

13d2c37

This reverts commit 3ca49d8. It also makes adjustments for the current source code state.

yuslepukhin mentioned this pull request Oct 17, 2025

Save much memory at model loading time by converting weights to OrtValues early #26345

Merged

gedoensmax mentioned this pull request Dec 2, 2025

[Regression]: Phi-3.5-mini-instruct fails to load in-memory initializer for NvTensorRtRtx #26653

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Properly remove in-memory references#25652

Properly remove in-memory references#25652
yuslepukhin merged 10 commits intomainfrom
yuslepukhin/node_to_proto_subgraph_fix

yuslepukhin commented Aug 5, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

skottmckay left a comment

Uh oh!

gedoensmax commented Aug 6, 2025

Uh oh!

Uh oh!

gaugarg-nv commented Aug 7, 2025

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

yuslepukhin commented Aug 5, 2025

Description

Motivation and Context

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

skottmckay left a comment

Choose a reason for hiding this comment

Uh oh!

gedoensmax commented Aug 6, 2025

Uh oh!

Uh oh!

gaugarg-nv commented Aug 7, 2025

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants