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

[GPU] Add GPU plugin metric to get statistics of GPU memory allocated by engine #7758

Merged
merged 2 commits into from
Oct 13, 2021
Merged

[GPU] Add GPU plugin metric to get statistics of GPU memory allocated by engine #7758

merged 2 commits into from
Oct 13, 2021

Conversation

andrew-k-park
Copy link
Contributor

Details:

  • Add GPU plugin metric to get statistics of GPU memory allocated by engine for each allocation type

Tickets:

  • 56842

@andrew-k-park andrew-k-park added the category: GPU OpenVINO GPU plugin label Sep 30, 2021
@andrew-k-park andrew-k-park added this to the 2022.1 milestone Sep 30, 2021
@yeonbok yeonbok changed the title [GPU] Add GPU plugin metric to get statistics of GPU memory allocated by engine WIP: Do not review [GPU] Add GPU plugin metric to get statistics of GPU memory allocated by engine Sep 30, 2021
@andrew-k-park andrew-k-park force-pushed the mem_statistics branch 5 times, most recently from 3c2b682 to b08b35b Compare October 7, 2021 09:28
@andrew-k-park andrew-k-park changed the title WIP: Do not review [GPU] Add GPU plugin metric to get statistics of GPU memory allocated by engine [GPU] Add GPU plugin metric to get statistics of GPU memory allocated by engine Oct 7, 2021
@andrew-k-park andrew-k-park marked this pull request as ready for review October 7, 2021 09:30
@andrew-k-park andrew-k-park requested review from a team as code owners October 7, 2021 09:30
@andrew-k-park andrew-k-park requested review from a team and avladimi and removed request for a team, geunhwan, yeonbok, kelvinchoi-intel and ahnyoung-paul October 7, 2021 09:30
@andrew-k-park andrew-k-park added the category: inference OpenVINO Runtime library - Inference label Oct 7, 2021
@andrew-k-park
Copy link
Contributor Author

@vladimir-paramuzov @iefode @avladimi Could you review this PR?

@vladimir-paramuzov vladimir-paramuzov enabled auto-merge (squash) October 8, 2021 08:27
@andrew-k-park
Copy link
Contributor Author

@iefode @avladimi Ping again. Could you review this PR?

