[G-API] Pipeline modeling tool: Skip frames by TolyaTalamanov · Pull Request #21719 · opencv/opencv

TolyaTalamanov · 2022-03-15T06:42:49Z

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 another license that is incompatible with OpenCV
The PR is proposed to the proper branch
There is a reference to the 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

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.4.1:20.04
build_image:Custom Win=openvino-2021.4.1
build_image:Custom Mac=openvino-2021.4.1

test_modules:Custom=gapi,python2,python3,java
test_modules:Custom Win=gapi,python2,python3,java
test_modules:Custom Mac=gapi,python2,python3,java

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

sivanov-work · 2022-03-15T10:28:49Z

modules/gapi/samples/pipeline_modeling_tool.cpp

+                auto call_every_nth =
+                    call_every_nth_opt.has_value() ? call_every_nth_opt.value() : 1;
+                if (call_every_nth <= 0) {
+                    throw std::logic_error(node_name + " call_every_nth must be greater than zero");


suggest to print value also

sivanov-work · 2022-03-15T10:35:35Z

modules/gapi/samples/pipeline_modeling_tool.cpp

                    auto output =
                        check_and_read<OutputDescr>(node_fn, "output", node_name);
-                    builder.addDummy(node_name, time, output);
+                    builder.addDummy(node_name, call_every_nth_u, time, output);


how's about making struct for call_every_nth_u and further possible params?

Do you mean like this:

builder.addDummy(NodeParams{node_name, call_every_nth_u}, time, output);

👍🏻 yes, something like that

sivanov-work · 2022-03-15T10:43:41Z

modules/gapi/samples/pipeline_modeling_tool/pipeline_builder.hpp

+            // G-API doesn't support dynamic number of inputs/outputs.
+            if (inputs.size() > 1u) {
+                throw std::logic_error(
+                        "skip_frame_nth is supported only for single input subgraphs");


printing of inputs.size() will help in troubleshooting

Good point.

sivanov-work · 2022-03-15T10:53:05Z

modules/gapi/samples/pipeline_modeling_tool/pipeline_builder.hpp

+            call.run(subgr_inputs, subgr_outputs);
+            auto comp = cv::GComputation(cv::GProtoInputArgs{subgr_inputs},
+                                         cv::GProtoOutputArgs{subgr_outputs});
+            call = CallNode{call.name,


do we over-wrap existing call operation with call_every_nth != 1 by a new operation with call_every_nth == 1 which has a counter and the same comp?
If my understanding is correct than in this case do we just return the previous Mat without actual calculation? if yes then why it is called as "skip frames"? I thought that by skip frames it means receiving something empty frames of increasing next frame awaiting duration

Agree, naming is confusing. This is about calling some nodes(kernels) on every n iterations. (not on every iteration)

I see,
Then question - Does it affect final measurement is some sort of way? maybe it had better to print information about node invocation count also in final report?

If node works not on every frame, it obviously improves performance.

maybe it had better to print information about node invocation count also in final report

Not sure that this is useful info, let's wait for the feedback firstly

…eline_modeling_tool-skip-frames-for-nodes

TolyaTalamanov · 2022-03-15T09:31:11Z

modules/gapi/samples/pipeline_modeling_tool/pipeline_builder.hpp

+                state.reset(new SubGraphState{});
+                state->cc = comp.compile(in, std::move(compile_args));
+
+                GAPI_Assert(state->cc.outMetas().size() == 1u);


There is no sense to double check it since it was checked in outMeta

TolyaTalamanov · 2022-03-16T09:21:15Z

modules/gapi/samples/pipeline_modeling_tool.cpp

                    auto output =
                        check_and_read<OutputDescr>(node_fn, "output", node_name);
-                    builder.addDummy(node_name, time, output);
+                    builder.addDummy(node_name, call_every_nth_u, time, output);


Do you mean like this:

builder.addDummy(NodeParams{node_name, call_every_nth_u}, time, output);

TolyaTalamanov · 2022-03-16T09:24:06Z

modules/gapi/samples/pipeline_modeling_tool/pipeline_builder.hpp

+            call.run(subgr_inputs, subgr_outputs);
+            auto comp = cv::GComputation(cv::GProtoInputArgs{subgr_inputs},
+                                         cv::GProtoOutputArgs{subgr_outputs});
+            call = CallNode{call.name,


Agree, naming is confusing. This is about calling some nodes(kernels) on every n iterations. (not on every iteration)

TolyaTalamanov · 2022-03-16T09:24:29Z

modules/gapi/samples/pipeline_modeling_tool/pipeline_builder.hpp

+            // G-API doesn't support dynamic number of inputs/outputs.
+            if (inputs.size() > 1u) {
+                throw std::logic_error(
+                        "skip_frame_nth is supported only for single input subgraphs");


Good point.

TolyaTalamanov · 2022-03-16T09:24:40Z

modules/gapi/samples/pipeline_modeling_tool.cpp

+                auto call_every_nth =
+                    call_every_nth_opt.has_value() ? call_every_nth_opt.value() : 1;
+                if (call_every_nth <= 0) {
+                    throw std::logic_error(node_name + " call_every_nth must be greater than zero");


sivanov-work

LGTM

dmatveev

Have a little idea what happens here but hope the app is ok.

Please provide a meaningful description in the future, it will be mandatory for all our MRs.

modules/gapi/samples/pipeline_modeling_tool.cpp

dmatveev · 2022-03-22T18:47:12Z

modules/gapi/samples/pipeline_modeling_tool/pipeline_builder.hpp

+                            SubGraphState&   state) {
+                // NB: Make a call on the first iteration and skip the furthers.
+                if (state.call_counter == 0) {
+                    state.cc(in, state.last_result);


So there may be an extra copy, right? Since this internal GComputation has its own writeBack() inside.

If fact, there is extra copy.
https://github.com/opencv/opencv/blob/4.x/modules/gapi/src/executor/gexecutor.cpp#L163

because adapter->data() == out_mat.data

Sorry, there is NO extract copy

Co-authored-by: Dmitry Matveev <dmitry.matveev@intel.com>

dmatveev

👍

TolyaTalamanov · 2022-03-25T07:44:21Z

@alalek Could you merge it, please?

…ing_tool-skip-frames-for-nodes [G-API] Pipeline modeling tool: Skip frames * Add skip feature * Refactoring * Fix warning * Put more comments * Fix comments to review * Agregate common params into structure * Fix warning * Clean up & add test * Add assert * Fix warning on Mac * Update modules/gapi/samples/pipeline_modeling_tool.cpp Co-authored-by: Dmitry Matveev <dmitry.matveev@intel.com>

Add skip feature

2811387

TolyaTalamanov added the category: g-api / gapi label Mar 15, 2022

TolyaTalamanov added this to the 4.6.0 milestone Mar 15, 2022

TolyaTalamanov added 3 commits March 15, 2022 11:03

Refactoring

7de23e3

Fix warning

988f1f8

Put more comments

cf921ad

sivanov-work reviewed Mar 15, 2022

View reviewed changes

Merge branch 'master' of https://github.com/opencv/opencv into at/pip…

9a7e2bd

…eline_modeling_tool-skip-frames-for-nodes

TolyaTalamanov commented Mar 16, 2022

View reviewed changes

TolyaTalamanov added 2 commits March 16, 2022 14:04

Fix comments to review

7ae217c

Agregate common params into structure

b511742

sivanov-work approved these changes Mar 16, 2022

View reviewed changes

TolyaTalamanov added 3 commits March 17, 2022 11:08

Fix warning

60d4288

Clean up & add test

f1f653c

Add assert

702cfb0

TolyaTalamanov force-pushed the at/pipeline_modeling_tool-skip-frames-for-nodes branch from 08d11b9 to 702cfb0 Compare March 17, 2022 09:13

Fix warning on Mac

2b0210b

dmatveev self-assigned this Mar 22, 2022

dmatveev approved these changes Mar 22, 2022

View reviewed changes

Update modules/gapi/samples/pipeline_modeling_tool.cpp

d43535e

Co-authored-by: Dmitry Matveev <dmitry.matveev@intel.com>

dmatveev approved these changes Mar 25, 2022

View reviewed changes

alalek merged commit 9dd8e4d into opencv:4.x Mar 25, 2022

opencv-pushbot mentioned this pull request Apr 23, 2022

(5.x) Merge 4.x #21903

Merged

Uh oh!

Conversation

TolyaTalamanov commented Mar 15, 2022

Pull Request Readiness Checklist

Build configuration

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!

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!

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!

sivanov-work left a comment

Choose a reason for hiding this comment

Uh oh!

dmatveev left a comment

Choose a reason for hiding this comment

Uh oh!

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!

dmatveev left a comment

Choose a reason for hiding this comment

Uh oh!

TolyaTalamanov commented Mar 25, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants