You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/04/18 18:01:30 UTC
[GitHub] [tvm] areusch opened a new pull request, #11041: Add vlogging for type-table registration
areusch opened a new pull request, #11041:
URL: https://github.com/apache/tvm/pull/11041
This PR makes it easy to determine the order in which `GetOrAllocRuntimeTypeIndex` is called and to make sure it is called. Adding as VLOG(3) since we mostly don't want to do this unless something is really screwed up.
@mbs-octoml @tqchen
--
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-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] mbs-octoml commented on a diff in pull request #11041: Add vlogging for type-table registration
Posted by GitBox <gi...@apache.org>.
mbs-octoml commented on code in PR #11041:
URL: https://github.com/apache/tvm/pull/11041#discussion_r863165716
##########
src/runtime/object.cc:
##########
@@ -103,6 +103,8 @@ class TypeContext {
if (static_tindex != TypeIndex::kDynamic) {
// statically assigned type
+ VLOG(3) << "TypeIndex[" << static_tindex << "]: static: " << skey << ", parent "
+ << type_table_[parent_tindex].name;
Review Comment:
The mechanism is identical to DLOG.
--
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-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] github-actions[bot] commented on pull request #11041: Add vlogging for type-table registration
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #11041:
URL: https://github.com/apache/tvm/pull/11041#issuecomment-1123763779
It has been a while since this PR was updated, @tqchen @junrushao1994 @mbs-octoml please leave a review or address the outstanding comments. @areusch if this PR is still a work in progress, please [convert it to a draft](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-stage-of-a-pull-request#converting-a-pull-request-to-a-draft) until it is ready for review.
--
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-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a diff in pull request #11041: Add vlogging for type-table registration
Posted by GitBox <gi...@apache.org>.
tqchen commented on code in PR #11041:
URL: https://github.com/apache/tvm/pull/11041#discussion_r853517210
##########
src/runtime/object.cc:
##########
@@ -103,6 +103,8 @@ class TypeContext {
if (static_tindex != TypeIndex::kDynamic) {
// statically assigned type
+ VLOG(3) << "TypeIndex[" << static_tindex << "]: static: " << skey << ", parent "
+ << type_table_[parent_tindex].name;
Review Comment:
DLOG is something that safely be removed under no debug mode, and may help certain cases to reduce binary size
--
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-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] areusch merged pull request #11041: Add vlogging for type-table registration
Posted by GitBox <gi...@apache.org>.
areusch merged PR #11041:
URL: https://github.com/apache/tvm/pull/11041
--
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-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a diff in pull request #11041: Add vlogging for type-table registration
Posted by GitBox <gi...@apache.org>.
tqchen commented on code in PR #11041:
URL: https://github.com/apache/tvm/pull/11041#discussion_r864816936
##########
src/runtime/object.cc:
##########
@@ -103,6 +103,8 @@ class TypeContext {
if (static_tindex != TypeIndex::kDynamic) {
// statically assigned type
+ VLOG(3) << "TypeIndex[" << static_tindex << "]: static: " << skey << ", parent "
+ << type_table_[parent_tindex].name;
Review Comment:
OK, thanks for clarifying. no more comments on my end
--
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-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] github-actions[bot] commented on pull request #11041: Add vlogging for type-table registration
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #11041:
URL: https://github.com/apache/tvm/pull/11041#issuecomment-1112495050
It has been a while since this PR was updated, @tqchen @junrushao1994 please leave a review or address the outstanding comments. @areusch if this PR is still a work in progress, please [convert it to a draft](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-stage-of-a-pull-request#converting-a-pull-request-to-a-draft) until it is ready for review.
--
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-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] areusch commented on a diff in pull request #11041: Add vlogging for type-table registration
Posted by GitBox <gi...@apache.org>.
areusch commented on code in PR #11041:
URL: https://github.com/apache/tvm/pull/11041#discussion_r863159527
##########
src/runtime/object.cc:
##########
@@ -103,6 +103,8 @@ class TypeContext {
if (static_tindex != TypeIndex::kDynamic) {
// statically assigned type
+ VLOG(3) << "TypeIndex[" << static_tindex << "]: static: " << skey << ", parent "
+ << type_table_[parent_tindex].name;
Review Comment:
@mbs-octoml said that while VLOG isn't explicitly compiled out, it's done in such a way that the compiler determines it's dead-code. i think this is better in case a variable is only used in VLOG.
--
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-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] areusch commented on a diff in pull request #11041: Add vlogging for type-table registration
Posted by GitBox <gi...@apache.org>.
areusch commented on code in PR #11041:
URL: https://github.com/apache/tvm/pull/11041#discussion_r854635836
##########
src/runtime/object.cc:
##########
@@ -103,6 +103,8 @@ class TypeContext {
if (static_tindex != TypeIndex::kDynamic) {
// statically assigned type
+ VLOG(3) << "TypeIndex[" << static_tindex << "]: static: " << skey << ", parent "
+ << type_table_[parent_tindex].name;
Review Comment:
if we had `DLOG(3)` i'd agree with you. what do you think about adding that?
--
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-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a diff in pull request #11041: Add vlogging for type-table registration
Posted by GitBox <gi...@apache.org>.
tqchen commented on code in PR #11041:
URL: https://github.com/apache/tvm/pull/11041#discussion_r854670461
##########
src/runtime/object.cc:
##########
@@ -103,6 +103,8 @@ class TypeContext {
if (static_tindex != TypeIndex::kDynamic) {
// statically assigned type
+ VLOG(3) << "TypeIndex[" << static_tindex << "]: static: " << skey << ", parent "
+ << type_table_[parent_tindex].name;
Review Comment:
DLOG was not enabled by default and do not come with the level(exception for DLOG(INFO) and DLOG(WARNING)), which should be OK for this case. if you think adding level is helpful, i have no objections :)
--
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-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a diff in pull request #11041: Add vlogging for type-table registration
Posted by GitBox <gi...@apache.org>.
tqchen commented on code in PR #11041:
URL: https://github.com/apache/tvm/pull/11041#discussion_r855432109
##########
src/runtime/object.cc:
##########
@@ -103,6 +103,8 @@ class TypeContext {
if (static_tindex != TypeIndex::kDynamic) {
// statically assigned type
+ VLOG(3) << "TypeIndex[" << static_tindex << "]: static: " << skey << ", parent "
+ << type_table_[parent_tindex].name;
Review Comment:
please double check, if that is the case we could be fine
--
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-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] areusch commented on a diff in pull request #11041: Add vlogging for type-table registration
Posted by GitBox <gi...@apache.org>.
areusch commented on code in PR #11041:
URL: https://github.com/apache/tvm/pull/11041#discussion_r855390237
##########
src/runtime/object.cc:
##########
@@ -103,6 +103,8 @@ class TypeContext {
if (static_tindex != TypeIndex::kDynamic) {
// statically assigned type
+ VLOG(3) << "TypeIndex[" << static_tindex << "]: static: " << skey << ", parent "
+ << type_table_[parent_tindex].name;
Review Comment:
actually on second thought--VLOG is debugging info, why is it not removed when `TVM_LOG_DEBUG` is unset? i don't see an INFO/WARNING setting that makes sense for this type of debug info.
--
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-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a diff in pull request #11041: Add vlogging for type-table registration
Posted by GitBox <gi...@apache.org>.
tqchen commented on code in PR #11041:
URL: https://github.com/apache/tvm/pull/11041#discussion_r853013821
##########
src/runtime/object.cc:
##########
@@ -103,6 +103,8 @@ class TypeContext {
if (static_tindex != TypeIndex::kDynamic) {
// statically assigned type
+ VLOG(3) << "TypeIndex[" << static_tindex << "]: static: " << skey << ", parent "
+ << type_table_[parent_tindex].name;
Review Comment:
Perhaps DLOG is better? not so sure about the level 3 here
--
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-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] areusch commented on a diff in pull request #11041: Add vlogging for type-table registration
Posted by GitBox <gi...@apache.org>.
areusch commented on code in PR #11041:
URL: https://github.com/apache/tvm/pull/11041#discussion_r853498771
##########
src/runtime/object.cc:
##########
@@ -103,6 +103,8 @@ class TypeContext {
if (static_tindex != TypeIndex::kDynamic) {
// statically assigned type
+ VLOG(3) << "TypeIndex[" << static_tindex << "]: static: " << skey << ", parent "
+ << type_table_[parent_tindex].name;
Review Comment:
why not level 3?
--
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-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a diff in pull request #11041: Add vlogging for type-table registration
Posted by GitBox <gi...@apache.org>.
tqchen commented on code in PR #11041:
URL: https://github.com/apache/tvm/pull/11041#discussion_r854670461
##########
src/runtime/object.cc:
##########
@@ -103,6 +103,8 @@ class TypeContext {
if (static_tindex != TypeIndex::kDynamic) {
// statically assigned type
+ VLOG(3) << "TypeIndex[" << static_tindex << "]: static: " << skey << ", parent "
+ << type_table_[parent_tindex].name;
Review Comment:
DLOG was not enabled by default and do not come with the level, how if you think adding level is helpful, i have no objections :)
--
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-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org