huajsj commented on a change in pull request #9012:
URL: https://github.com/apache/tvm/pull/9012#discussion_r711431026
##########
File path: src/runtime/logging.cc
##########
@@ -166,10 +167,118 @@ namespace tvm {
namespace runtime {
namespace detail {
+namespace {
+constexpr const char* kSrcPrefix = "/src/";
+constexpr const size_t kSrcPrefixLength = 5;
Review comment:
```
constexpr size_t kSrcPrefixLength = std::char_traits<const
char>::length(kSrcPrefix);
```
##########
File path: src/runtime/logging.cc
##########
@@ -166,10 +167,118 @@ namespace tvm {
namespace runtime {
namespace detail {
+namespace {
+constexpr const char* kSrcPrefix = "/src/";
+constexpr const size_t kSrcPrefixLength = 5;
+constexpr const char* kDefaultKeyword = "DEFAULT";
+} // namespace
+
+// Parse \p opt_spec as a VLOG specification as per comment in
+// DebugLoggingEnabled and VerboseLoggingEnabled.
+std::unordered_map<std::string, int> ParseTvmLogDebugSpec(const char*
opt_spec) {
+ std::unordered_map<std::string, int> map;
+ if (opt_spec == nullptr) {
+ // DLOG and VLOG disabled.
+ return map;
+ }
+ std::string spec(opt_spec);
+ if (spec.empty() || spec == "0") {
+ // DLOG and VLOG disabled.
+ return map;
+ }
+ if (spec == "1") {
+ // Legacy specification for enabling just DLOG.
+ // A wildcard entry in the map will signal DLOG is on, but all VLOG levels
are disabled.
+ LOG(INFO) << "TVM_LOG_DEBUG enables DLOG statements only";
+ map.emplace(kDefaultKeyword, -1);
+ return map;
+ }
+ std::istringstream spec_stream(spec);
+ while (spec_stream) {
+ std::string name;
+ if (!std::getline(spec_stream, name, '=')) {
+ // Reached end.
+ break;
+ }
+ if (name.empty()) {
+ LOG(FATAL) << "TVM_LOG_DEBUG ill-formed, empty name";
+ return map;
+ }
+
+ std::string level;
+ if (!std::getline(spec_stream, level, ';')) {
+ LOG(FATAL) << "TVM_LOG_DEBUG ill-formed, expecting level";
+ return map;
+ }
+ if (level.empty()) {
+ LOG(FATAL) << "TVM_LOG_DEBUG ill-formed, empty level";
+ return map;
+ }
+ // Parse level, default to 0 if ill-formed which we don't detect.
+ char* end_of_level = nullptr;
+ int level_val = static_cast<int>(strtol(level.c_str(), &end_of_level, 10));
+ if (end_of_level != level.c_str() + level.size()) {
+ LOG(FATAL) << "TVM_LOG_DEBUG ill-formed, invalid level";
+ return map;
+ }
Review comment:
recommend to use c++ here to make code style consistent and more
readable.
```
int level_val = std::stoi(level);
if (to_string(level_val) != level) {
LOG(FATAL) << "TVM_LOG_DEBUG ill-formed, invalid level";
return map;
}
```
##########
File path: src/runtime/logging.cc
##########
@@ -166,10 +167,118 @@ namespace tvm {
namespace runtime {
namespace detail {
+namespace {
+constexpr const char* kSrcPrefix = "/src/";
+constexpr const size_t kSrcPrefixLength = 5;
+constexpr const char* kDefaultKeyword = "DEFAULT";
+} // namespace
+
+// Parse \p opt_spec as a VLOG specification as per comment in
+// DebugLoggingEnabled and VerboseLoggingEnabled.
+std::unordered_map<std::string, int> ParseTvmLogDebugSpec(const char*
opt_spec) {
+ std::unordered_map<std::string, int> map;
+ if (opt_spec == nullptr) {
+ // DLOG and VLOG disabled.
+ return map;
+ }
+ std::string spec(opt_spec);
+ if (spec.empty() || spec == "0") {
+ // DLOG and VLOG disabled.
+ return map;
+ }
+ if (spec == "1") {
+ // Legacy specification for enabling just DLOG.
+ // A wildcard entry in the map will signal DLOG is on, but all VLOG levels
are disabled.
+ LOG(INFO) << "TVM_LOG_DEBUG enables DLOG statements only";
+ map.emplace(kDefaultKeyword, -1);
Review comment:
this logic seems like not backward compatible in performance. when spec
== "1", VerboseEnabledInMap still need to do string parse and map search, that
seems like not necessary, is there any specific concern here not just leave map
be empty and return false in VerboseEnabledInMap when map.empty() is true?
##########
File path: src/runtime/logging.cc
##########
@@ -166,10 +167,118 @@ namespace tvm {
namespace runtime {
namespace detail {
+namespace {
+constexpr const char* kSrcPrefix = "/src/";
+constexpr const size_t kSrcPrefixLength = 5;
+constexpr const char* kDefaultKeyword = "DEFAULT";
+} // namespace
+
+// Parse \p opt_spec as a VLOG specification as per comment in
+// DebugLoggingEnabled and VerboseLoggingEnabled.
+std::unordered_map<std::string, int> ParseTvmLogDebugSpec(const char*
opt_spec) {
+ std::unordered_map<std::string, int> map;
+ if (opt_spec == nullptr) {
+ // DLOG and VLOG disabled.
+ return map;
+ }
+ std::string spec(opt_spec);
+ if (spec.empty() || spec == "0") {
+ // DLOG and VLOG disabled.
+ return map;
+ }
+ if (spec == "1") {
+ // Legacy specification for enabling just DLOG.
+ // A wildcard entry in the map will signal DLOG is on, but all VLOG levels
are disabled.
+ LOG(INFO) << "TVM_LOG_DEBUG enables DLOG statements only";
+ map.emplace(kDefaultKeyword, -1);
+ return map;
+ }
+ std::istringstream spec_stream(spec);
+ while (spec_stream) {
+ std::string name;
+ if (!std::getline(spec_stream, name, '=')) {
+ // Reached end.
+ break;
+ }
+ if (name.empty()) {
+ LOG(FATAL) << "TVM_LOG_DEBUG ill-formed, empty name";
+ return map;
+ }
+
+ std::string level;
+ if (!std::getline(spec_stream, level, ';')) {
+ LOG(FATAL) << "TVM_LOG_DEBUG ill-formed, expecting level";
+ return map;
+ }
+ if (level.empty()) {
+ LOG(FATAL) << "TVM_LOG_DEBUG ill-formed, empty level";
+ return map;
+ }
+ // Parse level, default to 0 if ill-formed which we don't detect.
+ char* end_of_level = nullptr;
+ int level_val = static_cast<int>(strtol(level.c_str(), &end_of_level, 10));
+ if (end_of_level != level.c_str() + level.size()) {
+ LOG(FATAL) << "TVM_LOG_DEBUG ill-formed, invalid level";
+ return map;
+ }
+ if (map.empty()) {
+ LOG(INFO) << "TVM_LOG_DEBUG enables DLOG statements";
+ }
+ LOG(INFO) << "TVM_LOG_DEBUG enables VLOG statements in '" << name << "' up
to level " << level;
+ map.emplace(name, level_val);
+ }
+ return map;
+}
+
+const std::unordered_map<std::string, int>& TvmLogDebugSpec() {
+ // Parse and cache the verbosity level map.
+ static const auto* map =
+ new std::unordered_map<std::string,
int>(ParseTvmLogDebugSpec(std::getenv("TVM_LOG_DEBUG")));
+ return *map;
+}
+
+bool VerboseEnabledInMap(const std::string& filename, int level,
+ const std::unordered_map<std::string, int>& map) {
+ if (level < 0) {
Review comment:
seems like correct level start from 0 and negative level will no need
check, could you add some comments to explain this logic?
##########
File path: src/runtime/logging.cc
##########
@@ -166,10 +167,115 @@ namespace tvm {
namespace runtime {
namespace detail {
+// Parse \p opt_spec as a VLOG specification as per comment in
+// DebugLoggingEnabled and VerboseLoggingEnabled.
+std::unordered_map<std::string, int> ParseTvmLogDebugSpec(const char*
opt_spec) {
+ std::unordered_map<std::string, int> map;
+ if (opt_spec == nullptr) {
+ // DLOG and VLOG disabled.
+ return map;
+ }
+ std::string spec(opt_spec);
+ if (spec.empty() || spec == "0") {
+ // DLOG and VLOG disabled.
+ return map;
+ }
Review comment:
std::string spec(opt_spec);
if (opt_spec == nullptr || spec.emptry() || spec == "0"){
// DLOG and VLOG disabled.
return map;
}
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
