Skip to content

[ONNX] Refactor delegate memory management #32661

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

[ONNX] Refactor delegate memory management #32661

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f01eae9
[ONNX] Refactor delegate memory management
mvafin Nov 3, 2025
0e632bc
Update src/frontends/onnx/frontend/src/frontend.cpp
mvafin Nov 3, 2025
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: 2 additions & 5 deletions src/frontends/onnx/frontend/include/openvino/frontend/onnx/decoder.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "openvino/core/type/element_type.hpp"
#include "openvino/frontend/decoder.hpp"
#include "openvino/frontend/onnx/visibility.hpp"
#include "openvino/runtime/aligned_buffer.hpp"

namespace ov {
namespace frontend {
Expand All @@ -15,12 +16,8 @@ namespace onnx {
struct ONNX_FRONTEND_API TensorMetaInfo {
ov::PartialShape m_partial_shape;
ov::element::Type m_element_type;
const uint8_t* m_tensor_data;
ov::Any m_tensor_data_any;
size_t m_tensor_data_size;
std::shared_ptr<ov::AlignedBuffer> m_buffer;
Copy link
Contributor

@bumbosiepsak bumbosiepsak Nov 4, 2025

Choose a reason for hiding this comment

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

Missing include:

#include <memory> // For shared_ptr

Copy link
Contributor

@bumbosiepsak bumbosiepsak Nov 4, 2025

Choose a reason for hiding this comment

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

Actually your AlignedBuffer already is a resource manager. Maybe you could keep it by value in TensorMetaInfo (instead of shared_ptr) in order to avoid managing the memory twice (and paying with memory fragmentation / non-locality)?

const std::string* m_tensor_name;
std::shared_ptr<std::string> m_external_location;
bool m_is_raw;
};

class ONNX_FRONTEND_API DecoderBase : public ov::frontend::DecoderBase {
Expand Down
2 changes: 0 additions & 2 deletions src/frontends/onnx/frontend/src/core/decoder_proto.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,6 @@ class DecoderProtoTensor : public ov::frontend::onnx::DecoderBaseTensor {
m_tensor_meta_info.m_tensor_name = &name;
m_tensor_meta_info.m_element_type = ov::element::dynamic;
m_tensor_meta_info.m_partial_shape = ov::PartialShape::dynamic();
m_tensor_meta_info.m_tensor_data = nullptr;
m_tensor_meta_info.m_tensor_data_size = 0;
}

const ov::frontend::onnx::TensorMetaInfo& get_tensor_info() const override {
Expand Down
319 changes: 170 additions & 149 deletions src/frontends/onnx/frontend/src/core/graph_iterator_proto.cpp
View file
Open in desktop

Large diffs are not rendered by default.

18 changes: 3 additions & 15 deletions src/frontends/onnx/frontend/src/core/node.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -797,11 +797,7 @@ Tensor Node::get_attribute_value(const std::string& name) const {
tensor_meta_info.m_partial_shape,
tensor_meta_info.m_element_type,
std::vector<std::string>{*tensor_meta_info.m_tensor_name},
tensor_meta_info.m_tensor_data,
tensor_meta_info.m_tensor_data_size,
tensor_meta_info.m_tensor_data_any,
tensor_meta_info.m_external_location,
tensor_meta_info.m_is_raw);
tensor_meta_info.m_buffer);
return {tensor_place};
}
FRONT_END_NOT_IMPLEMENTED(get_attribute_value);
Expand All @@ -824,11 +820,7 @@ SparseTensor Node::get_attribute_value(const std::string& name) const {
values_meta_info.m_partial_shape,
values_meta_info.m_element_type,
std::vector<std::string>{*values_meta_info.m_tensor_name},
values_meta_info.m_tensor_data,
values_meta_info.m_tensor_data_size,
values_meta_info.m_tensor_data_any,
values_meta_info.m_external_location,
values_meta_info.m_is_raw);
values_meta_info.m_buffer);

auto indices_decoder =
std::dynamic_pointer_cast<ov::frontend::onnx::DecoderBaseTensor>(sparse_tensor_info.m_indices);
Expand All @@ -838,11 +830,7 @@ SparseTensor Node::get_attribute_value(const std::string& name) const {
indices_meta_info.m_partial_shape,
indices_meta_info.m_element_type,
std::vector<std::string>{*indices_meta_info.m_tensor_name},
indices_meta_info.m_tensor_data,
indices_meta_info.m_tensor_data_size,
indices_meta_info.m_tensor_data_any,
indices_meta_info.m_external_location,
indices_meta_info.m_is_raw);
indices_meta_info.m_buffer);
return {values_place, indices_place, sparse_tensor_info.m_partial_shape};
}
FRONT_END_NOT_IMPLEMENTED(get_attribute_value);
Expand Down
200 changes: 66 additions & 134 deletions src/frontends/onnx/frontend/src/core/tensor.cpp
View file
Open in desktop

