Implement async version for InferList & Infer2 by TolyaTalamanov · Pull Request #19709 · opencv/opencv

TolyaTalamanov · 2021-03-11T11:35:19Z

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

I agree to contribute to the project under Apache 2 License.
To the best of my knowledge, the proposed patch is not based on a code under GPL or other license that is incompatible with OpenCV
The PR is proposed to proper branch
There is reference to original bug report and related work
There is accuracy test, performance test and test data in opencv_extra repository, if applicable
Patch to opencv_extra has the same branch name.
The feature is well documented and sample code can be built with the project CMake

Overview

This PR brings functionality to run InferList and Infer2 asynchronously.

Build configuration

Xforce_builders_only=linux,docs
force_builders=Custom,Custom Win,Custom Mac
build_gapi_standalone:Linux x64=ade-0.1.1f
build_gapi_standalone:Win64=ade-0.1.1f
build_gapi_standalone:Mac=ade-0.1.1f
build_gapi_standalone:Linux x64 Debug=ade-0.1.1f

Xbuild_image:Custom=centos:7
Xbuildworker:Custom=linux-1
build_gapi_standalone:Custom=ade-0.1.1f

build_image:Custom=ubuntu-openvino-2021.1.0:20.04
build_image:Custom Win=openvino-2021.1.0
build_image:Custom Mac=openvino-2021.1.0

test_modules:Custom=gapi
test_modules:Custom Win=gapi
test_modules:Custom Mac=gapi

buildworker:Custom=linux-1
# disabled due high memory usage: test_opencl:Custom=ON
test_opencl:Custom=OFF
test_bigdata:Custom=1
test_filter:Custom=*

TolyaTalamanov · 2021-03-11T11:36:30Z

@AsyaPronina @rgarnov @smirnov-alexey Have a look, please

TolyaTalamanov · 2021-03-17T10:06:10Z

@AsyaPronina @rgarnov @smirnov-alexey Could you have a look, please ?

modules/gapi/src/backends/ie/giebackend.cpp

smirnov-alexey

Overall, looks good

rgarnov

Looks good

TolyaTalamanov · 2021-03-17T15:01:49Z

@dmatveev Could you have a look ?

AsyaPronina · 2021-03-17T18:35:23Z

modules/gapi/src/backends/ie/giebackend.cpp

+        std::shared_ptr<IECallContext> ctx;
+        std::vector<std::vector<int>> cached_dims;
+    };
+    std::shared_ptr<Priv> m_priv;


Do we really need this if class is only in implementation part?

Yes, because it contains atomic which doesn't have copy ctor

Another reason to implement it such way is: object should be shared between async requests

AsyaPronina · 2021-03-17T18:45:35Z

modules/gapi/src/backends/ie/giebackend.cpp

