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.

Sign up for GitHub

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

Jump to bottom

If Branch Constant Folding #18105

Merged
merged 65 commits into from
Nov 14, 2023
Merged

If Branch Constant Folding #18105

merged 65 commits into from
Nov 14, 2023

Conversation

yuslepukhin
Copy link
Member

@yuslepukhin yuslepukhin commented Oct 26, 2023 •
edited
Loading

Description

When and if If condition proves to be a constant value, inline the corresponding subgraph yielding to more constant folding and optimization.

Motivation and Context

Newly converted models feature lots of nested If nodes that can be inlined and collapsed.

In particular, for the sample models we are gaining on TorchScript exported models.
For HF Mobile Bert Dynamo runtime went down from 0.069 -> 0.046. In total, AOT inlining + If constant folding
yields improvement of about 50% 0.102 -> 0.046. Brining us very close to TorchScript exported models.

HF Bart Dynamo further improves 0.668 -> 0.45. AOT + If constant folding improves 0.98 -> 0.45

Earlier the size of
HF Mobile Bert 161Mb+, now 98Mb
HF Bart Dynamo pre-optimized model was about 1.2Gb. It is now 710MB

image

@yuslepukhin
Add InlineSubgraph
0de61fa 
Complete Finalize
@yuslepukhin
AOT Inlining
9bfaabf
@yuslepukhin
Add Span() based defs back
5637ad4
@yuslepukhin
Lint
9da7a1e
@yuslepukhin
Implement If node ConstantFolding.
c3419ad 
Currently, there is a problem QDQ pairing transformer that ensures pairing between
QDQ pairs including within the graph.
@yuslepukhin
Merge branch 'main' into yuslepukhin/aot_inline
3da463d
@yuslepukhin
Address some review comments
bf3a3b2
@yuslepukhin
Revert InlineFunction
18df323
@yuslepukhin
Remove local functions
5a19dcf
@yuslepukhin
Common graph_viewer
7b3efdc
@yuslepukhin
Merge branch 'main' into yuslepukhin/aot_inline
2d56e5a
@yuslepukhin
Merge branch 'yuslepukhin/aot_inline' into yuslepukhin/if_constant_fo…
fd8be36 
…lding
@yuslepukhin
Remove spurious test, lint.
c5ee067
@yuslepukhin
Merge branch 'yuslepukhin/aot_inline' into yuslepukhin/if_constant_fo…
7c1f9a6 
…lding
@yuslepukhin
Prevent If Constant Fold in the QDQ test
4845104
@yuslepukhin
If Constant Folding non-recursive
8dc17a3
@yuslepukhin
HF Bart works
c81c1bf
@yuslepukhin
Bug fixes
cce536f
@yuslepukhin
Merge branch 'main' into yuslepukhin/aot_inline
39439bb
@yuslepukhin
Do not remove non-inlined function definitins.
d91740c
@yuslepukhin
Merge branch 'yuslepukhin/aot_inline' into yuslepukhin/if_constant_fo…
bcb402f 
…lding
@yuslepukhin
Add topo order. HF Bert works.
04c6fd6
@yuslepukhin
Compute function id from domain, OpType of the node.
c3c20b4 
  Change the type of the templates map so memory is partially released
  when not all of the functions are removed.
  Change the inline naming schema with slashes.
@yuslepukhin
Compute function id from domain, OpType of the node.
1108cc1 
  Change the type of the templates map so memory is partially released
  when not all of the functions are removed.
  Change the inline naming schema with slashes.
@yuslepukhin
Revert back to underscore
c945f96
@yuslepukhin
Merge branch 'yuslepukhin/aot_inline' into yuslepukhin/if_constant_fo…
6a8897d 
…lding
@yuslepukhin
Add kill switch for AOT
2db2e59 
  Add tests
  Rename the function.
  Remove functions directly from the partitioner.
@yuslepukhin
Merge branch 'yuslepukhin/aot_inline' into yuslepukhin/if_constant_fo…
c86919b 
…lding
@yuslepukhin
build error
d9106d1
@yuslepukhin
Merge branch 'yuslepukhin/aot_inline' into yuslepukhin/if_constant_fo…
4468fc1 
…lding
gramalingam
gramalingam previously approved these changes Nov 8, 2023
View reviewed changes
edgchen1
edgchen1 previously approved these changes Nov 8, 2023
View reviewed changes
@snnn
Copy link
Member

snnn commented Nov 8, 2023

/azp run Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, orttraining-ortmodule-distributed

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@yuslepukhin
Immediate consequetive nested Ifs inlining causes an issue
a17b3e0 
  when GraphViewer is not able to topologically sort new nodes
  due to lack of edges. We generate edges and make sure that
  nodes with subgraphs get implicit inputs setup.
@yuslepukhin
Merge branch 'main' into yuslepukhin/if_constant_folding
60d99fb
@yuslepukhin yuslepukhin dismissed stale reviews from edgchen1 and gramalingam via 60d99fb November 10, 2023 01:36
pranavsharma
pranavsharma reviewed Nov 14, 2023
View reviewed changes
Copy link
Contributor

@pranavsharma pranavsharma left a comment

Choose a reason for hiding this comment

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

Some minor comments.

onnxruntime/core/optimizer/constant_folding.cc Show resolved Hide resolved
include/onnxruntime/core/graph/graph.h Show resolved Hide resolved
include/onnxruntime/core/graph/graph.h Show resolved Hide resolved
onnxruntime/core/graph/graph.cc Show resolved Hide resolved
onnxruntime/core/graph/graph.cc Show resolved Hide resolved
@yuslepukhin yuslepukhin merged commit f19c673 into main Nov 14, 2023
84 of 89 checks passed
@yuslepukhin yuslepukhin deleted the yuslepukhin/if_constant_folding branch November 14, 2023 01:33
@yuslepukhin yuslepukhin mentioned this pull request Nov 16, 2023
Merged
petermcaughan pushed a commit that referenced this pull request Nov 20, 2023
@yuslepukhin
Create edges with arg positons correctly accounting for non-existing …
cc54202 
…args (#18462)

### Description
Truncate traling non-existing arguments.
  Make sure we do not skip on the non-existing arguments in the middle,
  because shape inferece relies on their proper position.
This also affects the argument position in the Edges that must be
properly rebuilt
  each time If node branch is inlined.
Make sure that when we rename Defs in subgraphs, new renamed defs are
created in those subgraphs
  instead of pointing to outer scope defs.
  Add unit test.

### Motivation and Context
This is a follow up for
#18105
Currently, the non-trailing arguments are simply ignored and the edges
are created
with potentially incorrect positions.
kleiti pushed a commit to kleiti/onnxruntime that referenced this pull request Mar 22, 2024
@yuslepukhin @kleiti
If Branch Constant Folding (microsoft#18105)
ee3ec27 
### Description

When and if `If` condition proves to be a constant value, inline the
corresponding subgraph yielding to more constant folding and
optimization.

### Motivation and Context
Newly converted models feature lots of nested `If` nodes that can be
inlined and collapsed.

In particular, for the sample models we are gaining on TorchScript
exported models.
For `HF Mobile Bert Dynamo` runtime went down from 0.069 -> 0.046. In
total, AOT inlining + `If` constant folding
yields improvement of about 50% 0.102 -> 0.046. Brining us very close to
TorchScript exported models.

`HF Bart Dynamo` further improves 0.668 -> 0.45. AOT + `If` constant
folding improves 0.98 -> 0.45

Earlier the size of 
HF Mobile Bert **161Mb+**, now **98Mb**
HF Bart Dynamo pre-optimized model was about **1.2Gb**. It is now
**710MB**


![image](https://github.com/microsoft/onnxruntime/assets/11303988/1491a247-d371-4e66-85a3-2aeb702e8ca0)
kleiti pushed a commit to kleiti/onnxruntime that referenced this pull request Mar 22, 2024
@yuslepukhin @kleiti
Create edges with arg positons correctly accounting for non-existing …
ba881f7 
…args (microsoft#18462)

### Description
Truncate traling non-existing arguments.
  Make sure we do not skip on the non-existing arguments in the middle,
  because shape inferece relies on their proper position.
This also affects the argument position in the Edges that must be
properly rebuilt
  each time If node branch is inlined.
Make sure that when we rename Defs in subgraphs, new renamed defs are
created in those subgraphs
  instead of pointing to outer scope defs.
  Add unit test.

### Motivation and Context
This is a follow up for
microsoft#18105
Currently, the non-trailing arguments are simply ignored and the edges
are created
with potentially incorrect positions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@edgchen1 edgchen1 edgchen1 left review comments

@skottmckay skottmckay skottmckay left review comments

@gramalingam gramalingam gramalingam approved these changes

@pranavsharma pranavsharma pranavsharma approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@yuslepukhin @snnn @skottmckay @pranavsharma @gramalingam @edgchen1