Large diffs are not rendered by default.

71 changes: 12 additions & 59 deletions src/frontends/onnx/frontend/src/core/tensor.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,19 +52,6 @@ inline std::vector<T> __get_data(const Container& container) {
#endif
}

template <typename T, typename SRC>
inline std::vector<T> __get_data(const void* data, const size_t data_size) {
#if defined(_MSC_VER)
# pragma warning(push)
# pragma warning(disable : 4267)
# pragma warning(disable : 4244)
#endif
return std::vector<T>(static_cast<const SRC*>(data), static_cast<const SRC*>(data) + data_size);
#if defined(_MSC_VER)
# pragma warning(pop)
#endif
}

template <typename T>
inline std::vector<T> __get_raw_data(const std::string& raw_data, int onnx_data_type) {
auto it = reinterpret_cast<const T*>(raw_data.data());
Expand All @@ -83,18 +70,10 @@ class TensorONNXPlace : public ov::frontend::onnx::TensorPlace {
const ov::PartialShape& pshape,
ov::element::Type type,
const std::vector<std::string>& names,
const void* data,
const size_t data_size,
const ov::Any& data_any,
std::shared_ptr<std::string> data_location,
const bool is_raw)
const std::shared_ptr<ov::AlignedBuffer>& buffer)
: ov::frontend::onnx::TensorPlace(input_model, pshape, type, names),
m_input_model(input_model),
m_data(data),
m_data_any(data_any),
m_data_size(data_size),
m_data_location(data_location),
m_is_raw(is_raw) {};
m_buffer(buffer) {};

void translate(ov::Output<ov::Node>& output);

Expand All @@ -119,24 +98,8 @@ class TensorONNXPlace : public ov::frontend::onnx::TensorPlace {
m_output_idx = idx;
}

const void* get_data() const {
return m_data;
}

size_t get_data_size() const {
return m_data_size;
}

const ov::Any get_data_any() const {
return m_data_any;
}

std::shared_ptr<std::string> get_data_location() const {
return m_data_location;
}