+    if (finished == size) {
+        for (auto i : ade::util::iota(ctx->uu.params.num_out)) {
+            auto output = ctx->output(i);
+            ctx->out.meta(output, cv::GRunArg::Meta{});


I am not sure that we can somehow retrieve input meta here from input args, but is it okay to post empty meta?

Yes, it isn't ok and well-known problem, but this PR about: "Support async execution for InferList and Infer2".
But anyway I can fix it in this PR.

We shouldn't introduce new problems now if they are well known, so please address it here too.

Agree, fixed

AsyaPronina · 2021-03-17T18:51:02Z

modules/gapi/src/backends/ie/giebackend.cpp

+            for (auto i : ade::util::iota(ctx->uu.params.num_out)) {
+                auto output = ctx->output(i);
+                ctx->out.meta(output, cv::GRunArg::Meta{});
+                ctx->out.post(std::move(output));


this code is duplicated, may be we can retrieve it to some common method/function?

Yea, I think it can implemented as a separate function to reuse code

AsyaPronina · 2021-03-17T18:55:39Z

modules/gapi/src/backends/ie/giebackend.cpp

+            cached_dims[i] = toCV(ie_out->getTensorDesc().getDims());
+            // FIXME: Isn't this should be done automatically
+            // by some resetInternalData(), etc? (Probably at the GExecutor level)
+            ctx->outVecR<cv::Mat>(i).clear();


this code is also duplicated, how much does it cost to retrieve it to some common place?
Do we have some obstacles and this work seems extra?

This isn't my code, so I wouldn't touch at least in this PR, moreover it already has a comment about moving this to common place

dmatveev · 2021-03-18T07:19:37Z

modules/gapi/src/backends/ie/giebackend.cpp

+    if (finished == size) {
+        for (auto i : ade::util::iota(ctx->uu.params.num_out)) {
+            auto output = ctx->output(i);
+            ctx->out.meta(output, cv::GRunArg::Meta{});


We shouldn't introduce new problems now if they are well known, so please address it here too.

modules/gapi/src/backends/ie/giebackend.cpp

TolyaTalamanov · 2021-03-18T11:32:29Z

modules/gapi/src/backends/ie/giebackend.cpp

+            // FIXME: Isn't this should be done automatically
+            // by some resetInternalData(), etc? (Probably at the GExecutor level)
+            auto& out_vec = ctx->outVecR<cv::Mat>(i);
+            out_vec.clear();


In fact it isn't necessary because we reset it by calling Mat::create in callback, but I think call clear explicitly is clearer

dmatveev

👍

dmatveev · 2021-03-18T15:39:58Z

modules/gapi/src/backends/ie/giebackend.cpp

+        IE::Blob::Ptr out_blob = req.GetBlob(ctx->uu.params.output_names[i]);
+        GAPI_Assert(out_blob);
+
+        cv::Mat out_mat(cached_dims[i], toCV(out_blob->getTensorDesc().getPrecision()));


It seems you don't need this mat here?

Damn, silly mistake, done

TolyaTalamanov · 2021-03-18T20:17:36Z

@alalek Can it be merged ?

…t-infer2 G-API: Implement async version for InferList & Infer2 * Implement async version for InferList & Infer2 * Fix warning * Fix bug with roi ordering * Post input meta instead of empty * Fix comments to review

Implement async version for InferList & Infer2

9c968a8

TolyaTalamanov requested review from AsyaPronina, rgarnov and smirnov-alexey March 11, 2021 11:36

Fix warning

34e5990

smirnov-alexey reviewed Mar 17, 2021

View reviewed changes

modules/gapi/src/backends/ie/giebackend.cpp Outdated Show resolved Hide resolved

smirnov-alexey reviewed Mar 17, 2021

View reviewed changes

modules/gapi/src/backends/ie/giebackend.cpp Show resolved Hide resolved

smirnov-alexey reviewed Mar 17, 2021

View reviewed changes

modules/gapi/src/backends/ie/giebackend.cpp Show resolved Hide resolved

smirnov-alexey approved these changes Mar 17, 2021

View reviewed changes

rgarnov approved these changes Mar 17, 2021

View reviewed changes

TolyaTalamanov requested a review from dmatveev March 17, 2021 15:01

AsyaPronina reviewed Mar 17, 2021

View reviewed changes

dmatveev reviewed Mar 18, 2021

View reviewed changes

Fix bug with roi ordering

5574013

TolyaTalamanov force-pushed the at/async-inferlist-infer2 branch from b9b6592 to f6c0fc1 Compare March 18, 2021 11:29

TolyaTalamanov commented Mar 18, 2021

View reviewed changes

dmatveev approved these changes Mar 18, 2021

View reviewed changes

dmatveev self-assigned this Mar 18, 2021

dmatveev added this to the 4.5.2 milestone Mar 18, 2021

Post input meta instead of empty

1ee49d8

TolyaTalamanov force-pushed the at/async-inferlist-infer2 branch from f6c0fc1 to 1ee49d8 Compare March 18, 2021 17:13

Fix comments to review

7bf2935

alalek merged commit dc31e20 into opencv:master Mar 18, 2021

alalek mentioned this pull request Apr 9, 2021

(5.x) Merge 4.x #19885

Merged

Uh oh!

Conversation

TolyaTalamanov commented Mar 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Readiness Checklist

Overview

Build configuration

Uh oh!

TolyaTalamanov commented Mar 11, 2021

Uh oh!

TolyaTalamanov commented Mar 17, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

smirnov-alexey left a comment

Choose a reason for hiding this comment

Uh oh!

rgarnov left a comment

Choose a reason for hiding this comment

Uh oh!

TolyaTalamanov commented Mar 17, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AsyaPronina Mar 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TolyaTalamanov Mar 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dmatveev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TolyaTalamanov commented Mar 18, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

TolyaTalamanov commented Mar 11, 2021 •

edited

Loading

AsyaPronina Mar 17, 2021 •

edited

Loading

TolyaTalamanov Mar 17, 2021 •

edited

Loading