The Wayback Machine - https://web.archive.org/web/20210905221927/https://github.com/opencv/opencv/pull/19487
Skip to content

/ opencv

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

Jump to bottom

[G-API] Support multiple asynchronous requests #19487

Merged
merged 11 commits into from Feb 26, 2021
Merged

[G-API] Support multiple asynchronous requests #19487

merged 11 commits into from Feb 26, 2021

Conversation

@TolyaTalamanov
Copy link
Contributor

@TolyaTalamanov TolyaTalamanov commented Feb 9, 2021

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

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
Support nireq option
448962b
@TolyaTalamanov
TolyaTalamanov reviewed Feb 9, 2021
View changes
modules/gapi/include/opencv2/gapi/infer/ie.hpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ie/giebackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ie/giebackend.cpp Show resolved Hide resolved
modules/gapi/src/backends/ie/giebackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ie/giebackend.cpp Outdated Show resolved Hide resolved
@TolyaTalamanov
Merge branch 'master' of https://github.com/opencv/opencv into at/sup…
7ed698b 
…port-nireq-option
@TolyaTalamanov
Disable tests to check CI
dc468b6
@TolyaTalamanov
Fix bug with hanging
4c284db
@dmatveev
Copy link
Contributor

@dmatveev dmatveev commented Feb 12, 2021

It seems a rebase is required

@dmatveev
Copy link
Contributor

@dmatveev dmatveev commented Feb 12, 2021

@TolyaTalamanov can you please rebase to make the changeset clear?

@TolyaTalamanov
Merge branch 'master' of https://github.com/opencv/opencv into at/sup…
9bdd303 
…port-nireq-option
@TolyaTalamanov
TolyaTalamanov reviewed Feb 16, 2021
View changes
modules/gapi/src/compiler/gislandmodel.hpp Outdated Show resolved Hide resolved
modules/gapi/src/executor/conc_queue.hpp Outdated Show resolved Hide resolved
@TolyaTalamanov
WA to green CI
5d3a219
@dmatveev
dmatveev reviewed Feb 17, 2021
View changes
Copy link
Contributor

@dmatveev dmatveev left a comment

Reviewed ~40%, but didn't review the actual asynchronous pool part. Please clean-up code first (and, if possible, isolate changes for a single infer only.
modules/gapi/include/opencv2/gapi/infer/ie.hpp
@@ -67,6 +67,9 @@ namespace detail {
Kind kind;
bool is_generic;
IEConfig config;

// NB: Number of asyncrhonious infer requests
size_t nireq;

This comment has been minimized.

Sign in to view
@dmatveev

dmatveev Feb 17, 2021
Contributor

Just wondering why this and the above variables are not initialized by default.

Initially it was just num_in / num_out I've left empty for some reason (as those are always initialized in ctor), but now it doesn't looks so good.

This comment has been minimized.

Sign in to view
@TolyaTalamanov

TolyaTalamanov Feb 17, 2021
Author Contributor

Didn't get your point, now it is initialized in ctor. What's the problem ?

This comment has been minimized.

Sign in to view
@dmatveev

dmatveev Feb 19, 2021
Contributor

The problem is when a new ctor is added (who knows), these fields may left uninitialized

This comment has been minimized.

Sign in to view
@TolyaTalamanov

TolyaTalamanov Feb 26, 2021
Author Contributor

Partial initialization isn't supported in C++11

modules/gapi/include/opencv2/gapi/infer/ie.hpp Show resolved Hide resolved
modules/gapi/include/opencv2/gapi/infer/ie.hpp Outdated Show resolved Hide resolved
modules/gapi/include/opencv2/gapi/infer/ie.hpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ie/giebackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ie/giebackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ie/giebackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ie/giebackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ie/giebackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ie/giebackend.cpp Outdated Show resolved Hide resolved
@TolyaTalamanov TolyaTalamanov force-pushed the TolyaTalamanov:at/support-nireq-option branch 2 times, most recently from 2ada749 to 13d7c1b Feb 18, 2021
@TolyaTalamanov
Snapshot
eaa36ff
@TolyaTalamanov TolyaTalamanov force-pushed the TolyaTalamanov:at/support-nireq-option branch from 13d7c1b to 5db2378 Feb 19, 2021
@TolyaTalamanov
TolyaTalamanov reviewed Feb 19, 2021
View changes
modules/gapi/src/backends/ie/giebackend.cpp Show resolved Hide resolved
modules/gapi/src/backends/ie/giebackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ie/giebackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ie/giebackend.cpp Outdated Show resolved Hide resolved
@TolyaTalamanov TolyaTalamanov force-pushed the TolyaTalamanov:at/support-nireq-option branch 2 times, most recently from 19be9a6 to 3a97e94 Feb 19, 2021
@TolyaTalamanov
TolyaTalamanov reviewed Feb 19, 2021
View changes
modules/gapi/src/executor/gstreamingexecutor.cpp Show resolved Hide resolved
@TolyaTalamanov
TolyaTalamanov reviewed Feb 19, 2021
View changes
modules/gapi/src/backends/ie/giebackend.cpp Outdated Show resolved Hide resolved
@TolyaTalamanov TolyaTalamanov force-pushed the TolyaTalamanov:at/support-nireq-option branch from 3a97e94 to 161a880 Feb 19, 2021
@dmatveev
dmatveev reviewed Feb 19, 2021
View changes
Copy link
Contributor

@dmatveev dmatveev left a comment

Let's setup an overview call tomorrow.

What can be done for sure now is moving the async worker pool into something generic and unit-testable
modules/gapi/include/opencv2/gapi/infer/ie.hpp Show resolved Hide resolved
modules/gapi/src/backends/ie/giebackend.cpp Show resolved Hide resolved
modules/gapi/src/backends/ie/giebackend.cpp Show resolved Hide resolved
modules/gapi/src/backends/ie/giebackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ie/giebackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ie/giebackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ie/giebackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ie/giebackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ie/giebackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ie/giebackend.cpp Outdated Show resolved Hide resolved
@TolyaTalamanov
Merge branch 'master' of https://github.com/opencv/opencv into at/sup…
d265a86 
…port-nireq-option
@TolyaTalamanov TolyaTalamanov force-pushed the TolyaTalamanov:at/support-nireq-option branch 2 times, most recently from ff27092 to 115231f Feb 20, 2021
@TolyaTalamanov
Simplify RequestPool
ea0cf6f
@TolyaTalamanov TolyaTalamanov force-pushed the TolyaTalamanov:at/support-nireq-option branch from 115231f to ea0cf6f Feb 24, 2021
@dmatveev
dmatveev reviewed Feb 25, 2021
View changes
Copy link
Contributor

@dmatveev dmatveev left a comment

Approved if tests pass
modules/gapi/src/backends/ie/giebackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ie/giebackend.cpp Outdated Show resolved Hide resolved
@TolyaTalamanov
Add default values to id
f413e1c
@TolyaTalamanov
Copy link
Contributor Author

@TolyaTalamanov TolyaTalamanov commented Feb 26, 2021

@alalek

@alalek
alalek reviewed Feb 26, 2021
View changes
modules/gapi/src/backends/ie/giebackend.cpp
return requests;
}

class cv::gimpl::ie::RequestPool {

This comment has been minimized.

Sign in to view
@alalek

alalek Feb 26, 2021 •
edited
Contributor

class cv::gimpl::ie::RequestPool {
struct RequestPool;

Warning on "Custom Win" builder:

C:\build\precommit_custom_windows\opencv\modules\gapi\src\backends\ie\giebackend.cpp(497): warning C4099: 'cv::gimpl::ie::RequestPool': type name first seen using 'struct' now seen using 'class' [C:\build\precommit_custom_windows\build\modules\gapi\opencv_gapi.vcxproj]

"Custom Mac":

/build/precommit_custom_mac/opencv/modules/gapi/src/backends/ie/giebackend.cpp:497:1: warning: 'RequestPool' defined as a class here but previously declared as a struct; this is valid, but may result in linker errors under the Microsoft C++ ABI [-Wmismatched-tags]

This comment has been minimized.

Sign in to view
@TolyaTalamanov

TolyaTalamanov Feb 26, 2021
Author Contributor

Fixed

@TolyaTalamanov
Fix win warning
49a9547
@TolyaTalamanov
Copy link
Contributor Author

@TolyaTalamanov TolyaTalamanov commented Feb 26, 2021

@alalek

@alalek alalek added this to the 4.5.2 milestone Feb 26, 2021
@alalek alalek merged commit 28c064f into opencv:master Feb 26, 2021
6 checks passed
6 checks passed
@opencv-cn-ci-bot
OpenCV CN RISC-V Build finished.
Details
@opencv-cn-ci-bot
OpenCV CN RISC-V with RVV Build finished.
Details
@opencv-cn-ci-bot
OpenCV CN Ubuntu 18.04 x86-64 Build finished.
Details
@opencv-cn-ci-bot
OpenCV CN Ubuntu 20.04 arm64 Build finished.
Details
@opencv-cn-ci-bot
OpenCV CN Windows 10 x64 Build finished.
Details
@opencv-pushbot
default Required builds passed
Details
@alalek alalek mentioned this pull request Apr 9, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmatveev dmatveev

@alalek alalek

Assignees

@dmatveev dmatveev

Labels
category: g-api / gapi feature
Milestone
4.5.2
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
@TolyaTalamanov @dmatveev @alalek