areusch commented on a change in pull request #7919:
URL: https://github.com/apache/tvm/pull/7919#discussion_r625221430
##########
File path: include/tvm/runtime/c_runtime_api.h
##########
@@ -32,6 +32,11 @@
* The common flow is:
* - Use TVMFuncListGlobalNames to get global function name
* - Use TVMFuncCall to call these functions.
+ *
+ * Possible return values of the API functions:
Review comment:
it would be nice to instead refer to an enum, similar to
`tvm/runtime/crt/error_codes.h`
##########
File path: src/runtime/registry.cc
##########
@@ -102,6 +103,76 @@ std::vector<std::string> Registry::ListNames() {
return keys;
}
+/*!
+ * \brief Execution environment specific API registry.
+ *
+ * This registry stores C API function pointers about
+ * execution environment(e.g. python) specific API function that
+ * we need for specific low-level handling(e.g. signal checking).
+ *
+ * We only stores the C API function when absolutely necessary (e.g. when
signal handler
+ * cannot trap back into python). Always consider use the PackedFunc FFI when
possible
+ * in other cases.
+ */
+class EnvCAPIRegistry {
+ public:
+ /*!
+ * \brief Callback to check if signals have been sent to the process and
+ * if so invoke the registered signal handler in the frontend
environment.
+ *
+ * When runnning TVM in another langugage(python), the signal handler
+ * may not be immediately executed, but instead the signal is marked
+ * in the interpreter state(to ensure non-blocking of the signal handler).
+ *
+ * \return 0 if no error happens, -1 if error happens.
+ */
+ typedef int (*F_PyErr_CheckSignals)();
+
+ // NOTE: the following function are only registered
+ // in a python environment.
+ /*!
+ * \brief PyErr_CheckSignal function
+ */
+ F_PyErr_CheckSignals pyerr_check_signals = nullptr;
+
+ static EnvCAPIRegistry* Global() {
+ static EnvCAPIRegistry* inst = new EnvCAPIRegistry();
+ return inst;
+ }
+
+ // register environment(e.g. python) specific api functions
+ void Register(const std::string& symbol_name, void* fptr) {
+ if (symbol_name == "PyErr_CheckSignals") {
Review comment:
constant?
##########
File path: include/tvm/runtime/logging.h
##########
@@ -198,6 +198,19 @@ class Error : public ::dmlc::Error { // for backwards
compatibility
explicit Error(const std::string& s) : ::dmlc::Error(s) {}
};
+/*!
+ * \brief Error message already set in frontend env.
+ * This error can be thrown by EnvCheckSignals
+ */
+class EnvErrorAlreadySet : public ::dmlc::Error {
Review comment:
as an error, seems like we could catch something named e.g.
FrontendError. it should be possible to refer to a doc describing this by a
canonical name to learn things like:
- why is this error thrown?
- who does it?
- what is the generally-suggested cleanup/try-catch behavior when this
happens?
##########
File path: include/tvm/runtime/c_runtime_api.h
##########
@@ -32,6 +32,11 @@
* The common flow is:
* - Use TVMFuncListGlobalNames to get global function name
* - Use TVMFuncCall to call these functions.
+ *
+ * Possible return values of the API functions:
+ * * 0: success
+ * * -1: the error can be retrieved through TVMGetLastError.
+ * * -2: a frontend error ocurred and recorded in the frontend.
Review comment:
nit: sp: occurred
##########
File path: include/tvm/runtime/registry.h
##########
@@ -52,6 +52,45 @@
namespace tvm {
namespace runtime {
+/*!
+ * \brief Check if signals have been sent to the process and if so
+ * invoke the registered signal handler in the frontend environment.
+ *
+ * When runnning TVM in another langugage(python), the signal handler
+ * may not be immediately executed, but instead the signal is marked
+ * in the interpreter state(to ensure non-blocking of the signal handler).
+ *
+ * This function can be explicitly invoked to check the cached signal
+ * and run the related processing if a signal is marked.
+ *
+ * Invoke this function periodically in a long running C++ function
+ * to check if KeyboardInterrupt happens in a python execution environment.
+ *
+ * Not inserting this function will not cause any correctness
+ * issue, but will delay the KeyboardInterrupt until the function returns
+ * to the python side. So this function is not needed in most API
+ * functions can finish quickly in a reasonable amount of time.
+ *
+ * \code
+ *
+ * int check_signal_every_kiter = 10;
+ *
+ * for (int iter = 0; iter < very_large_number; ++iter) {
+ * if (iter % check_signal_every_kiter == 0) {
+ * tvm::runtime::EnvCheckSignals();
+ * }
+ * // do work here
+ * }
+ *
+ * \endcode
+ *
+ * \note This function is a nop when no signal checking function is registered.
Review comment:
can you just update the docstring to refer to specific names rather than
the general concept of "signal checking function"?
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
