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
Code tracing support #8766
Code tracing support #8766
Conversation
|
||
//! Supported logging levels and their semantic | ||
enum LogLevel { | ||
LOG_LEVEL_SILENT = 0, //!< for using in setLogVevel() call |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it will be better to use CamelCase naming for enum items to avoid possible conflicts with user-defined macros.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This header should not be included automatically with any of public OpenCV header file (like opencv.hpp
), so serious problems are not expected.
BTW, CamelCase for "enumeration constants" violates Naming conventions guidelines.
|
||
//! Specify region flags | ||
enum RegionLocationFlag { | ||
REGION_FLAG_FUNCTION = (1 << 0), //< region is function (=1) / nested named region (=0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment about enum item naming.
//! Macro to trace argument value (expanded version) | ||
#define CV_TRACE_ARG_VALUE(arg_id, arg_name, value) | ||
|
||
#ifndef CV_DOXYGEN |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it better to use //! @cond IGNORED
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
@@ -21,6 +21,10 @@ if(HAVE_CUDA) | |||
ocv_warnings_disable(CMAKE_CXX_FLAGS -Wundef -Wenum-compare -Wunused-function -Wshadow) | |||
endif() | |||
|
|||
if(CV_TRACE AND HAVE_ITT AND BUILD_ITT) | |||
add_definitions(-DOPENCV_WITH_ITT=1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it better to add this definition to cvconfig.h?
/cc @slyalin |
Looks good to me! 👍 I have one question: are we going to add HAL-implementation and/or CPU dispatched blocks measurements? |
env = {} | ||
if not self.options.valgrind and self.options.trace: | ||
env['OPENCV_TRACE'] = '1' | ||
env['OPENCV_TRACE_LOCATION'] = 'OpenCVTrace-{}'.format(self.getLogBaseName(exe)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please help me to locate where this env var is really used in the tracing code? Is it in a new code (cannot find it easily)? And when we set this var, do we automatically switch to a text backend instead of ITT backend. I was just trying to follow https://github.com/opencv/opencv/wiki/Profiling-OpenCV-Applications, and searching for this part "An alternative to the ITT-based instrumentation, is to trace the OpenCV calls in the compact machine-readable text form". Could you please point me to specific file/line when I can start reviewing this specific part?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is located in modules/core/src/trace.cpp
file (GitHub doesn't load its content due "Large diffs are not rendered by default") - you need to click "Load diffs" button.
There is link to parameter in .cpp file
static int param_maxRegionDepthOpenCV = (int)utils::getConfigurationParameterSizeT("OPENCV_TRACE_DEPTH_OPENCV", 1); | ||
static int param_maxRegionChildrenOpenCV = (int)utils::getConfigurationParameterSizeT("OPENCV_TRACE_MAX_CHILDREN_OPENCV", 1000); | ||
static int param_maxRegionChildren = (int)utils::getConfigurationParameterSizeT("OPENCV_TRACE_MAX_CHILDREN", 10000); | ||
static cv::String param_traceLocation = utils::getConfigurationParameterString("OPENCV_TRACE_LOCATION", "OpenCVTrace"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@slyalin OPENCV_TRACE_LOCATION
is used here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it correct observation that there is no "loopinizer" script here?
It's correct. There is no script with "loop folding". |
CV_INSTRUMENT_FUN_IPP
macro is not covered - requires C++11 support or huge refactoring). Done: added processing ofCV_RUN_IPP*
callsclFinish
)run.py
scriptparallel_for
calls processing (thread count amplification)trace_test
branch)CV_TRACE=OFF
Examples:
chrome://tracing
(open URL in new tab): trace file from IntelSEAPI toolBinary size increase with enabled CV_TRACE (set to default):