Introduce RemovableAttributes #14868

xadupre · 2023-03-01T13:38:47Z

Description

TreeEnsemble* kernels fully copies all the parameters from the onnx graph. Even if they are no longer needed or unused (hitrates), they remain in memory. For big models >= 200 trees, max_depth > 10, the model usually weights more than 10 Mb. This change offers a kernel the possibility to remove all unneeded attributes after they were used to create the session. Attributes are deleted after the model was possibly saved, at the of the session creation.

The current design is to be debatted:

it stored the list of removable attributes in class onnxruntime::Node,
the node is marked as const everytime this implementation needs to register the name of a removable attribute or to remove them.

The current implementation is just a POC as it needs to cast onnxruntime::Node* into const onnxruntime::Node*.

Should we keep the list of removable attributes in onnxruntime::Node?

Motivation and Context

Motivation is mostly to reduce memory consumption.

…removeatt

include/onnxruntime/core/graph/graph.h

yuslepukhin · 2023-03-01T19:24:06Z

// Licensed under the MIT License.

This file contains no useful change (or so I think)

In reply to: 1450728870

Refers to: onnxruntime/core/framework/op_kernel_info.cc:2 in 6eed668. [](commit_id = 6eed668, deletion_comment = True)

onnxruntime/core/framework/session_state.h

onnxruntime/core/providers/cpu/ml/tree_ensemble_common.h

pranavsharma · 2023-03-01T20:36:41Z

This needs more thought. One idea is that each kernel can expose an API to return the set of attributes that it deems removable. Then in session state finalization we can go through all the kernels to retrieve these attributes and remove them from the corresponding graph node as the session state still holds a mutable version of the graph (and hence no const_cast required). We do something similar with pre-packed tensors. The kernel exposes an API to pre-pack a tensor and after the session state is done with calling PrePack on each kernel, we remove the corresponding initializer from the graph.

Edit: Obviously, not all kernels have to implement this API thereby relieving them of doing one more thing. Hence we can have a default implementation that returns an empty set of attrs.

xadupre · 2023-03-02T07:43:28Z

That would work. I tried to change the code to follow this idea. Let me know if this what you had in mind.

…removeatt

pranavsharma · 2023-03-02T20:53:41Z

include/onnxruntime/core/graph/graph.h

+  * Clears removable attributes. These are no longer needed after the initialization
+  * of the session.
+  */
+  int PruneRemovableAttributes(const InlinedVector<std::string>& removable_attributes);


Document what the return value indicates.

pranavsharma · 2023-03-02T20:56:03Z

onnxruntime/core/framework/session_state.cc

