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 2021/09/13 17:16:42 UTC
[GitHub] [tvm] petersalas opened a new pull request #8995: [Autoscheduler] Reduce task weight coercion overhead
petersalas opened a new pull request #8995:
URL: https://github.com/apache/tvm/pull/8995
In autoscheduler, a meaningful amount of time is spent collecting (discarded) backtraces in the `TaskScheduler` `objective_func`.
Remove `LOG(FATAL)` usage from `ReflectionVTable::GetAttr` to avoid backtrace collection during coercion, and modify `extract_tasks` to return `List[int]` instead of `List[IntImm]` to remove coercion altogether from the `objective_func`.
--
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] comaniac commented on a change in pull request #8995: [Autoscheduler] Reduce task weight coercion overhead
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #8995:
URL: https://github.com/apache/tvm/pull/8995#discussion_r707551632
##########
File path: src/node/reflection.cc
##########
@@ -106,8 +106,12 @@ runtime::TVMRetValue ReflectionVTable::GetAttr(Object* self, const String& field
}
}
if (!success) {
- LOG(FATAL) << "AttributeError: " << self->GetTypeKey() << " object has no attributed "
- << getter.skey;
Review comment:
I agree that LOG(FATAL) would be more debug-friendly, but I'm not sure if it is practical to fix where ever is calling GetAttr in an inefficient way, as GetAttr is used basically every where. IMHO, it doesn't hurt to make an exception use (i.e., `throw`) for this special case. Meanwhile, if we want to keep LOG(FATAL) for debugging purpose, we may consider using a macro or DLOG(FATAL).
--
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] junrushao1994 commented on pull request #8995: [Autoscheduler] Reduce task weight coercion overhead
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #8995:
URL: https://github.com/apache/tvm/pull/8995#issuecomment-918419864
LGTM. BTW this is interesting that we observe meaningful amount of time is used in error handling. Would you like to provide some details? Just curious 😄 Thanks a lot!
--
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] petersalas commented on pull request #8995: [Autoscheduler] Reduce task weight coercion overhead
Posted by GitBox <gi...@apache.org>.
petersalas commented on pull request #8995:
URL: https://github.com/apache/tvm/pull/8995#issuecomment-918441903
@junrushao1994 the (hidden) place this ends up getting hit is this function:
```python
self.objective_func = lambda costs: sum(c * w for c, w in zip(costs, task_weights))
```
because `extract_tasks` was returning `IntImm` for task weights (rather than `int`) every multiplication went down a numpy reflection path with a stack like:
```
> _dl_addr (/lib/x86_64-linux-gnu/libc-2.27.so:0)
> backtrace_symbols (/lib/x86_64-linux-gnu/libc-2.27.so:0)
> dmlc::StackTrace[abi:cxx11] (/usr/hulk/3rdparty/tvm/build/libtvm.so:0)
> tvm::runtime::Backtrace[abi:cxx11] (/usr/hulk/3rdparty/tvm/build/libtvm.so:0)
> tvm::runtime::detail::LogFatal::Entry::Finalize (/usr/hulk/3rdparty/tvm/build/libtvm.so:0)
> tvm::ReflectionVTable::GetAttr (/usr/hulk/3rdparty/tvm/build/libtvm.so:0)
> tvm::NodeGetAttr (/usr/hulk/3rdparty/tvm/build/libtvm.so:0)
> std::_Function_handler<void (tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*), void (*)(tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*)>::_M_invoke (/usr/hulk/3rdparty/tvm/build/libtvm.so:0)
> TVMFuncCall (/usr/hulk/3rdparty/tvm/build/libtvm.so:0)
> ffi_call_unix64 (/usr/lib/x86_64-linux-gnu/libffi.so.6.0.4:0)
> ffi_call (/usr/lib/x86_64-linux-gnu/libffi.so.6.0.4:0)
> _ctypes_callproc (/usr/lib/python3.7/lib-dynload/_ctypes.cpython-37m-x86_64-linux-gnu.so:0)
> 0x7ffb404bf9e3 (/usr/lib/python3.7/lib-dynload/_ctypes.cpython-37m-x86_64-linux-gnu.so:0)
> __call__ (/usr/tvm/python/tvm/_ffi/_ctypes/packed_func.py:233)
> __getattr__ (/usr/tvm/python/tvm/runtime/object.py:65)
> PyArray_FromArrayAttr (/usr/local/lib/python3.7/dist-packages/numpy/core/_multiarray_umath.cpython-37m-x86_64-linux-gnu.so:0)
> _array_from_array_like (/usr/local/lib/python3.7/dist-packages/numpy/core/_multiarray_umath.cpython-37m-x86_64-linux-gnu.so:0)
> PyArray_DiscoverDTypeAndShape_Recursive (/usr/local/lib/python3.7/dist-packages/numpy/core/_multiarray_umath.cpython-37m-x86_64-linux-gnu.so:0)
> PyArray_DiscoverDTypeAndShape (/usr/local/lib/python3.7/dist-packages/numpy/core/_multiarray_umath.cpython-37m-x86_64-linux-gnu.so:0)
> PyArray_FromAny (/usr/local/lib/python3.7/dist-packages/numpy/core/_multiarray_umath.cpython-37m-x86_64-linux-gnu.so:0)
> get_ufunc_arguments (/usr/local/lib/python3.7/dist-packages/numpy/core/_multiarray_umath.cpython-37m-x86_64-linux-gnu.so:0)
> PyUFunc_GenericFunction_int (/usr/local/lib/python3.7/dist-packages/numpy/core/_multiarray_umath.cpython-37m-x86_64-linux-gnu.so:0)
> ufunc_generic_call (/usr/local/lib/python3.7/dist-packages/numpy/core/_multiarray_umath.cpython-37m-x86_64-linux-gnu.so:0)
> array_multiply (/usr/local/lib/python3.7/dist-packages/numpy/core/_multiarray_umath.cpython-37m-x86_64-linux-gnu.so:0)
> double_multiply (/usr/local/lib/python3.7/dist-packages/numpy/core/_multiarray_umath.cpython-37m-x86_64-linux-gnu.so:0)
> <genexpr> (/usr/tvm/python/tvm/auto_scheduler/task_scheduler.py:226)
> <lambda> (/usr/tvm/python/tvm/auto_scheduler/task_scheduler.py:226)
> _compute_score (/usr/tvm/python/tvm/auto_scheduler/task_scheduler.py:483)
```
Note that either part of this change is sufficient to make the cost negligible. The open question is whether there may be other scenarios where the `LOG(FATAL)` could cause the same issue elsewhere. I don't have a lot of context with which to evaluate that perf/debuggability tradeoff so I'll defer to reviewers on 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] masahi merged pull request #8995: [Autoscheduler] Reduce task weight coercion overhead
Posted by GitBox <gi...@apache.org>.
masahi merged pull request #8995:
URL: https://github.com/apache/tvm/pull/8995
--
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] petersalas commented on a change in pull request #8995: [Autoscheduler] Reduce task weight coercion overhead
Posted by GitBox <gi...@apache.org>.
petersalas commented on a change in pull request #8995:
URL: https://github.com/apache/tvm/pull/8995#discussion_r707595552
##########
File path: python/tvm/auto_scheduler/relay_integration.py
##########
@@ -171,7 +171,7 @@ def extract_tasks(
desc=",".join(func_names),
)
)
- weights.append(weight)
+ weights.append(int(weight))
Review comment:
Need to confirm that nothing expects this to be an `IntImm` downstream
--
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] tkonolige commented on a change in pull request #8995: [Autoscheduler] Reduce task weight coercion overhead
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #8995:
URL: https://github.com/apache/tvm/pull/8995#discussion_r707573318
##########
File path: src/node/reflection.cc
##########
@@ -106,8 +106,12 @@ runtime::TVMRetValue ReflectionVTable::GetAttr(Object* self, const String& field
}
}
if (!success) {
- LOG(FATAL) << "AttributeError: " << self->GetTypeKey() << " object has no attributed "
- << getter.skey;
Review comment:
It seems like the issue is various python functions repeatedly calling `getattr` on TVM Objects. Could we have a separate `GetAttr` for python that does not collect a backtrace? Or we could add `HasAttr` and have the python code check that the attr exists before calling getattr.
--
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] tkonolige commented on a change in pull request #8995: [Autoscheduler] Reduce task weight coercion overhead
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #8995:
URL: https://github.com/apache/tvm/pull/8995#discussion_r707545836
##########
File path: src/node/reflection.cc
##########
@@ -106,8 +106,12 @@ runtime::TVMRetValue ReflectionVTable::GetAttr(Object* self, const String& field
}
}
if (!success) {
- LOG(FATAL) << "AttributeError: " << self->GetTypeKey() << " object has no attributed "
- << getter.skey;
Review comment:
I think we should keep the LOG(FATAL). It is used in a lot of places. Instead, we should fix where ever is calling GetAttr a lot.
--
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] petersalas commented on a change in pull request #8995: [Autoscheduler] Reduce task weight coercion overhead
Posted by GitBox <gi...@apache.org>.
petersalas commented on a change in pull request #8995:
URL: https://github.com/apache/tvm/pull/8995#discussion_r707603172
##########
File path: python/tvm/auto_scheduler/relay_integration.py
##########
@@ -171,7 +171,7 @@ def extract_tasks(
desc=",".join(func_names),
)
)
- weights.append(weight)
+ weights.append(int(weight))
Review comment:
Ah, I see that `_compute_score` recently had some changes to ensure that it returns a float (02f885aacde5739c060507afa0b534019d5439cc) which address this.
##########
File path: python/tvm/auto_scheduler/relay_integration.py
##########
@@ -171,7 +171,7 @@ def extract_tasks(
desc=",".join(func_names),
)
)
- weights.append(weight)
+ weights.append(int(weight))
Review comment:
Ah, I see that `_compute_score` recently had some changes to ensure that it returns a float (02f885aacde5739c060507afa0b534019d5439cc) which addresses this.
--
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] petersalas commented on a change in pull request #8995: [Autoscheduler] Reduce task weight coercion overhead
Posted by GitBox <gi...@apache.org>.
petersalas commented on a change in pull request #8995:
URL: https://github.com/apache/tvm/pull/8995#discussion_r707604222
##########
File path: src/node/reflection.cc
##########
@@ -106,8 +106,12 @@ runtime::TVMRetValue ReflectionVTable::GetAttr(Object* self, const String& field
}
}
if (!success) {
- LOG(FATAL) << "AttributeError: " << self->GetTypeKey() << " object has no attributed "
- << getter.skey;
Review comment:
Removed the `GetAttr` changes as they're not necessary for this scenario with the change to `extract_tasks` -- if we see other scenarios where `__getattr__` shows up as hot we can pursue a deeper solution.
--
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