@andrew-k-park andrew-k-park added category: docs OpenVINO documentation category: IE Tests OpenVINO Test: plugins and common labels Oct 12, 2021
myshevts
myshevts reviewed Oct 12, 2021
View reviewed changes
inference-engine/src/cldnn_engine/cldnn_executable_network.cpp
if (m_context != nullptr) {
auto impl = getContextImpl(m_context);
impl->acquire_lock();
std::shared_ptr<cldnn::engine> eng = impl->GetEngine();
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not a clDNNPlugin expert, but where is a loop over multiples graphs (streams)?

Copy link
Contributor

Choose a reason for hiding this comment

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

If this is NOT per-network, but overall (plugin) mem footprint as the https://github.com/openvinotoolkit/openvino/pull/7758/files#diff-0931e76736f996144670536e6a3712a65ad7277637ff564b7511fe8b827223a1R112 suggests... then the metric should be for the plugin, not for the ExecNetwork

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The engine object of cldnn library already stores the accumulated memory usage in multiple graphs. And since the requirement of this PR is to query memory usage statistics after LoadNetwork, we decided to implement it as ExecNetwork API.

Copy link
Contributor

@vladimir-paramuzov vladimir-paramuzov Oct 13, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@andrew-k-park I think @myshevts has fair point and @ilya-lavrenov mentioned the same thing in jira. If it's device-level statistics, then the query should be added to Core.
For executable network we can track memory consumption of this particular network, but currently it's just memory usage snapshot for the whole device.

Copy link
Contributor

Choose a reason for hiding this comment

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

As I understand

  • [This PR] statistics for ExecNetwork Metric : this will show the entire memory footprint of the networks including all streams requested for that networks.
  • statistics for Core Metric if we add in the future: this will show the entire memory footprint of all networks & all streams in the target device.

In this sense I could not understand what @vladimir-paramuzov mentioned as "but currently it's just memory usage snapshot for the whole device." ...

Copy link
Contributor

@vladimir-paramuzov vladimir-paramuzov Oct 13, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@yeonbok
As I can see, current impl works as follows:

auto net1 = ie.LoadNetwork(model1);
auto stat1 = net1.GetMetric(GPU_METRIC_KEY(MEMORY_STATISTICS)); // mem consumption for net1 only
auto net2 = ie.LoadNetwork(model2);
auto stat2 = net2.GetMetric(GPU_METRIC_KEY(MEMORY_STATISTICS)); // mem consumption for net1 + net2

stat2 contains unexpected values for metric in executable network.

How it should work:

auto net1 = ie.LoadNetwork(model1);
auto stat1 = net1.GetMetric(GPU_METRIC_KEY(MEMORY_STATISTICS)); // mem consumption for net1 only
auto net2 = ie.LoadNetwork(model2);
auto stat2 = net2.GetMetric(GPU_METRIC_KEY(MEMORY_STATISTICS)); // mem consumption for net2 only
auto stat_global = core.GetMetric(GPU_METRIC_KEY(MEMORY_STATISTICS)); // mem consumption for net1+net2

Copy link
Contributor

Choose a reason for hiding this comment

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

The latter was our intention, too. @andrew-k-park Could yuu confirm the behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yeonbok Based on the current implementation, multiple executable networks have same context impl, So the first case is correct. @vladimir-paramuzov My question is, is the latter your expected implementation to meet the requirement?

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought that the context impl is created for each ExecNetwork. If it is not true and the current behavior is not as we intended, I think we need to come up with an additional update so that we could be alinged with the initial concept. Current behavior is somewhat strange in the sense that it is implemented in Network. Lets come up with additional fix. Thanks for the review @myshevts @vladimir-paramuzov

myshevts
myshevts reviewed Oct 12, 2021
View reviewed changes
inference-engine/thirdparty/clDNN/api/cldnn/runtime/engine.hpp
@@ -109,6 +109,10 @@ class engine {
/// Returns the amount of GPU memory specified allocation @p type that currently used by the engine
uint64_t get_used_device_memory(allocation_type type) const;

/// Returns statistics of GPU memory allocated by engine in current process for all allocation types.
Copy link
Contributor

@myshevts myshevts Oct 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

if this is current process only, how do I track the overall GPU device memory utilization (e.g. how much free me I have to load another network)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, It's impossible to query information about overall GPU memory through multiple networks or multiple processes because only total size of memory information can retrieved from GPU device, and memory usage is managed by engine object in cldnn library through the recently called LoadNetwork API, So memory from multiple loaded networks must be tracked on application side.

@vladimir-paramuzov vladimir-paramuzov merged commit 4e2cc3e into openvinotoolkit:master Oct 13, 2021
vladimir-paramuzov
vladimir-paramuzov reviewed Oct 13, 2021
View reviewed changes
inference-engine/thirdparty/clDNN/runtime/engine.cpp
memory_usage = iter->second.load();
}
return memory_usage;
}

void engine::get_memory_statistics(std::map<std::string, uint64_t>* statistics) const {
Copy link
Contributor

Choose a reason for hiding this comment

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

btw, why statistics is an argument of this method? can it be

std::map<std::string, uint64_t> engine::get_memory_statistics() const

?

@andrew-k-park
Add GPU plugin metric to get statistics of GPU memory allocated by en…
9bcd299 
…gine for each allocation type

Signed-off-by: Andrew Kwangwoong Park <[email protected]>
@andrew-k-park
Use << operatorfor allocation_type and update docs
a1ea29a 
Signed-off-by: Andrew Kwangwoong Park <[email protected]>
@andrew-k-park andrew-k-park mentioned this pull request Nov 5, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yeonbok yeonbok yeonbok left review comments

@vladimir-paramuzov vladimir-paramuzov vladimir-paramuzov approved these changes

@myshevts myshevts myshevts left review comments

@ilya-lavrenov ilya-lavrenov ilya-lavrenov approved these changes

@avladimi avladimi Awaiting requested review from avladimi avladimi is a code owner automatically assigned from openvinotoolkit/openvino-docs-maintainers

Assignees

@vladimir-paramuzov vladimir-paramuzov

Labels
category: docs OpenVINO documentation category: GPU OpenVINO GPU plugin category: IE Tests OpenVINO Test: plugins and common category: inference OpenVINO Runtime library - Inference
Projects
None yet
Milestone
2022.1
Development

Successfully merging this pull request may close these issues.
5 participants
@andrew-k-park @ilya-lavrenov @myshevts @vladimir-paramuzov @yeonbok