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.

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

[GPU] fix memory conflict for multi iteration in loop. #28056

Closed
wants to merge 13 commits into from

Conversation

timxu826
Copy link
Contributor

Cause is shown in graph below.
image
The black, green, and dotted line donates the primitive dependency, memory buffer reuse, and memory conflict (at the second iteration) respectively.
The body of the loop is compiled as a separate model and is not aware the data reuse in backedge in multiple iteration.

Tickets:

@timxu826 timxu826 requested review from a team as code owners December 13, 2024 09:33
@github-actions github-actions bot added the category: GPU OpenVINO GPU plugin label Dec 13, 2024
@sys-openvino-ci sys-openvino-ci added the ExternalIntelPR External contributor from Intel label Dec 13, 2024
@@ -1009,6 +1009,12 @@ void loop_inst::set_memory_in_body_network(cldnn::network::ptr body_network,
"impl_params layout size(", impl_layout.to_short_string(),
") should not exceed memory size(", updated_mem->get_layout().to_short_string(), ")");
// Set need_to_check_memory_to_set to false to set output memory even if the input node has static shape,
if (impl_layout.bytes_count() < updated_mem->get_layout().bytes_count()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this check condition creates redundant memory copies in cases where there is no memory conflict.

Copy link
Contributor

Choose a reason for hiding this comment

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

@timxu826 From the description, you seem to resolve memory conflicts issue?
If so, is this enough?
Shouldn't we prevent memory reuse for the problematic case?

@p-durandin
Copy link
Contributor

build_jenkins

@ahnyoung-paul ahnyoung-paul added the pr: needs tests PR needs tests updating label Jan 9, 2025
@p-durandin
Copy link
Contributor

build_jenkins

@p-durandin
Copy link
Contributor

build_jenkins

@timxu826 timxu826 requested review from a team as code owners January 16, 2025 12:04
@github-actions github-actions bot added category: transformations OpenVINO Runtime library - Transformations category: LP transformations OpenVINO Low Precision transformations category: samples OpenVINO Runtime Samples category: IR FE OpenVINO IR v10 / v11 FrontEnd category: CI OpenVINO public CI category: docs OpenVINO documentation category: tools OpenVINO C++ / Python tools category: TEMPLATE OpenVINO Template plugin category: ONNX FE OpenVINO ONNX FrontEnd category: dependency_changes Pull requests that update a dependency file category: AUTO OpenVINO AUTO device selection plugin category: PDPD FE OpenVINO PaddlePaddle FrontEnd category: TF FE OpenVINO TensorFlow FrontEnd category: CPP API OpenVINO CPP API bindings category: packaging OpenVINO packaging / distribution category: AUTO BATCH OpenVINO Auto Batch plugin github_actions Pull requests that update GitHub Actions code category: PyTorch FE OpenVINO PyTorch Frontend category: TFL FE OpenVINO TensorFlow Lite FrontEnd category: JS API OpenVino JS API Bindings no-match-files category: docs_snippets OpenVINO docs snippets (docs/snippets) category: NPU OpenVINO NPU plugin category: licensing Changes in OpenVINO licenses category: dockerfiles category: JAX FE OpenVINO JAX FrontEnd category: OVC OVC tool category: NPUW NPUW plugin labels Jan 16, 2025
@timxu826 timxu826 closed this Jan 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: AUTO BATCH OpenVINO Auto Batch plugin category: AUTO OpenVINO AUTO device selection plugin category: build OpenVINO cmake script / infra category: C API OpenVINO C API bindings category: CI OpenVINO public CI category: Core OpenVINO Core (aka ngraph) category: CPP API OpenVINO CPP API bindings category: CPU OpenVINO CPU plugin category: dependency_changes Pull requests that update a dependency file category: dockerfiles category: docs_snippets OpenVINO docs snippets (docs/snippets) category: docs OpenVINO documentation category: GPU OpenVINO GPU plugin category: IE Tests OpenVINO Test: plugins and common category: inference OpenVINO Runtime library - Inference category: IR FE OpenVINO IR v10 / v11 FrontEnd category: JAX FE OpenVINO JAX FrontEnd category: JS API OpenVino JS API Bindings category: licensing Changes in OpenVINO licenses category: LP transformations OpenVINO Low Precision transformations category: NPU OpenVINO NPU plugin category: NPUW NPUW plugin category: ONNX FE OpenVINO ONNX FrontEnd category: OVC OVC tool category: packaging OpenVINO packaging / distribution category: PDPD FE OpenVINO PaddlePaddle FrontEnd category: Python API OpenVINO Python bindings category: PyTorch FE OpenVINO PyTorch Frontend category: samples OpenVINO Runtime Samples category: TEMPLATE OpenVINO Template plugin category: TF FE OpenVINO TensorFlow FrontEnd category: TFL FE OpenVINO TensorFlow Lite FrontEnd category: tools OpenVINO C++ / Python tools category: transformations OpenVINO Runtime library - Transformations ExternalIntelPR External contributor from Intel github_actions Pull requests that update GitHub Actions code no-match-files pr: needs tests PR needs tests updating under_perf_check
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants