Skip to content

Logging revamp for 4.1.0 (#11003, OE-11)#13642

Closed
kinchungwong wants to merge 2 commits intoopencv:masterfrom
kinchungwong:issue_11003_logging_revamp_cv410
Closed

Logging revamp for 4.1.0 (#11003, OE-11)#13642
kinchungwong wants to merge 2 commits intoopencv:masterfrom
kinchungwong:issue_11003_logging_revamp_cv410

Conversation

@kinchungwong
Copy link
Copy Markdown
Contributor

@kinchungwong kinchungwong commented Jan 17, 2019

does_not_resolve_yet #11003

This pullrequest changes

Added class LogMeta and others.
Added setLogFilter, getLogFilter.
Added CV_LOGMETA.
Added CV_LOG_LOC, CV_LOG_THIS.
Modified CV_LOG_FATAL, CV_LOG_ERROR and others to use CV_LOGMETA.
Added defaultLogFilter as a default implementation.

Known issues

Building with CV_LOG_STRIP_LEVEL set to (CV_LOG_LEVEL_VERBOSE + 1) will cause compile errors due to syntax errors in the following files:

  • modules/core/src/ocl.cpp
  • modules/dnn/src/ocl4dnn/src/ocl4dnn_conv_spatial.cpp

This is due to incorrect usage of the CV_LOG_VERBOSE macro, or trying to access non-existent C++ class members.
This commit does not fix these known issues; a separate PR will be submitted independently of Issue 11003.

Progress

Checklist for core module changes

  • LogMeta class, and classes for pieces of metadata, such as LogLoc, LogObjInfo, LogModuleInfo
  • Global filter function that accept LogMeta class, and default implementation
  • CV_LOGMETA macro
  • Updated existing CV_LOG_(INFO, WARNING, etc) macros to use CV_LOGMETA
  • Change backend function prototype (cv::utils::logging::internal::writeLogMessage)
    to accept LogMeta (or its data members) so that backends that can benefit from those
    data can use them programmatically without parsing from strings.
  • Implement backend switchability
  • Implement backend multicast
  • Implement runtime configurable module-specific log level threshold
    • Add static functions to get and set threshold for specific modules
    • Update the default filter implementation to use that module-specific threshold
  • Capture parallelized work thread-relationship information in cv::ParallelLoopBody executor implementations
  • Implement string formatter switchability (for example fmtlib)
  • Implement large matrix logging (frontend and backend work)
  • Implement binary blob logging (frontend and backend work)

Checklist for required changes in every OpenCV module

  • Define a macro for module name string that is visible to all code inside that module, without leaking outside
  • Override (redefine) existing CV_LOG_(...) macros in each module to pass module name into LogMeta

Checklist for long-term improvement changes in every OpenCV module

  • For modules that could use logging but don't have it, add some, strategically.
  • For modules that use logging badly, fix those issues
  • Where applicable, include LogObjInfo (CV_LOG_THIS) into LogMeta to help identify object instances
  • Replace kludge, such as in stitching module

@kinchungwong kinchungwong changed the title Issue 11003 OE-11 Logging revamp for 4.1.0 initial commit. Logging revamp for 4.1.0 (#11003, OE-11) Jan 17, 2019
@kinchungwong
Copy link
Copy Markdown
Contributor Author

To prevent forced changes to all existing logging in application code, the new design must make sure existing log macros continue to work, even if their definitions are changed.

The existing log macros refer to:

  • CV_LOG_FATAL
  • CV_LOG_ERROR
  • CV_LOG_WARNING
  • CV_LOG_INFO
  • CV_LOG_DEBUG
  • CV_LOG_VERBOSE

However, that means new features cannot be tested on these macros. To test the new features, currently one must use the new macro, CV_LOGMETA.

Here are some examples of using CV_LOGMETA. Line breaks and indentation are added for readability.

Example 1

CV_LOGMETA(
    NULL, 
    cv::utils::logging::LOG_LEVEL_DEBUG, 
    cv::utils::logging::LogModuleInfo("imgcodecs")
    << CV_LOG_LOC(),
    "findDecoder(" << filename << ")"
);

Example 2

CV_LOGMETA(
    NULL, 
    cv::utils::logging::LOG_LEVEL_DEBUG, 
    cv::utils::logging::LogModuleInfo("imgcodecs") 
    << CV_LOG_LOC() 
    << CV_LOG_THIS(), 
    "Reading PNG header");

In these examples,

  • CV_LOG_LOC captures the source file name, line number, and function name
  • CV_LOG_THIS captures the type information and address of the "this" pointer, but only if inside a class non-static member function.
  • LogModuleInfo captures the module name. Later, a macro will be added to each module to reduce clutter.

@kinchungwong
Copy link
Copy Markdown
Contributor Author

This document provides some of the underlying thinking about why certain design decisions are made. https://gist.github.com/kinchungwong/6d24bd3d45424197a4d1495237cd9be3

Added class LogMeta and others.
Added setLogFilter, getLogFilter.
Added CV_LOGMETA.
Added CV_LOG_LOC, CV_LOG_THIS.
Modified CV_LOG_FATAL, CV_LOG_ERROR and others to use CV_LOGMETA.
Added defaultLogFilter as a default implementation.

Known issues:

Building with CV_LOG_STRIP_LEVEL set to (CV_LOG_LEVEL_VERBOSE + 1) will cause compile errors due to syntax errors in the following files:
- modules/core/src/ocl.cpp
- modules/dnn/src/ocl4dnn/src/ocl4dnn_conv_spatial.cpp
This is due to incorrect usage of the CV_LOG_VERBOSE macro, or trying to access non-existent C++ class members.
This commit does not fix these known issues; a separate PR will be submitted independently of Issue 11003.
Spec:
#11003 (comment)

Summary:
added class LogScope;
added enum class LogConfigLayer;
added class LogThreshold;
added class LogManager;
added hash helper functions in namespace cv::utils::logging::hash_utility;

Details:

A prototype that demonstrates configuring log level filtering threshold
at runtime using a hierarchy (nesting) of LogScope. Typically LogScope
are instantiated as static variables at namespace, class, or function
local scope level. A nested LogScope is declared by providing a ref to
the outer LogScope when calling the constructor.

Each LogScope has a name. A LogScope has a fully-qualified name formed
from concatenating the names of its parents with its own, joined by the
period char (decimal 46, hex 2E). A FNV-1a 64-bit hash is computed from
this fully-qualified name, and used as the lookup key in LogManager.
(LogManager assumes there is no collision for this 64-bit hash value;
string comparison is not performed due to speed reasons.)

Log filtering thresholds for any scope can be changed at any time.

Requires C++11:
unordered_map, atomic, memory

Remarks:
The source code is now in a hybrid state. The older LogMeta class is
still in the commit; LogModuleInfo is replaced by LogScope. However,
the macros have not been updated yet.
@kinchungwong
Copy link
Copy Markdown
Contributor Author

@alalek I probably need help writing a preprocessor check for GCC versions and C++ language versions (inside preprocessor code) in which std::make_unique is missing. std::make_unique is a C++14 thing, but it is available in some other compilers. I need to provide a replacement but only if there isn't one provided from the combination of compiler settings.

@kinchungwong
Copy link
Copy Markdown
Contributor Author

This PR is too far off from current design, not possible to contribute (merge) and/or improve. Closing PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants