Added ITT traces to GStreamingExecutor by AsyaPronina · Pull Request #19917 · opencv/opencv

AsyaPronina · 2021-04-16T01:33:16Z

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

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

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=*

modules/gapi/src/backends/cpu/gcpubackend.cpp

modules/gapi/src/executor/gstreamingexecutor.cpp

dmatveev

Looks cool!

modules/gapi/CMakeLists.txt

modules/gapi/src/executor/gstreamingexecutor.cpp

modules/gapi/src/utils/gapi_itt.cpp

modules/gapi/src/utils/gapi_itt.hpp

dmatveev

I believe SyncQueue instrumentation can be dropped for now as it only bloats the trace and is nearly equal to StreamingInput::get and so on. Overall well done!

modules/gapi/CMakeLists.txt

dmatveev · 2021-04-21T20:18:44Z

modules/gapi/src/compiler/gislandmodel.cpp

+            backend_name = matches[1].str();
+            island_name = island_name + "_" + backend_name;
+        }
+    }


If match fails, I'd use the full backend type name instead of nothing.

Full backend name will contain some mangled symbhols and numbers which are maps to "::" namespace resolution operator, is it applicable?

Yes, please use the full name in that case.

Thanks, fixed!

modules/gapi/src/compiler/gislandmodel.cpp

modules/gapi/src/executor/gstreamingexecutor.cpp

AsyaPronina · 2021-04-26T10:06:25Z

Hi Anton(@anton-potapov)!
Could you please review changes in gtbbexecutor.cpp?
I had to do them because Linux OpenCL build wasn't green without.

dmatveev

I think TBB changes are ok and you can proceed with merge after fixing the non-regular island name handling.

dmatveev · 2021-04-28T11:55:11Z

modules/gapi/src/compiler/gislandmodel.cpp

+    if (std::regex_search(backend_impl_type_name, matches, rgx)) {
+        if (2ul == matches.size()) {
+            backend_name = matches[1].str();
+            island_name = island_name + "_" + backend_name;


This one is reworked!

dmatveev · 2021-04-28T11:55:31Z

modules/gapi/src/compiler/gislandmodel.cpp

+            backend_name = matches[1].str();
+            island_name = island_name + "_" + backend_name;
+        }
+    }


Yes, please use the full name in that case.

dmatveev · 2021-04-28T11:56:31Z

modules/gapi/src/executor/gstreamingexecutor.cpp


+        // Create just empty island meta information
+        std::string island_meta_info { };
+#if defined(OPENCV_WITH_ITT) && !defined(GAPI_STANDALONE)


btw whats wrong with STANDALONE here?

Fixed!
Thanks a lot!
I have realized that OPENCV_WITH_ITT will be defined only if ITT dependency will be successfully resolved, what can be done only if we building with OpenCV. So, there will be no such define in G-API standalone mode and check for !GAPI_STANDALONE is extra.
I have added a comment about this in itt.hpp file and TODO to support ITT traces in GAPI standalone mode in CMakeLists.txt.

anton-potapov · 2021-05-04T07:48:32Z

modules/gapi/src/compiler/gislandmodel.hpp

    // FIXME: GAPI_EXPORTS because of tests only!
    ade::NodeHandle GAPI_EXPORTS producerOf(const ConstGraph &g, ade::NodeHandle &data_nh);
-
+    // traceName - returns pretty island name for passed island node


I believe in should be traceIslandName

Thanks, Anton!!

AsyaPronina · 2021-05-04T13:34:06Z

Hi Maxim(@mshabunin), Alexander(@alalek)!
Assigning this PR to Ruslan(@rgarnov) as I didn't have time to finish it before DmitryM(@dmatveev)'s vacation.

mshabunin · 2021-05-05T13:31:10Z

modules/gapi/src/compiler/gislandmodel.cpp

+    std::string backend_name = "";
+    auto& backend_impl = island_ptr->backend().priv();
+    std::string backend_impl_type_name = typeid(backend_impl).name();
+    std::regex rgx("G([a-zA-Z0-9]+)BackendImpl");