+  for (size_t i = 0; i < session_kernels_.size(); ++i) {
+    auto status = session_kernels_[i].get()->RemovableAttributes(removable_attributes);
+    if (!status.IsOK()) {
+      ORT_THROW("Kernel ", i, " failed at retrieving the removable attributes.");


I suppose we don't need to throw here because this is merely an optimization. We must log a warning though. Also, the index of the kernel doesn't really give us any useful information; would be good to log the node type and name.

pranavsharma · 2023-03-02T22:26:59Z

onnxruntime/core/session/inference_session.cc

@@ -1528,6 +1528,13 @@ common::Status InferenceSession::Initialize() {
        session_id_, model_->IrVersion(), model_->ProducerName(), model_->ProducerVersion(), model_->Domain(),
        model_->MainGraph().DomainToVersionMap(), model_->MainGraph().Name(), model_->MetaData(),
        telemetry_.event_name_, execution_providers_.GetIds(), model_has_fp16_inputs);
+
+    // once the model is saved, we may remove unnecessary attributes for inference


Can this be placed before the telemetry is logged?

pranavsharma · 2023-03-02T22:28:25Z

onnxruntime/core/graph/graph.cc

+  graph_->SetGraphResolveNeeded();
+  graph_->SetGraphProtoSyncNeeded();
+  int n_removed = 0;
+  for (auto name : removable_attributes) {


const auto& name

pranavsharma · 2023-03-02T22:31:24Z

include/onnxruntime/core/framework/op_kernel.h

+  // All attributes returned by this method will be removed by method
+  // PruneRemovableAttributes of they exists.
+  // @param removable_attributes set of attributes the session can safely remove.
+  virtual Status RemovableAttributes(InlinedVector<std::string>& removable_attributes) const {


Can this be renamed to GetRemovableAttributes?

pranavsharma · 2023-03-02T22:32:15Z

Can you add a test case to exercise this?

yuslepukhin · 2023-03-03T01:34:12Z

How does this affect exporting optimized model into flatbuffers and loading such an optimized model?

include/onnxruntime/core/graph/graph.h

onnxruntime/core/graph/graph.cc

…removeatt

xadupre · 2023-03-03T13:39:41Z

Can you add a test case to exercise this?

I did not add any because all test based on TreeEnsemble are going through the code I added. I'll try to see how I can add a few lines to make sure one attribute was removed.

I was thinking to add a test case like the following:

TEST(GraphRuntimeOptimizationTest, RemovableAttributes) {
  SaveAndLoadRuntimeOptimizationsForModel(
      ORT_TSTR("testdata/transform/SklearnHGBRegressorTrue.model.onnx"),
      ORT_TSTR("testdata/transform/runtime_optimization/SklearnHGBRegressorTrue.model.onnx"),
      [](const OpCountMap& loaded_ops, const OpCountMap& initialized_ops) {
        EXPECT_EQ(loaded_ops,
                  (OpCountMap{{"TreeEnsembleRegressor", 1}}));

        EXPECT_EQ(initialized_ops,
                  (OpCountMap{{"TreeEnsembleRegressor", 1}}));
      });
}

It checks saving the optimized model is not broken. It does not change the fact a removable attribute was removed. The public API of InferenceSession seems to give access to SessionState and then to the nodes. Is there a unit test I could use as a starting point for that?

onnxruntime/core/providers/cpu/ml/treeregressor.cc

onnxruntime/core/providers/cpu/ml/tree_ensemble_classifier.cc

Signed-off-by: xadupre <[email protected]>

…removeatt

yuslepukhin

### Description TreeEnsemble* kernels fully copies all the parameters from the onnx graph. Even if they are no longer needed or unused (hitrates), they remain in memory. For big models >= 200 trees, max_depth > 10, the model usually weights more than 10 Mb. This change offers a kernel the possibility to remove all unneeded attributes after they were used to create the session. Attributes are deleted after the model was possibly saved, at the of the session creation. The current design is to be debatted: * it stored the list of removable attributes in class `onnxruntime::Node`, * the node is marked as `const` everytime this implementation needs to register the name of a removable attribute or to remove them. The current implementation is just a POC as it needs to cast `onnxruntime::Node*` into `const onnxruntime::Node*`. Should we keep the list of removable attributes in `onnxruntime::Node`? ### Motivation and Context Motivation is mostly to reduce memory consumption. --------- Signed-off-by: xadupre <[email protected]>

xadupre added 2 commits March 1, 2023 09:15

removable attributes

ef2c15c

Merge branch 'main' of https://github.com/microsoft/onnxruntime into …

6eed668

…removeatt

xadupre added the core runtime issues related to core runtime label Mar 1, 2023

yuslepukhin reviewed Mar 1, 2023

View reviewed changes

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

yuslepukhin reviewed Mar 1, 2023

View reviewed changes

onnxruntime/core/framework/session_state.h Outdated Show resolved Hide resolved

yuslepukhin reviewed Mar 1, 2023

View reviewed changes

onnxruntime/core/providers/cpu/ml/tree_ensemble_common.h Outdated Show resolved Hide resolved

yuslepukhin reviewed Mar 1, 2023

View reviewed changes

onnxruntime/core/providers/cpu/ml/tree_ensemble_common.h Outdated Show resolved Hide resolved

xadupre added 7 commits March 2, 2023 13:39

Merge branch 'main' of https://github.com/microsoft/onnxruntime into …

cf06393

…removeatt

revert unnecessary changes

1045ae2

better design, no use of const operator

eea121a

small improvment

a52e71a

remove unnecessary changes

8d6d4c6

warning

89a2ff6

override

ffc68a6

pranavsharma reviewed Mar 2, 2023

View reviewed changes

yuslepukhin reviewed Mar 3, 2023

View reviewed changes

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

yuslepukhin reviewed Mar 3, 2023

View reviewed changes

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

Merge branch 'main' of https://github.com/microsoft/onnxruntime into …

0b19bfb

…removeatt

xadupre added 2 commits March 3, 2023 13:41

update the design with all the comments

780626e

skip null pointers

9b51b72

xadupre changed the title ~~[WIP] Introduce RemovableAttributes~~ Introduce RemovableAttributes Mar 3, 2023

yuslepukhin reviewed Mar 3, 2023

View reviewed changes

onnxruntime/core/providers/cpu/ml/treeregressor.cc Outdated Show resolved Hide resolved

yuslepukhin reviewed Mar 3, 2023

View reviewed changes

onnxruntime/core/providers/cpu/ml/treeregressor.cc Outdated Show resolved Hide resolved

yuslepukhin reviewed Mar 3, 2023

View reviewed changes

onnxruntime/core/providers/cpu/ml/tree_ensemble_classifier.cc Outdated Show resolved Hide resolved

xadupre added 2 commits March 4, 2023 08:47

use swap

00811bc

Signed-off-by: xadupre <[email protected]>

Merge branch 'main' of https://github.com/microsoft/onnxruntime into …

d9d9b5e

…removeatt

yuslepukhin approved these changes Mar 6, 2023

View reviewed changes

pranavsharma approved these changes Mar 6, 2023

View reviewed changes

xadupre merged commit 5930e7e into microsoft:main Mar 7, 2023

xadupre deleted the removeatt branch November 7, 2024 10:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce RemovableAttributes #14868

Introduce RemovableAttributes #14868

xadupre commented Mar 1, 2023 •

edited

Loading

yuslepukhin commented Mar 1, 2023 •

edited

Loading

pranavsharma commented Mar 1, 2023 •

edited

Loading

xadupre commented Mar 2, 2023 •

edited

Loading

pranavsharma Mar 2, 2023

pranavsharma Mar 2, 2023

xadupre Mar 3, 2023

pranavsharma Mar 2, 2023

xadupre Mar 3, 2023

pranavsharma Mar 2, 2023

xadupre Mar 3, 2023

pranavsharma Mar 2, 2023

xadupre Mar 3, 2023

pranavsharma commented Mar 2, 2023

yuslepukhin commented Mar 3, 2023

xadupre commented Mar 3, 2023 •

edited

Loading

yuslepukhin left a comment

Introduce RemovableAttributes #14868

Introduce RemovableAttributes #14868

Conversation

xadupre commented Mar 1, 2023 • edited Loading

Description

Motivation and Context

yuslepukhin commented Mar 1, 2023 • edited Loading

pranavsharma commented Mar 1, 2023 • edited Loading

xadupre commented Mar 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pranavsharma commented Mar 2, 2023

yuslepukhin commented Mar 3, 2023

xadupre commented Mar 3, 2023 • edited Loading

yuslepukhin left a comment

Choose a reason for hiding this comment

xadupre commented Mar 1, 2023 •

edited

Loading

yuslepukhin commented Mar 1, 2023 •

edited

Loading

pranavsharma commented Mar 1, 2023 •

edited

Loading

xadupre commented Mar 2, 2023 •

edited

Loading

xadupre commented Mar 3, 2023 •

edited

Loading