huajsj commented on a change in pull request #9012:
URL: https://github.com/apache/tvm/pull/9012#discussion_r709444137
##########
File path: include/tvm/runtime/logging.h
##########
@@ -395,10 +399,13 @@ class LogMessageVoidify {
inline bool DebugLoggingEnabled() {
static int state = 0;
if (state == 0) {
- if (auto var = std::getenv("TVM_LOG_DEBUG")) {
- if (std::string(var) == "1") {
+ if (const char* var = std::getenv("TVM_LOG_DEBUG")) {
Review comment:
seems like change not necessary, should keep no change "auto var =
std::getenv("TVM_LOG_DEBUG")"
##########
File path: include/tvm/runtime/logging.h
##########
@@ -129,8 +132,9 @@
* a = ...
* b = ...
* // if quit_on_assertion is true, if a==b, continue, otherwise quit.
- * // if quit_on_assertion is false, if a==b, continue, otherwise 'return
false' (default
- * behaviour) COND_CHECK_EQ(quit_on_assertion, a, b) << "some error message
when quiting"
+ * // if quit_on_assertion is false, if a==b, continue, otherwise 'return
false'
+ * // (default behaviour)
+ * COND_CHECK_EQ(quit_on_assertion, a, b) << "some error message when
quiting"
Review comment:
136 and 137 should be same line.
##########
File path: src/runtime/logging.cc
##########
@@ -166,10 +167,127 @@ namespace tvm {
namespace runtime {
namespace detail {
+std::unordered_map<std::string, int> ParseTvmLogDebugSpec(const char*
opt_spec) {
+ // Cache the verbosity level map.
+ std::unordered_map<std::string, int> map;
+ LOG(INFO) << "initializing VLOG map";
+ if (opt_spec == nullptr) {
+ LOG(INFO) << "VLOG disabled, no TVM_LOG_DEBUG environment variable";
+ return map;
+ }
+ std::string spec(opt_spec);
+ // Check we are enabled overall with at least one VLOG option.
+ if (spec.rfind("1;", 0) != 0) {
+ LOG(INFO) << "VLOG disabled, TVM_LOG_DEBUG does not start with '1;'";
+ return map;
+ }
+ size_t start = 2UL;
+ while (start < spec.size()) {
+ // We are looking for "name=level;" or "*=level;"
+ size_t end = start;
+ // Scan up to '='.
+ while (spec[end] != '=') {
+ ++end;
+ if (end >= spec.size()) {
+ LOG(FATAL) << "TVM_LOG_DEBUG ill-formed, missing '='";
+ return map;
+ }
+ }
+ if (end == start) {
+ LOG(FATAL) << "TVM_LOG_DEBUG ill-formed, empty name";
+ return map;
+ }
+ std::string name(spec.substr(start, end - start));
+ // Skip '='
+ ++end;
+ if (end >= spec.size()) {
+ LOG(FATAL) << "TVM_LOG_DEBUG ill-formed, missing level";
+ return map;
+ }
+ // Scan up to ';'.
+ start = end;
+ while (spec[end] != ';') {
+ ++end;
+ if (end >= spec.size()) {
+ LOG(FATAL) << "TVM_LOG_DEBUG ill-formed, missing ';'";
+ return map;
+ }
+ }
+ if (end == start) {
+ LOG(FATAL) << "TVM_LOG_DEBUG ill-formed, empty level";
+ return map;
+ }
+ std::string level_str(spec.substr(start, end - start));
+ // Skip ';'.
+ ++end;
+ // Parse level, default to 0 if ill-formed which we don't detect.
+ char* end_of_level = nullptr;
+ int level = static_cast<int>(strtol(level_str.c_str(), &end_of_level, 10));
+ if (end_of_level != level_str.c_str() + level_str.size()) {
+ LOG(FATAL) << "TVM_LOG_DEBUG ill-formed, invalid level";
+ }
+ LOG(INFO) << "adding VLOG entry for '" << name << "' at level " << level;
+ map.emplace(name, level);
+ start = end;
+ }
+ return map;
+}
+
+constexpr const char* kSrcPrefix = "/src/";
+constexpr const size_t kSrcPrefixLength = 5;
+
+bool VerboseEnabledInMap(const std::string& filename, int level,
+ const std::unordered_map<std::string, int>& map) {
+ if (level < 0) {
+ return false;
+ }
+ // Canonicalize filename.
+ // TODO(mbs): Not Windows friendly.
+
+ size_t last_src = filename.rfind(kSrcPrefix, std::string::npos,
kSrcPrefixLength);
+ // Strip anything before the /src/ prefix, on the assumption that will yield
the
+ // TVM project relative filename. If no such prefix fallback to filename
without
+ // canonicalization.
+ std::string key =
+ last_src == std::string::npos ? filename : filename.substr(last_src +
kSrcPrefixLength);
+ // Check for exact.
+ auto itr = map.find(key);
+ if (itr != map.end()) {
+ return level <= itr->second;
+ }
+ // Check for '*' wildcard.
+ itr = map.find("*");
+ if (itr != map.end()) {
+ return level <= itr->second;
+ }
+ return false;
+}
+
+bool VerboseLoggingEnabled(const char* filename, int level) {
+ // Cache the verbosity level map.
+ static const std::unordered_map<std::string, int>* map =
+ new std::unordered_map<std::string,
int>(ParseTvmLogDebugSpec(std::getenv("TVM_LOG_DEBUG")));
Review comment:
recommend to use smart pointer.
##########
File path: include/tvm/runtime/logging.h
##########
@@ -395,10 +399,13 @@ class LogMessageVoidify {
inline bool DebugLoggingEnabled() {
static int state = 0;
if (state == 0) {
- if (auto var = std::getenv("TVM_LOG_DEBUG")) {
- if (std::string(var) == "1") {
+ if (const char* var = std::getenv("TVM_LOG_DEBUG")) {
+ std::string var_str(var);
+ if (var_str == "1" || var_str.rfind("1;", 0) == 0) {
Review comment:
seems like there are multiple type information get put into
TVM_LOG_DEBUG, this would make the use of such environment variable become
trivial and complicated, please avoid such usage.
##########
File path: CMakeLists.txt
##########
@@ -575,7 +575,7 @@ endif()
# Create the `cpptest` target if we can find GTest. If not, we create dummy
# targets that give the user an informative error message.
if(GTEST_INCLUDE_DIR AND GTEST_LIB)
- file(GLOB TEST_SRCS tests/cpp/*.cc)
+ file(GLOB_RECURSE TEST_SRCS tests/cpp/*.cc)
Review comment:
why changed from GLOB into GLOB_RECURSE?
##########
File path: include/tvm/runtime/logging.h
##########
@@ -409,6 +416,68 @@ inline bool DebugLoggingEnabled() {
return state == 1;
}
+/*! \brief Helpers for \p VerboseLoggingEnabled. Exposed for unit testing
only. */
+std::unordered_map<std::string, int> ParseTvmLogDebugSpec(const char*
opt_spec);
+bool VerboseEnabledInMap(const std::string& filename, int level,
+ const std::unordered_map<std::string, int>& map);
+
+/*!
+ * \brief Returns true if a VLOG statement in \p filename is enabled by the \p
TVM_LOG_DEBUG
+ * environment variable for logging at verbosity \p level.
+ *
+ * Filenames are canonicalized to be w.r.t. the src/ dir of the TVM tree.
(VLOG's should not
+ * appear under include/).
+ *
+ * To enable file \p relay/foo.cc up to level 2 and \p ir/bar.cc for level 0
only set:
+ * \code
+ * TVM_LOG_DEBUG="1;relay/foo.cc=2;ir/bar.cc=0;"
+ * \endcode
+ *
+ * To enable all files up to level 3 but disable \p ir/bar.cc set:
+ * \code
+ * TVM_LOG_DEBUG="1;*=2;ir/bar.cc=-1;"
Review comment:
This usage looks like not user friendly, is there any other better way
to do this? for example
TVM_LOG_DEBUG_LEVEL_2="relay/foo.cc;"
TVM_LOG_DEBUG_LEVEL_0="ir/bar.cc;"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@tvm.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