std::regex is not working in GCC<4.9 (https://stackoverflow.com/questions/12530406/is-gcc-4-8-or-earlier-buggy-about-regular-expressions), but we still kinda support CentOS 7 which uses GCC 4.8.5. Is it possible to rewrite this part without regex usage?

AsyaPronina requested review from anton-potapov, dbudniko and dmatveev April 16, 2021 01:33

AsyaPronina commented Apr 16, 2021

View reviewed changes

modules/gapi/src/backends/cpu/gcpubackend.cpp Show resolved Hide resolved

AsyaPronina commented Apr 16, 2021

View reviewed changes

modules/gapi/src/executor/gstreamingexecutor.cpp Outdated Show resolved Hide resolved

AsyaPronina commented Apr 16, 2021

View reviewed changes

modules/gapi/src/executor/gstreamingexecutor.cpp Show resolved Hide resolved

AsyaPronina commented Apr 16, 2021

View reviewed changes

modules/gapi/src/executor/gstreamingexecutor.cpp Show resolved Hide resolved

AsyaPronina force-pushed the asyadev/itt_traces_in_gstreamingexecutor branch from ff8bb51 to 3757ce2 Compare April 16, 2021 01:54

AsyaPronina commented Apr 19, 2021

View reviewed changes

modules/gapi/src/executor/gstreamingexecutor.cpp Outdated Show resolved Hide resolved

dmatveev self-assigned this Apr 19, 2021

dmatveev added the category: g-api / gapi label Apr 19, 2021

dmatveev added this to the 4.5.3 milestone Apr 19, 2021

dmatveev reviewed Apr 19, 2021

View reviewed changes

AsyaPronina force-pushed the asyadev/itt_traces_in_gstreamingexecutor branch 7 times, most recently from 66a90b7 to 09a5fa9 Compare April 21, 2021 17:36

dmatveev reviewed Apr 21, 2021

View reviewed changes

AsyaPronina force-pushed the asyadev/itt_traces_in_gstreamingexecutor branch 9 times, most recently from 9ad402b to a2b654a Compare April 23, 2021 20:27

AsyaPronina force-pushed the asyadev/itt_traces_in_gstreamingexecutor branch 3 times, most recently from 7caf350 to d8ece4a Compare April 23, 2021 21:31

dmatveev reviewed Apr 28, 2021

View reviewed changes

anton-potapov reviewed May 4, 2021

View reviewed changes

AsyaPronina force-pushed the asyadev/itt_traces_in_gstreamingexecutor branch 3 times, most recently from a6faabb to 96e2e5a Compare May 4, 2021 13:17

AsyaPronina assigned rgarnov May 4, 2021

AsyaPronina force-pushed the asyadev/itt_traces_in_gstreamingexecutor branch from 96e2e5a to 12170ab Compare May 4, 2021 13:30

rgarnov approved these changes May 4, 2021

View reviewed changes

AsyaPronina force-pushed the asyadev/itt_traces_in_gstreamingexecutor branch from 12170ab to 6d630ab Compare May 4, 2021 20:33

mshabunin reviewed May 5, 2021

View reviewed changes

AsyaPronina force-pushed the asyadev/itt_traces_in_gstreamingexecutor branch 3 times, most recently from f82892a to 4568709 Compare May 7, 2021 18:20

mshabunin approved these changes May 7, 2021

View reviewed changes

Added ITT traces to GStreamingExecutor

3a49ff9

AsyaPronina force-pushed the asyadev/itt_traces_in_gstreamingexecutor branch from 4568709 to 3a49ff9 Compare May 11, 2021 09:55

alalek unassigned dmatveev May 11, 2021

alalek merged commit df05bc6 into opencv:master May 11, 2021

alalek mentioned this pull request Jun 4, 2021

(5.x) Merge 4.x #20216

Merged

dmatveev mentioned this pull request Sep 14, 2023

Fail to build 4.5.2 with cuda, TBB and mkl #19954

Closed

1 task

Uh oh!

Conversation

AsyaPronina commented Apr 16, 2021

Pull Request Readiness Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dmatveev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

AsyaPronina Apr 21, 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!

Uh oh!

Uh oh!

AsyaPronina commented Apr 26, 2021

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!

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!

AsyaPronina commented May 4, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

AsyaPronina Apr 21, 2021 •

edited

Loading