bool is_raw() const {
return m_is_raw;
std::shared_ptr<ov::AlignedBuffer> get_buffer() const {
return m_buffer;
}

detail::MappedMemoryHandles get_mmap_cache();
Expand All @@ -145,11 +108,7 @@ class TensorONNXPlace : public ov::frontend::onnx::TensorPlace {
protected:
int64_t m_input_idx = -1, m_output_idx = -1;
const ov::frontend::InputModel& m_input_model;
const void* m_data;
ov::Any m_data_any;
size_t m_data_size;
std::shared_ptr<std::string> m_data_location;
bool m_is_raw;
std::shared_ptr<ov::AlignedBuffer> m_buffer;
};

class Tensor {
Expand Down Expand Up @@ -298,19 +257,18 @@ class Tensor {
private:
bool has_external_data() const {
if (m_tensor_place != nullptr) {
return m_tensor_place->get_data_location() != nullptr;
return false;
}
return m_tensor_proto->has_data_location() &&
m_tensor_proto->data_location() == TensorProto_DataLocation::TensorProto_DataLocation_EXTERNAL;
}

template <typename T>
std::vector<T> get_external_data() const {
const auto ext_data = m_tensor_place != nullptr
? detail::TensorExternalData(*m_tensor_place->get_data_location(),
reinterpret_cast<size_t>(m_tensor_place->get_data()),
m_tensor_place->get_data_size())
: detail::TensorExternalData(*m_tensor_proto);
if (m_tensor_place != nullptr) {
FRONT_END_NOT_IMPLEMENTED(get_external_data);
}
const auto ext_data = detail::TensorExternalData(*m_tensor_proto);
std::shared_ptr<ov::AlignedBuffer> buffer = nullptr;
if (ext_data.data_location() == detail::ORT_MEM_ADDR) {
buffer = ext_data.load_external_mem_data();
Expand All @@ -327,7 +285,7 @@ class Tensor {
FRONT_END_THROW("Unexpected usage of method for externally stored data");
}
if (m_tensor_place != nullptr) {
return m_tensor_place->get_data();
FRONT_END_NOT_IMPLEMENTED(get_data_ptr);
}

if (m_tensor_proto->has_raw_data()) {
Expand All @@ -350,12 +308,7 @@ class Tensor {

size_t get_data_size() const {
if (m_tensor_place != nullptr) {
if (m_tensor_place->is_raw()) {
return m_tensor_place->get_data_size() /
get_onnx_data_size(ov_to_onnx_data_type(m_tensor_place->get_element_type()));
} else {
return m_tensor_place->get_data_size();
}
FRONT_END_NOT_IMPLEMENTED(get_data_size);
Copy link
Contributor

@bumbosiepsak bumbosiepsak Nov 4, 2025

Choose a reason for hiding this comment

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

Question: is it planned to be implemented?

Copy link
Contributor Author

@mvafin mvafin Nov 4, 2025

Choose a reason for hiding this comment

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

Maybe yes, or maybe will delete it later

}
if (has_external_data()) {
const auto ext_data = detail::TensorExternalData(*m_tensor_proto);
Expand Down
8 changes: 2 additions & 6 deletions src/frontends/onnx/frontend/src/input_model.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -651,11 +651,7 @@ std::shared_ptr<ov::frontend::onnx::TensorONNXPlace> decode_tensor_place(
tensor_meta_info.m_partial_shape,
tensor_meta_info.m_element_type,
std::vector<std::string>{*tensor_meta_info.m_tensor_name},
tensor_meta_info.m_tensor_data,
tensor_meta_info.m_tensor_data_size,
tensor_meta_info.m_tensor_data_any,
tensor_meta_info.m_external_location,
tensor_meta_info.m_is_raw);
tensor_meta_info.m_buffer);
return tensor_place;
}

Expand Down Expand Up @@ -688,7 +684,7 @@ void InputModel::InputModelONNXImpl::load_model() {
tensor_place->set_input_index(tensor_decoder->get_input_idx());
tensor_place->set_output_index(tensor_decoder->get_output_idx());
// Constant with data has been found
if (tensor_place->get_data() != nullptr)
if (tensor_place->get_buffer() != nullptr)
continue;
auto name = tensor_place->get_names()[0];
if (m_tensor_places.count(name) == 0) {
Expand Down
2 changes: 1 addition & 1 deletion src/frontends/onnx/frontend/src/translate_session.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ void TranslateSession::translate_graph(const ov::frontend::InputModel::Ptr& inpu
auto create_const_or_param = [&](const std::string& name,
const std::shared_ptr<ov::frontend::onnx::TensorONNXPlace>& input_tensor) {
std::shared_ptr<ov::Node> node;
if (input_tensor->get_data_location() != nullptr || input_tensor->get_data() != nullptr) {
if (input_tensor->get_buffer() != nullptr) {
Tensor tensor = Tensor(input_tensor);
node = tensor.get_ov_constant();
} else if (input_tensor->get_partial_shape() == PartialShape{0}) { // empty constant
Expand Down
6 changes: 5 additions & 1 deletion src/frontends/onnx/tests/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ set(SRC
skip_tests_config.cpp
../frontend/src/core/graph_iterator_proto.cpp
../frontend/src/core/decoder_proto.cpp
../frontend/src/utils/tensor_external_data.cpp
)

foreach(src IN LISTS SRC MULTI_TEST_SRC)
Expand Down Expand Up @@ -129,7 +130,10 @@ set_property(TEST ov_onnx_frontend_tests PROPERTY LABELS OV UNIT ONNX_FE)
add_dependencies(ov_onnx_frontend_tests openvino_template_extension)


target_include_directories(ov_onnx_frontend_tests PRIVATE "${CMAKE_CURRENT_SOURCE_DIR}")
target_include_directories(ov_onnx_frontend_tests PRIVATE
Copy link
Contributor

@bumbosiepsak bumbosiepsak Nov 4, 2025

Choose a reason for hiding this comment

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

I have a feeling, that objects in frontend/src belong to a separate / own target. Need of using relative paths traversing the file tree backwards indicates that.

In kosher CMake these libraries would export their interface directories. So that linking them would "infect" the current target with necessary paths. See: https://cmake.org/cmake/help/latest/prop_tgt/INTERFACE_INCLUDE_DIRECTORIES.html

Copy link
Contributor Author

@mvafin mvafin Nov 4, 2025

Choose a reason for hiding this comment

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

Yeah, those are files from frontend library. We include them here to not duplicate code (or to not create a separate target for decoder)

"${CMAKE_CURRENT_SOURCE_DIR}"
"${CMAKE_CURRENT_SOURCE_DIR}/../frontend/src"
)

target_compile_definitions(ov_onnx_frontend_tests
PRIVATE
Expand Down
Loading