Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions include/onnxruntime/core/graph/graph.h
Original file line number Diff line number Diff line change
Expand Up @@ -1843,6 +1843,12 @@ class Graph { // NOLINT(clang-analyzer-optin.performance.Padding): preserve exi
std::unordered_map<std::string_view, NodeIndex> node_name_to_index;
std::unordered_set<Node*> nodes_with_subgraphs;

// Subgraphs that already had type/shape inferencing performed during this Resolve pass via the
// containing op's inference function (e.g. Scan/If/Loop). The "verify subgraphs" loop in
// VerifyNodeAndOpMatch uses this to avoid redundantly re-verifying the same subgraph, which
// would otherwise cause exponential re-traversal of deeply nested subgraphs.
std::unordered_set<const Graph*> inferred_subgraphs;

// check if the provided name is an input/initialize/node output of this Graph instance during Graph::Resolve.
// Graph::node_args_ can have stale entries so we can't rely on that.
bool IsLocalValue(const std::string& name) const;
Expand All @@ -1856,6 +1862,7 @@ class Graph { // NOLINT(clang-analyzer-optin.performance.Padding): preserve exi
inputs_and_initializers.clear();
node_name_to_index.clear();
nodes_with_subgraphs.clear();
inferred_subgraphs.clear();
}

private:
Expand Down
19 changes: 18 additions & 1 deletion onnxruntime/core/graph/graph.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ using namespace ::onnxruntime::common;
namespace onnxruntime {

#if !defined(ORT_MINIMAL_BUILD)

#define NO_CHANGE_ON_SYNC_FLAG(...) \
do { \
const bool sync_needed = GraphProtoSyncNeeded(); \
Expand Down Expand Up @@ -3179,6 +3180,15 @@ Status Graph::InferAndVerifySubgraphTypes(const Node& node, Graph& subgraph,
status = subgraph.PerformTypeAndShapeInferencing(options);
ORT_RETURN_IF_ERROR(status);

// Record that this subgraph had type/shape inferencing (and thus node/op verification via
// VerifyNodeAndOpMatch) performed here through the containing op's inference function
// (Scan/If/Loop and similar). The parent's "verify subgraphs" loop uses this to skip a redundant
// VerifyNodeAndOpMatch on the same subgraph, avoiding exponential re-traversal of deeply nested
// subgraphs.
if (subgraph.parent_graph_ != nullptr) {
subgraph.parent_graph_->resolve_context_.inferred_subgraphs.insert(&subgraph);
}

auto& subgraph_outputs = subgraph.GetOutputs();
for (const auto* output : subgraph_outputs) {
output_types.push_back(output->TypeAsProto());
Expand Down Expand Up @@ -3672,7 +3682,14 @@ Status Graph::VerifyNodeAndOpMatch(const ResolveOptions& options) {
}
}

ORT_RETURN_IF_ERROR(subgraph->VerifyNodeAndOpMatch(options));
// Skip verification if this subgraph already had type/shape inferencing (and node/op
// verification) performed via the containing op's inference function (e.g. Scan/If/Loop).
// This avoids exponential re-traversal of deeply nested subgraphs. Ops whose inference
// function does not descend into subgraphs (e.g. BeamSearch) are not recorded, so their
// subgraphs are still verified here.
if (resolve_context_.inferred_subgraphs.count(subgraph) == 0) {
ORT_RETURN_IF_ERROR(subgraph->VerifyNodeAndOpMatch(options));
}
Comment thread
skottmckay marked this conversation as resolved.
}
}

Expand Down
25 changes: 25 additions & 0 deletions onnxruntime/test/ir/graph_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License.

#include <iostream>
#include <chrono>
#include <fstream>
#include "core/common/inlined_containers.h"
#include "core/common/span_utils.h"
Expand Down Expand Up @@ -3806,5 +3807,29 @@ TEST_F(GraphTest, GH_Issue_29071_HasExternalDataInMemory) {
ASSERT_STATUS_OK(session_object.Initialize());
}

// Regression test for exponential subgraph type/shape inferencing.
// A model with deeply nested Loop nodes (each subgraph containing a single nested Loop) previously
// triggered O(2^depth) re-traversal during Graph::Resolve because both InferAndVerifyTypeMatch and
// the "verify subgraphs" loop in VerifyNodeAndOpMatch independently recursed into every subgraph.
// With 30 levels that made model loading take many minutes/hours. The fix memoizes subgraphs that
// already had type/shape inferencing performed, collapsing the work back to O(depth). This test
// simply verifies the model loads well within a generous time bound.
TEST_F(GraphTest, DeeplyNestedLoopSubgraphsResolveInReasonableTime) {
const auto start = std::chrono::steady_clock::now();

std::shared_ptr<Model> model;
ASSERT_STATUS_OK(Model::Load(ORT_TSTR("testdata/30_nested_loops.onnx"), model, nullptr, *logger_));

const auto elapsed = std::chrono::steady_clock::now() - start;
const auto elapsed_seconds = std::chrono::duration_cast<std::chrono::seconds>(elapsed).count();

// Without the memoization fix this takes many minutes (exponential in the nesting depth).
// A generous 60s bound reliably distinguishes the fixed O(depth) behavior from the regression
// without being flaky on slow/debug builds.
EXPECT_LT(elapsed_seconds, 60) << "Loading the 30-level nested Loop model took " << elapsed_seconds
<< "s, which suggests the subgraph type/shape inferencing recursion "
"regression has returned.";
}

} // namespace test
} // namespace onnxruntime
Binary file added onnxruntime/test/testdata/30_nested_loops.onnx
Binary file not shown.
Loading